Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(gnovm): add gno test parallel package testing #3431

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
Draft
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
2 changes: 1 addition & 1 deletion .github/workflows/examples.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ jobs:
echo "LOG_LEVEL=debug" >> $GITHUB_ENV
echo "LOG_PATH_DIR=$LOG_PATH_DIR" >> $GITHUB_ENV
- run: go install -v ./gnovm/cmd/gno
- run: go run ./gnovm/cmd/gno test -v -print-runtime-metrics -print-events ./examples/...
- run: go run ./gnovm/cmd/gno test -parallel=0 -v -print-runtime-metrics -print-events ./examples/...
Copy link
Member

Choose a reason for hiding this comment

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

why specify this?

lint:
strategy:
fail-fast: false
Expand Down
21 changes: 20 additions & 1 deletion .github/workflows/gnovm.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,27 @@ jobs:
tests-extra-args: "-coverpkg=github.com/gnolang/gno/gnovm/..."
secrets:
codecov-token: ${{ secrets.CODECOV_TOKEN }}
fmt:

stdlibs-fmt:
name: Run gno fmt on stdlibs
uses: ./.github/workflows/gnofmt_template.yml
with:
path: "gnovm/stdlibs/..."

stdlibs-test:
name: Run gno test on stdlibs
runs-on: ubuntu-latest
steps:
- name: Checkout code
uses: actions/checkout@v4
- name: Install Go
uses: actions/setup-go@v5
with:
go-version: 1.22
- name: Install Gno
working-directory: gnovm
run: go install -v ./cmd/gno
- name: Go test
working-directory: gnovm
# use 0 for GOMAXPROCS
run: gno test -v -parallel=0 ./stdlibs/...
10 changes: 5 additions & 5 deletions .github/workflows/test_template.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@ jobs:
COVERDIR: /tmp/coverdir # final output
run: |
set -x # print commands

mkdir -p "$GOCOVERDIR" "$TXTARCOVERDIR" "$COVERDIR"

# Craft a filter flag based on the module path to avoid expanding coverage on unrelated tags.
export filter="-pkg=github.com/gnolang/gno/${{ inputs.modulepath }}/..."

# codecov only supports "boolean" coverage (whether a line is
# covered or not); so using -covermode=count or atomic would be
# pointless here.
Expand All @@ -49,13 +49,13 @@ jobs:
# 1.23 regarding coverage, so we can use this as a workaround until
# then.
go test -covermode=set -timeout ${{ inputs.tests-timeout }} ${{ inputs.tests-extra-args }} ./... -test.gocoverdir=$GOCOVERDIR

# Print results
(set +x; echo 'go coverage results:')
go tool covdata percent $filter -i=$GOCOVERDIR
(set +x; echo 'txtar coverage results:')
go tool covdata percent $filter -i=$TXTARCOVERDIR

# Generate final coverage output
go tool covdata textfmt -v 1 $filter -i=$GOCOVERDIR,$TXTARCOVERDIR -o gocoverage.out

Expand Down
91 changes: 66 additions & 25 deletions gnovm/cmd/gno/test.go
Copy link
Member

Choose a reason for hiding this comment

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

We need to make the output buffered for each test if maxWorkers > 1, so that the full output of a package is all printed together rather than being mixed with that of other tests.

Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
goio "io"
"log"
"path/filepath"
"runtime"
"strings"
"sync"
"time"

"github.com/gnolang/gno/gnovm/pkg/gnoenv"
Expand All @@ -16,13 +18,15 @@
"github.com/gnolang/gno/gnovm/pkg/test"
"github.com/gnolang/gno/tm2/pkg/commands"
"github.com/gnolang/gno/tm2/pkg/random"
"golang.org/x/sync/semaphore"
)

type testCfg struct {
verbose bool
rootDir string
run string
timeout time.Duration
parallel int
updateGoldenTests bool
printRuntimeMetrics bool
printEvents bool
Expand Down Expand Up @@ -123,6 +127,13 @@
"test name filtering pattern",
)

fs.IntVar(
&c.parallel,
"parallel",
0,
"number of packages to run concurrently; 0 = GOMAXPROCS",
)

