From 2dc588ea9578e54e5971c0730ae78313e5a804cf Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Mon, 14 Oct 2024 18:16:30 +0300 Subject: [PATCH 1/5] neotest: distinguish coverage modes Use calls frequency calculated by executor in the final coverage profile for `atomic` cover mode. Support only `set` cover mode for now due to #3587. Signed-off-by: Anna Shaleva --- pkg/neotest/coverage.go | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/pkg/neotest/coverage.go b/pkg/neotest/coverage.go index f5bbf2e34..4dc285865 100644 --- a/pkg/neotest/coverage.go +++ b/pkg/neotest/coverage.go @@ -16,13 +16,22 @@ import ( ) const ( - // goCoverProfileFlag specifies the name of `go test` flag that tells it where to save coverage data. - // Neotest coverage can be enabled only when this flag is set. + // goCoverProfileFlag specifies the name of `go test` command flag `coverprofile` + // that tells it where to save coverage data. Neotest coverage can be enabled + // only when this flag is set. goCoverProfileFlag = "test.coverprofile" + // goCoverModeFlag specifies the name of `go test` command flag `covermode` that + // specifies the coverage calculation mode. + goCoverModeFlag = "test.covermode" // disableNeotestCover is name of the environmental variable used to explicitly disable neotest coverage. disableNeotestCover = "DISABLE_NEOTEST_COVER" ) +const ( + // goCoverModeSet is the name of "set" go test coverage mode. + goCoverModeSet = "set" +) + var ( // coverageLock protects all vars below from concurrent modification when tests are run in parallel. coverageLock sync.Mutex @@ -34,6 +43,8 @@ var ( coverageEnabled bool // coverProfile specifies the file all coverage data is written to, unless empty. coverProfile = "" + // coverMode is the mode of go coverage collection. + coverMode = goCoverModeSet ) type scriptRawCoverage struct { @@ -83,11 +94,17 @@ func isCoverageEnabled() bool { goToolCoverageEnabled = true coverProfile = f.Value.String() } + if f.Name == goCoverModeFlag && f.Value != nil && f.Value.String() != "" { + coverMode = f.Value.String() + } }) coverageEnabled = !disabledByEnvironment && goToolCoverageEnabled if coverageEnabled { + if coverMode != goCoverModeSet { + t.Fatalf("coverage: only '%s' cover mode is currently supported (#3587), got '%s'", goCoverModeSet, coverMode) + } // This is needed so go cover tool doesn't overwrite // the file with our coverage when all tests are done. err := flag.Set(goCoverProfileFlag, "") @@ -119,19 +136,19 @@ func reportCoverage(t testing.TB) { } func writeCoverageReport(w io.Writer) { - fmt.Fprintf(w, "mode: set\n") + fmt.Fprintf(w, "mode: %s\n", coverMode) cover := processCover() for name, blocks := range cover { for _, b := range blocks { - c := 0 - if b.counts > 0 { - c = 1 + var counts = b.counts + if coverMode == goCoverModeSet && counts > 0 { + counts = 1 } fmt.Fprintf(w, "%s:%d.%d,%d.%d %d %d\n", name, b.startLine, b.startCol, b.endLine, b.endCol, b.stmts, - c, + counts, ) } } From 49f2e1dc64dd38ceaa1cbd82c99b814e6f50cb11 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Tue, 15 Oct 2024 15:17:11 +0300 Subject: [PATCH 2/5] neotest: gracefully report about coverage setup error Don't use panic when we can use t.Fatal. Signed-off-by: Anna Shaleva --- pkg/neotest/basic.go | 6 +++--- pkg/neotest/coverage.go | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/neotest/basic.go b/pkg/neotest/basic.go index 3a0de0305..3e03b5a68 100644 --- a/pkg/neotest/basic.go +++ b/pkg/neotest/basic.go @@ -48,7 +48,7 @@ func NewExecutor(t testing.TB, bc *core.Blockchain, validator, committee Signer) Validator: validator, Committee: committee, CommitteeHash: committee.ScriptHash(), - collectCoverage: isCoverageEnabled(), + collectCoverage: isCoverageEnabled(t), } } @@ -452,8 +452,8 @@ func (e *Executor) GetTxExecResult(t testing.TB, h util.Uint256) *state.AppExecR } // EnableCoverage enables coverage collection for this executor, but only when `go test` is running with coverage enabled. -func (e *Executor) EnableCoverage() { - e.collectCoverage = isCoverageEnabled() +func (e *Executor) EnableCoverage(t testing.TB) { + e.collectCoverage = isCoverageEnabled(t) } // DisableCoverage disables coverage collection for this executor until enabled explicitly through EnableCoverage. diff --git a/pkg/neotest/coverage.go b/pkg/neotest/coverage.go index 4dc285865..bd6218511 100644 --- a/pkg/neotest/coverage.go +++ b/pkg/neotest/coverage.go @@ -70,7 +70,7 @@ type coverBlock struct { // documentName makes it clear when a `string` maps path to the script file. type documentName = string -func isCoverageEnabled() bool { +func isCoverageEnabled(t testing.TB) bool { coverageLock.Lock() defer coverageLock.Unlock() @@ -83,7 +83,7 @@ func isCoverageEnabled() bool { if v, ok := os.LookupEnv(disableNeotestCover); ok { disabled, err := strconv.ParseBool(v) if err != nil { - panic(fmt.Sprintf("coverage: error when parsing environment variable '%s', expected bool, but got '%s'", disableNeotestCover, v)) + t.Fatalf("coverage: error when parsing environment variable '%s', expected bool, but got '%s'", disableNeotestCover, v) } disabledByEnvironment = disabled } @@ -109,7 +109,7 @@ func isCoverageEnabled() bool { // the file with our coverage when all tests are done. err := flag.Set(goCoverProfileFlag, "") if err != nil { - panic(err) + t.Fatalf("coverage: failed to overwrite coverprofile flag: %v", err) } } From c747bb8ff781fbe8a9194f0728ec4691eea28177 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Tue, 15 Oct 2024 15:18:45 +0300 Subject: [PATCH 3/5] neotest: preallocate coverage blocks list And always use pointers for coverage block processing, dereference is excessive in this context. Signed-off-by: Anna Shaleva --- pkg/neotest/coverage.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/neotest/coverage.go b/pkg/neotest/coverage.go index bd6218511..7e502d9dc 100644 --- a/pkg/neotest/coverage.go +++ b/pkg/neotest/coverage.go @@ -154,7 +154,7 @@ func writeCoverageReport(w io.Writer) { } } -func processCover() map[documentName][]coverBlock { +func processCover() map[documentName][]*coverBlock { documents := make(map[documentName]struct{}) for _, scriptRawCoverage := range rawCoverage { for _, documentName := range scriptRawCoverage.debugInfo.Documents { @@ -162,7 +162,7 @@ func processCover() map[documentName][]coverBlock { } } - cover := make(map[documentName][]coverBlock) + cover := make(map[documentName][]*coverBlock) for documentName := range documents { mappedBlocks := make(map[int]*coverBlock) @@ -197,9 +197,9 @@ func processCover() map[documentName][]coverBlock { } } - var blocks []coverBlock + var blocks = make([]*coverBlock, 0, len(mappedBlocks)) for _, b := range mappedBlocks { - blocks = append(blocks, *b) + blocks = append(blocks, b) } cover[documentName] = blocks } From d8e945978af6fe7ee32ee62d249700d0758458cf Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Tue, 15 Oct 2024 15:22:54 +0300 Subject: [PATCH 4/5] neotest: sort coverage blocks within a test scope Make the behaviour similar to the `go test` output. It's not a problem for the `go cover` tool, but the sorted file is easier to debug and analize. Signed-off-by: Anna Shaleva --- pkg/neotest/coverage.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/pkg/neotest/coverage.go b/pkg/neotest/coverage.go index 7e502d9dc..f22580ab3 100644 --- a/pkg/neotest/coverage.go +++ b/pkg/neotest/coverage.go @@ -1,10 +1,12 @@ package neotest import ( + "cmp" "flag" "fmt" "io" "os" + "slices" "strconv" "sync" "testing" @@ -201,6 +203,15 @@ func processCover() map[documentName][]*coverBlock { for _, b := range mappedBlocks { blocks = append(blocks, b) } + slices.SortFunc(blocks, func(a, b *coverBlock) int { + return cmp.Or( + cmp.Compare(a.startLine, b.startLine), + cmp.Compare(a.endLine, b.endLine), + cmp.Compare(a.startCol, b.startCol), + cmp.Compare(a.endCol, b.endCol), + cmp.Compare(a.stmts, b.stmts), + cmp.Compare(a.counts, b.counts)) + }) cover[documentName] = blocks } From c1444d45a4504f1fd1e072839e133725e2e29811 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Tue, 15 Oct 2024 16:46:16 +0300 Subject: [PATCH 5/5] nns: update neo-go dependency Signed-off-by: Anna Shaleva --- examples/nft-nd-nns/go.mod | 2 +- examples/nft-nd-nns/go.sum | 4 ++-- examples/nft-nd-nns/nns_test.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/examples/nft-nd-nns/go.mod b/examples/nft-nd-nns/go.mod index 03db73d5d..67fa23930 100644 --- a/examples/nft-nd-nns/go.mod +++ b/examples/nft-nd-nns/go.mod @@ -3,7 +3,7 @@ module github.com/nspcc-dev/neo-go/examples/nft-nd-nns go 1.22 require ( - github.com/nspcc-dev/neo-go v0.106.4-0.20241007161539-0968c3a81fb0 + github.com/nspcc-dev/neo-go v0.106.4-0.20241016130346-d8e945978af6 github.com/nspcc-dev/neo-go/pkg/interop v0.0.0-20240727093519-1a48f1ce43ec github.com/stretchr/testify v1.9.0 ) diff --git a/examples/nft-nd-nns/go.sum b/examples/nft-nd-nns/go.sum index afc7ebda5..5f827d3ac 100644 --- a/examples/nft-nd-nns/go.sum +++ b/examples/nft-nd-nns/go.sum @@ -74,8 +74,8 @@ github.com/nspcc-dev/go-ordered-json v0.0.0-20240830112754-291b000d1f3b h1:DRG4c github.com/nspcc-dev/go-ordered-json v0.0.0-20240830112754-291b000d1f3b/go.mod h1:d3cUseu4Asxfo9/QA/w4TtGjM0AbC9ynyab+PfH+Bso= github.com/nspcc-dev/hrw/v2 v2.0.1 h1:CxYUkBeJvNfMEn2lHhrV6FjY8pZPceSxXUtMVq0BUOU= github.com/nspcc-dev/hrw/v2 v2.0.1/go.mod h1:iZAs5hT2q47EGq6AZ0FjaUI6ggntOi7vrY4utfzk5VA= -github.com/nspcc-dev/neo-go v0.106.4-0.20241007161539-0968c3a81fb0 h1:8IYGFnZQ+wyH5Qdtq4oxBEyiWSNMdi6AXSHQunyZ9VU= -github.com/nspcc-dev/neo-go v0.106.4-0.20241007161539-0968c3a81fb0/go.mod h1:ds91T4WJwtk7eWUo0fuVC36HpTQKkkdj5AjNxbjXAR0= +github.com/nspcc-dev/neo-go v0.106.4-0.20241016130346-d8e945978af6 h1:3/V1U2ZgbFZQ5xGjTD9NMsz4NHzPy/nYJzcaHDVDu88= +github.com/nspcc-dev/neo-go v0.106.4-0.20241016130346-d8e945978af6/go.mod h1:ds91T4WJwtk7eWUo0fuVC36HpTQKkkdj5AjNxbjXAR0= github.com/nspcc-dev/neo-go/pkg/interop v0.0.0-20240727093519-1a48f1ce43ec h1:vDrbVXF2+2uP0RlkZmem3QYATcXCu9BzzGGCNsNcK7Q= github.com/nspcc-dev/neo-go/pkg/interop v0.0.0-20240727093519-1a48f1ce43ec/go.mod h1:/vrbWSHc7YS1KSYhVOyyeucXW/e+1DkVBOgnBEXUCeY= github.com/nspcc-dev/neofs-api-go/v2 v2.14.1-0.20240305074711-35bc78d84dc4 h1:arN0Ypn+jawZpu1BND7TGRn44InAVIqKygndsx0y2no= diff --git a/examples/nft-nd-nns/nns_test.go b/examples/nft-nd-nns/nns_test.go index d0d043c7c..607bc4465 100644 --- a/examples/nft-nd-nns/nns_test.go +++ b/examples/nft-nd-nns/nns_test.go @@ -381,7 +381,7 @@ func TestTransfer(t *testing.T) { func OnNEP11Payment(from interop.Hash160, amount int, token []byte, data any) {}`), &compiler.Options{Name: "foo"}) e.DeployContract(t, ctr, nil) - e.EnableCoverage() // contracts above have no source files which leads to unprocessable coverage data + e.EnableCoverage(t) // contracts above have no source files which leads to unprocessable coverage data cTo.Invoke(t, true, "transfer", ctr.Hash, []byte("neo.com"), nil) cFrom.Invoke(t, 1, "totalSupply") cFrom.Invoke(t, ctr.Hash.BytesBE(), "ownerOf", []byte("neo.com"))