From 9d9143d4cb58bcccefe98291c2656482af544975 Mon Sep 17 00:00:00 2001 From: Thomas Bruyelle Date: Thu, 17 Aug 2023 18:49:57 +0200 Subject: [PATCH] test(gnovm): fix TestPackages (#964) `TestPackages` wasn't actually reporting test failures, due to a bug in the underlying function `machine.TestMemPackage()`. `TestMemPackage` was running the gno tests by just calling the test function with a new `testing.T`, but then it's not possible to detect if there's any failure, because `testing.T` has only private fields. The fix is to change that to a call to `testing.RunTest()` function, which is already used by `gno test`, and returns a `testing.Report` on which we can detect failures. A test has been added (see `TestMachineTestMemPackage()`) in gnovm/tests folder (because a `TestStore` is needed). On master, this test is failing. ``` go test ./gnovm/tests -v -run TestMachineTestMemPackage ``` The next step should be to merge the 2 ways that currently exists to run gno tests, i.e. `gno test` and `go test -run TestPackages`. Potentially this could fix #896. ## Contributors Checklist - [x] Added new tests, or not needed, or not feasible - [ ] Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory - [ ] Updated the official documentation or not needed - [ ] No breaking changes were made, or a `BREAKING CHANGE: xxx` message was included in the description - [ ] Added references to related issues and PRs - [x] Provided any useful hints for running manual tests - [ ] Added new benchmarks to [generated graphs](https://gnoland.github.io/benchmarks), if any. More info [here](../.benchmarks/README.md). ## Maintainers Checklist - [x] Checked that the author followed the guidelines in `CONTRIBUTING.md` - [x] Checked the conventional-commit (especially PR title and verb, presence of `BREAKING CHANGE:` in the body) - [x] Ensured that this PR is not a significant change or confirmed that the review/consideration process was appropriate for the change --------- Co-authored-by: Manfred Touron <94029+moul@users.noreply.github.com> --- gnovm/pkg/gnolang/machine.go | 113 +++++++++++------- gnovm/tests/machine_test.go | 65 ++++++++++ .../TestMemPackage/fail/file_test.gno | 7 ++ .../TestMemPackage/success/file_test.gno | 5 + 4 files changed, 147 insertions(+), 43 deletions(-) create mode 100644 gnovm/tests/machine_test.go create mode 100644 gnovm/tests/testdata/TestMemPackage/fail/file_test.gno create mode 100644 gnovm/tests/testdata/TestMemPackage/success/file_test.gno diff --git a/gnovm/pkg/gnolang/machine.go b/gnovm/pkg/gnolang/machine.go index 0ad52550684..60008d3c34d 100644 --- a/gnovm/pkg/gnolang/machine.go +++ b/gnovm/pkg/gnolang/machine.go @@ -3,6 +3,7 @@ package gnolang // XXX rename file to machine.go. import ( + "encoding/json" "fmt" "io" "os" @@ -251,10 +252,6 @@ func (m *Machine) TestMemPackage(t *testing.T, memPkg *std.MemPackage) { defer m.injectLocOnPanic() DisableDebug() fmt.Println("DEBUG DISABLED (FOR TEST DEPENDENCIES INIT)") - // prefetch the testing package. - testingpv := m.Store.GetPackage("testing", false) - testingtv := TypedValue{T: gPackageType, V: testingpv} - testingcx := &ConstExpr{TypedValue: testingtv} // parse test files. tfiles, itfiles := ParseMemPackageTests(memPkg) { // first, tfiles which run in the same package. @@ -266,25 +263,8 @@ func (m *Machine) TestMemPackage(t *testing.T, memPkg *std.MemPackage) { m.RunFiles(tfiles.Files...) // run all tests in test files. for i := pvSize; i < len(pvBlock.Values); i++ { - tv := &pvBlock.Values[i] - if !(tv.T.Kind() == FuncKind && - strings.HasPrefix( - string(tv.V.(*FuncValue).Name), - "Test")) { - continue // not a test function. - } - // XXX ensure correct func type. - name := tv.V.(*FuncValue).Name - t.Run(string(name), func(t *testing.T) { - defer m.injectLocOnPanic() - x := Call(name, Call(Sel(testingcx, "NewT"), Str(string(name)))) - res := m.Eval(x) - if len(res) != 0 { - panic(fmt.Sprintf( - "expected no results but got %d", - len(res))) - } - }) + tv := pvBlock.Values[i] + m.TestFunc(t, tv) } } { // run all (import) tests in test files. @@ -299,30 +279,77 @@ func (m *Machine) TestMemPackage(t *testing.T, memPkg *std.MemPackage) { EnableDebug() fmt.Println("DEBUG ENABLED") for i := 0; i < len(pvBlock.Values); i++ { - tv := &pvBlock.Values[i] - if !(tv.T.Kind() == FuncKind && - strings.HasPrefix( - string(tv.V.(*FuncValue).Name), - "Test")) { - continue // not a test function. - } - // XXX ensure correct func type. - name := tv.V.(*FuncValue).Name - t.Run(string(name), func(t *testing.T) { - defer m.injectLocOnPanic() - x := Call(name, Call(Sel(testingcx, "NewT"), Str(string(name)))) - res := m.Eval(x) - if len(res) != 0 { - fmt.Println("ERROR") - panic(fmt.Sprintf( - "expected no results but got %d", - len(res))) - } - }) + tv := pvBlock.Values[i] + m.TestFunc(t, tv) } } } +// TestFunc calls tv with testing.RunTest, if tv is a function with a name that +// starts with `Test`. +func (m *Machine) TestFunc(t *testing.T, tv TypedValue) { + if !(tv.T.Kind() == FuncKind && + strings.HasPrefix(string(tv.V.(*FuncValue).Name), "Test")) { + return // not a test function. + } + // XXX ensure correct func type. + name := string(tv.V.(*FuncValue).Name) + // prefetch the testing package. + testingpv := m.Store.GetPackage("testing", false) + testingtv := TypedValue{T: gPackageType, V: testingpv} + testingcx := &ConstExpr{TypedValue: testingtv} + + t.Run(name, func(t *testing.T) { + defer m.injectLocOnPanic() + x := Call( + Sel(testingcx, "RunTest"), // Call testing.RunTest + Str(name), // First param, the name of the test + X("true"), // Second Param, verbose bool + &CompositeLitExpr{ // Third param, the testing.InternalTest + Type: Sel(testingcx, "InternalTest"), + Elts: KeyValueExprs{ + {Key: X("Name"), Value: Str(name)}, + {Key: X("F"), Value: X(name)}, + }, + }, + ) + res := m.Eval(x) + ret := res[0].GetString() + if ret == "" { + t.Errorf("failed to execute unit test: %q", name) + return + } + + // mirror of stdlibs/testing.Report + var report struct { + Name string + Verbose bool + Failed bool + Skipped bool + Filtered bool + Output string + } + err := json.Unmarshal([]byte(ret), &report) + if err != nil { + t.Errorf("failed to parse test output %q", name) + return + } + + switch { + case report.Filtered: + // noop + case report.Skipped: + t.SkipNow() + case report.Failed: + t.Fail() + } + + if report.Output != "" && (report.Verbose || report.Failed) { + t.Log(report.Output) + } + }) +} + // in case of panic, inject location information to exception. func (m *Machine) injectLocOnPanic() { if r := recover(); r != nil { diff --git a/gnovm/tests/machine_test.go b/gnovm/tests/machine_test.go new file mode 100644 index 00000000000..a67d67f1ff2 --- /dev/null +++ b/gnovm/tests/machine_test.go @@ -0,0 +1,65 @@ +package tests + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + + gno "github.com/gnolang/gno/gnovm/pkg/gnolang" +) + +func TestMachineTestMemPackage(t *testing.T) { + matchFunc := func(pat, str string) (bool, error) { return true, nil } + + tests := []struct { + name string + path string + shouldSucceed bool + }{ + { + name: "TestSuccess", + path: "testdata/TestMemPackage/success", + shouldSucceed: true, + }, + { + name: "TestFail", + path: "testdata/TestMemPackage/fail", + shouldSucceed: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // NOTE: Because the purpose of this test is to ensure testing.T.Failed() + // returns true if a gno test is failing, and because we don't want this + // to affect the current testing.T, we are creating an other one thanks + // to testing.RunTests() function. + testing.RunTests(matchFunc, []testing.InternalTest{ + { + Name: tt.name, + F: func(t2 *testing.T) { //nolint:thelper + rootDir := filepath.Join("..", "..") + store := TestStore(rootDir, "test", os.Stdin, os.Stdout, os.Stderr, ImportModeStdlibsOnly) + store.SetLogStoreOps(true) + m := gno.NewMachineWithOptions(gno.MachineOptions{ + PkgPath: "test", + Output: os.Stdout, + Store: store, + Context: nil, + }) + memPkg := gno.ReadMemPackage(tt.path, "test") + + m.TestMemPackage(t2, memPkg) + + if tt.shouldSucceed { + assert.False(t, t2.Failed(), "test %q should have succeed", tt.name) + } else { + assert.True(t, t2.Failed(), "test %q should have failed", tt.name) + } + }, + }, + }) + }) + } +} diff --git a/gnovm/tests/testdata/TestMemPackage/fail/file_test.gno b/gnovm/tests/testdata/TestMemPackage/fail/file_test.gno new file mode 100644 index 00000000000..b202c40bc46 --- /dev/null +++ b/gnovm/tests/testdata/TestMemPackage/fail/file_test.gno @@ -0,0 +1,7 @@ +package test + +import "testing" + +func TestFail(t *testing.T) { + t.Errorf("OUPS") +} diff --git a/gnovm/tests/testdata/TestMemPackage/success/file_test.gno b/gnovm/tests/testdata/TestMemPackage/success/file_test.gno new file mode 100644 index 00000000000..0fc1d898199 --- /dev/null +++ b/gnovm/tests/testdata/TestMemPackage/success/file_test.gno @@ -0,0 +1,5 @@ +package test + +import "testing" + +func TestSucess(t *testing.T) {}