fs.DurationVar(
&c.timeout,
"timeout",
Expand Down Expand Up @@ -181,15 +192,19 @@
if cfg.verbose {
stdout = io.Out()
}
opts := test.NewTestOptions(cfg.rootDir, io.In(), stdout, io.Err())
opts.RunFlag = cfg.run
opts.Sync = cfg.updateGoldenTests
opts.Verbose = cfg.verbose
opts.Metrics = cfg.printRuntimeMetrics
opts.Events = cfg.printEvents

buildErrCount := 0
testErrCount := 0

var buildErrCount, testErrCount uint64

maxWorkers := runtime.GOMAXPROCS(0)
if cfg.parallel > 0 {
maxWorkers = cfg.parallel
}

Check warning on line 201 in gnovm/cmd/gno/test.go

View check run for this annotation

Codecov / codecov/patch

gnovm/cmd/gno/test.go#L200-L201

Added lines #L200 - L201 were not covered by tests
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
}
}
maxWorkers = min(maxWorkers, subPkgs)

sem := semaphore.NewWeighted(int64(maxWorkers))

ctx, cancel := context.WithCancel(context.Background())
defer cancel()

var wg sync.WaitGroup
for _, pkg := range subPkgs {
if len(pkg.TestGnoFiles) == 0 && len(pkg.FiletestGnoFiles) == 0 {
io.ErrPrintfln("? %s \t[no test files]", pkg.Dir)
Expand All @@ -211,26 +226,52 @@

memPkg := gno.MustReadMemPackage(pkg.Dir, gnoPkgPath)

startedAt := time.Now()
hasError := catchRuntimeError(gnoPkgPath, io.Err(), func() {
err = test.Test(memPkg, pkg.Dir, opts)
})
var syncWrite sync.Mutex

duration := time.Since(startedAt)
dstr := fmtDuration(duration)
wg.Add(1)
go func() {
defer wg.Done()

if hasError || err != nil {
if err != nil {
io.ErrPrintfln("%s: test pkg: %v", pkg.Dir, err)
if err := sem.Acquire(ctx, 1); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

This could be a channel where you write at the beginning ch <- struct{}{} and do a defer func() { <- ch } at the end.

return

Check warning on line 236 in gnovm/cmd/gno/test.go

View check run for this annotation

Codecov / codecov/patch

gnovm/cmd/gno/test.go#L236

Added line #L236 was not covered by tests
}
io.ErrPrintfln("FAIL")
io.ErrPrintfln("FAIL %s \t%s", pkg.Dir, dstr)
io.ErrPrintfln("FAIL")
testErrCount++
} else {
io.ErrPrintfln("ok %s \t%s", pkg.Dir, dstr)
}
defer sem.Release(1)

opts := test.NewTestOptions(cfg.rootDir, io.In(), stdout, io.Err())
opts.RunFlag = cfg.run
opts.Sync = cfg.updateGoldenTests
opts.Verbose = cfg.verbose
opts.Metrics = cfg.printRuntimeMetrics
opts.Events = cfg.printEvents

startedAt := time.Now()
hasError := catchRuntimeError(gnoPkgPath, io.Err(), func() {
err = test.Test(memPkg, pkg.Dir, opts)
})

duration := time.Since(startedAt)
dstr := fmtDuration(duration)

syncWrite.Lock()
defer syncWrite.Unlock()

if hasError || err != nil {
if err != nil {
io.ErrPrintfln("%s: test pkg: %v", pkg.Dir, err)
}
io.ErrPrintfln("FAIL")
io.ErrPrintfln("FAIL %s \t%s", pkg.Dir, dstr)
io.ErrPrintfln("FAIL")

testErrCount++
} else {
io.ErrPrintfln("ok %s \t%s", pkg.Dir, dstr)
}
}()
}

wg.Wait()

if testErrCount > 0 || buildErrCount > 0 {
io.ErrPrintfln("FAIL")
return fmt.Errorf("FAIL: %d build errors, %d test errors", buildErrCount, testErrCount)
Expand Down
148 changes: 73 additions & 75 deletions gnovm/pkg/gnolang/files_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package gnolang_test

import (
"bytes"
"flag"
"fmt"
"io"
Expand All @@ -11,7 +10,6 @@ import (
"strings"
"testing"

"github.com/gnolang/gno/gnovm/pkg/gnolang"
"github.com/gnolang/gno/gnovm/pkg/test"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -107,76 +105,76 @@ func TestFiles(t *testing.T) {
}

// TestStdlibs tests all the standard library packages.
func TestStdlibs(t *testing.T) {
t.Parallel()

rootDir, err := filepath.Abs("../../../")
require.NoError(t, err)

newOpts := func() (capture *bytes.Buffer, opts *test.TestOptions) {
var out io.Writer
if testing.Verbose() {
out = os.Stdout
} else {
capture = new(bytes.Buffer)
out = capture
}
opts = test.NewTestOptions(rootDir, nopReader{}, out, out)
opts.Verbose = true
return
}
sharedCapture, sharedOpts := newOpts()

dir := "../../stdlibs/"
fsys := os.DirFS(dir)
err = fs.WalkDir(fsys, ".", func(path string, de fs.DirEntry, err error) error {
switch {
case err != nil:
return err
case !de.IsDir() || path == ".":
return nil
}

fp := filepath.Join(dir, path)
memPkg := gnolang.MustReadMemPackage(fp, path)
t.Run(strings.ReplaceAll(memPkg.Path, "/", "-"), func(t *testing.T) {
capture, opts := sharedCapture, sharedOpts
switch memPkg.Path {
// Excluded in short
case
"bufio",
"bytes",
"strconv":
if testing.Short() {
t.Skip("Skipped because of -short, and this stdlib is very long currently.")
}
fallthrough
// Run using separate store, as it's faster
case
"math/rand",
"regexp",
"regexp/syntax",
"sort":
t.Parallel()
capture, opts = newOpts()
}

if capture != nil {
capture.Reset()
}

err := test.Test(memPkg, "", opts)
if !testing.Verbose() {
t.Log(capture.String())
}
if err != nil {
t.Error(err)
}
})

return nil
})
if err != nil {
t.Fatal(err)
}
}
// func TestStdlibs(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

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

If we don't have this, then the tests in the standard libraries doesn't count as coverage, and I think it should

// t.Parallel()

// rootDir, err := filepath.Abs("../../../")
// require.NoError(t, err)

// newOpts := func() (capture *bytes.Buffer, opts *test.TestOptions) {
// var out io.Writer
// if testing.Verbose() {
// out = os.Stdout
// } else {
// capture = new(bytes.Buffer)
// out = capture
// }
// opts = test.NewTestOptions(rootDir, nopReader{}, out, out)
// opts.Verbose = true
// return
// }
// sharedCapture, sharedOpts := newOpts()

// dir := "../../stdlibs/"
// fsys := os.DirFS(dir)
// err = fs.WalkDir(fsys, ".", func(path string, de fs.DirEntry, err error) error {
// switch {
// case err != nil:
// return err
// case !de.IsDir() || path == ".":
// return nil
// }

// fp := filepath.Join(dir, path)
// memPkg := gnolang.MustReadMemPackage(fp, path)
// t.Run(strings.ReplaceAll(memPkg.Path, "/", "-"), func(t *testing.T) {
// capture, opts := sharedCapture, sharedOpts
// switch memPkg.Path {
// // Excluded in short
// case
// "bufio",
// "bytes",
// "strconv":
// if testing.Short() {
// t.Skip("Skipped because of -short, and this stdlib is very long currently.")
// }
// fallthrough
// // Run using separate store, as it's faster
// case
// "math/rand",
// "regexp",
// "regexp/syntax",
// "sort":
// t.Parallel()
// capture, opts = newOpts()
// }

// if capture != nil {
// capture.Reset()
// }

// err := test.Test(memPkg, "", opts)
// if !testing.Verbose() {
// t.Log(capture.String())
// }
// if err != nil {
// t.Error(err)
// }
// })

// return nil
// })
// if err != nil {
// t.Fatal(err)
// }
// }
Loading