Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 22 additions & 8 deletions pkg/neotest/basic.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"math/big"
"strings"
"sync"
"testing"

"github.com/nspcc-dev/neo-go/pkg/config/netmode"
Expand Down Expand Up @@ -32,8 +33,16 @@ type Executor struct {
Validator Signer
Committee Signer
CommitteeHash util.Uint160

// collectCoverage is true if coverage is being collected when running this executor.
collectCoverage bool
// scheduleCoverageReport is a cleanup function that schedules the report of
// collected coverage data for this Executor.
scheduleCoverageReport func()
scheduleCoverageReportOnce sync.Once

coverageLock sync.RWMutex
rawCoverage map[util.Uint160]*scriptRawCoverage
}

// NewExecutor creates a new executor instance from the provided blockchain and committee.
Expand All @@ -43,13 +52,20 @@ func NewExecutor(t testing.TB, bc *core.Blockchain, validator, committee Signer)
checkMultiSigner(t, validator)
checkMultiSigner(t, committee)

return &Executor{
e := &Executor{
Chain: bc,
Validator: validator,
Committee: committee,
CommitteeHash: committee.ScriptHash(),
collectCoverage: isCoverageEnabled(t),
rawCoverage: make(map[util.Uint160]*scriptRawCoverage),
}

if e.collectCoverage {
e.scheduleCoverageReport = func() { t.Cleanup(func() { e.reportCoverage(t) }) }
}

return e
}

// TopBlock returns the block with the highest index.
Expand Down Expand Up @@ -148,7 +164,7 @@ func (e *Executor) DeployContract(t testing.TB, c *Contract, data any) util.Uint
// data is an optional argument to `_deploy`.
// It returns the hash of the deploy transaction.
func (e *Executor) DeployContractBy(t testing.TB, signer Signer, c *Contract, data any) util.Uint256 {
e.trackCoverage(t, c)
e.trackCoverage(c)
tx := e.NewDeployTxBy(t, signer, c, data)
e.AddNewBlock(t, tx)
e.CheckHalt(t, tx.Hash())
Expand All @@ -168,19 +184,17 @@ func (e *Executor) DeployContractBy(t testing.TB, signer Signer, c *Contract, da
// DeployContractCheckFAULT compiles and deploys a contract to the bc using the validator
// account. It checks that the deploy transaction FAULTed with the specified error.
func (e *Executor) DeployContractCheckFAULT(t testing.TB, c *Contract, data any, errMessage string) {
e.trackCoverage(t, c)
e.trackCoverage(c)
tx := e.NewDeployTx(t, c, data)
e.AddNewBlock(t, tx)
e.CheckFault(t, tx.Hash(), errMessage)
}

// trackCoverage switches on coverage tracking for provided script if `go test` is running with coverage enabled.
func (e *Executor) trackCoverage(t testing.TB, c *Contract) {
func (e *Executor) trackCoverage(c *Contract) {
if e.collectCoverage {
addScriptToCoverage(c)
t.Cleanup(func() {
reportCoverage(t)
})
e.scheduleCoverageReportOnce.Do(e.scheduleCoverageReport)
}
}

Expand Down Expand Up @@ -417,7 +431,7 @@ func (e *Executor) TestInvoke(tx *transaction.Transaction) (*vm.VM, error) {
ic, _ := e.Chain.GetTestVM(trigger.Application, &ttx, b)

if e.collectCoverage {
ic.VM.SetOnExecHook(coverageHook)
ic.VM.SetOnExecHook(e.coverageHook)
}

defer ic.Finalize()
Expand Down
4 changes: 2 additions & 2 deletions pkg/neotest/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func (c *ContractInvoker) TestInvokeScript(t testing.TB, script []byte, signers
t.Cleanup(ic.Finalize)

if c.collectCoverage {
ic.VM.SetOnExecHook(coverageHook)
ic.VM.SetOnExecHook(c.coverageHook)
}

ic.VM.LoadWithFlags(tx.Script, callflag.All)
Expand All @@ -83,7 +83,7 @@ func (c *ContractInvoker) TestInvoke(t testing.TB, method string, args ...any) (
t.Cleanup(ic.Finalize)

if c.collectCoverage {
ic.VM.SetOnExecHook(coverageHook)
ic.VM.SetOnExecHook(c.coverageHook)
}

ic.VM.LoadWithFlags(tx.Script, callflag.All)
Expand Down
26 changes: 20 additions & 6 deletions pkg/neotest/coverage.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (

"github.com/nspcc-dev/neo-go/pkg/compiler"
"github.com/nspcc-dev/neo-go/pkg/util"
"github.com/nspcc-dev/neo-go/pkg/vm"
"github.com/nspcc-dev/neo-go/pkg/vm/opcode"
)

Expand Down Expand Up @@ -118,17 +117,32 @@ func isCoverageEnabled(t testing.TB) bool {
return coverageEnabled
}

var coverageHook vm.OnExecHook = func(scriptHash util.Uint160, offset int, opcode opcode.Opcode) {
coverageLock.Lock()
defer coverageLock.Unlock()
if cov, ok := rawCoverage[scriptHash]; ok {
func (e *Executor) coverageHook(scriptHash util.Uint160, offset int, _ opcode.Opcode) {
e.coverageLock.Lock()
defer e.coverageLock.Unlock()
if cov, ok := e.rawCoverage[scriptHash]; ok {
cov.offsetsVisited = append(cov.offsetsVisited, offset)
}
}

func reportCoverage(t testing.TB) {
func (e *Executor) reportCoverage(t testing.TB) {
coverageLock.Lock()
defer coverageLock.Unlock()

e.coverageLock.RLock()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this one probably doesn't require any locking since it's executed as the test cleanup function, Executor should be done by that time.

for h, cov := range e.rawCoverage {
fullCov := rawCoverage[h]
if fullCov == nil {
fullCov = &scriptRawCoverage{
debugInfo: cov.debugInfo,
}
rawCoverage[h] = fullCov
}

fullCov.offsetsVisited = append(fullCov.offsetsVisited, cov.offsetsVisited...)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can break with t.Parallel(), but maybe coverageLock is exactly what we need for the loop.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

t.Parallel() will break (*Executor).EnableCoverage and (*Executor).DisableCoverage as far since e.collectCoverage is not atomic and not supposed to be shared between multiple parallel tests running for a single instance of Executor:

neo-go/pkg/neotest/basic.go

Lines 454 to 462 in 93fe450

// EnableCoverage enables coverage collection for this executor, but only when `go test` is running with coverage enabled.
func (e *Executor) EnableCoverage(t testing.TB) {
e.collectCoverage = isCoverageEnabled(t)
}
// DisableCoverage disables coverage collection for this executor until enabled explicitly through EnableCoverage.
func (e *Executor) DisableCoverage() {
e.collectCoverage = false
}

So right now it looks like we don't fully support parallel coverage collection. Do we need an issue for that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, would be nice to have one since it's not obvious. Should be mentioned in neotest docs then.

}
e.coverageLock.RUnlock()

f, err := os.Create(coverProfile)
if err != nil {
t.Fatalf("coverage: can't create file '%s' to write coverage report", coverProfile)
Expand Down
Loading