fix(gnovm): PrevRealm in _test files

[tbruyelle]

Description

Related to #667 #891

When _test files are executed, the name of the package is set to the real path to those files, which starts by ./examples/gno.land/.... This leads to interpreting the tested package as a non-realm, because a realm must start with gno.land/r.

Because of that, when calling PrevRealm, a tested package is never considered as a realm.

The fix truncates the ./examples prefix from the tested package, which ensures it is well detected as a realm when required.
The fix replaces the usage of the package directory /examples/gno.land/... by a real gno package path.

To determine it, we try to read a gno.mod in the directory passed in argument or in its parents:

• If found, the gno package path is set to the module name found in gno.mod.
• If not found, a randomly generated one is used, that will start with gno.land/r/ (so it will be considered as a realm).

Tests

A test TestPrevRealm has been added:

$ gno test -verbose ./examples/gno.land/r/demo/tests -run TestPrevRealm 
// FAIL with gno binary compiled from master
// OK with gno binary compiled from this PR

Contributors Checklist

• 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
• Provided any useful hints for running manual tests

Maintainers Checklist

• Checked that the author followed the guidelines in CONTRIBUTING.md
• Checked the conventional-commit (especially PR title and verb, presence of BREAKING CHANGE: in the body)
• Ensured that this PR is not a significant change or confirmed that the review/consideration process was appropriate for the change

[r3v4s]

Hmm.. unfortunately It doesn't work on my side.

$ git rev-parse HEAD
c2c716420c67369cc58584e10b9ac342676b8374

$ gno test -root-dir ~/0615/gno -verbose=true ./
=== RUN   TestAssertOriginCall
--- PASS: TestAssertOriginCall (0.00s)
=== RUN   TestPrevRealm
--- FAIL: TestPrevRealm (0.00s)
output: want GetRSubtestsPrevRealm().Addr==g1gz4ycmx0s6ln2wdrsh4e00l9fsel2wskqa3snq, got g1wymu47drhr0kuq2098m792lytgtj2nyx77yrsm
=== RUN   file/z0_filetest.gno
--- PASS: file/z0_filetest.gno (0.00s)
=== RUN   file/z1_filetest.gno
--- PASS: file/z1_filetest.gno (0.00s)
./.: test pkg: failed: "TestPrevRealm"
FAIL
FAIL    ./. 	0.47s
FAIL
?       ./subtests 	[no test files]
FAIL
FAIL: 0 build errors, 1 test errors

[tbruyelle]

@r3v4s Sorry to ask, but are you sure that you recompiled the gno binary with this PR ?

[r3v4s]

@tbruyelle Yep, I did.
I've removed all of existing gno related binary, then ran make install

can you share me pre-built gno binary? (I'm using m1)

[tbruyelle]

@tbruyelle Yep, I did. I've removed all of existing gno related binary, then ran make install

can you share me pre-built gno binary? (I'm using m1)

If you run make install at the root directory, that's enough, I don't understand why the test isn't working. I can't reproduce on my side, it's working well.

[moul]

Can you also check how it goes with external repos? (Not using examples/)?

Will give a deeper look of the diff later, thank you.

[tbruyelle]

Can you also check how it goes with external repos? (Not using examples/)?

Will give a deeper look of the diff later, thank you.

Thank you. Do you have any example of external repos that contains gno packages ?

[tbruyelle]

Can you also check how it goes with external repos? (Not using examples/)?

Will give a deeper look of the diff later, thank you.

@moul So thanks to yesterday meeting I found the gnodaos external repo and I tried to use the gno binary from this PR.

After a couple of changes :

• fix gnodaos code because it is based on gno test3, so for instance it uses gno.land/p/avl while now it's gno.land/p/demo/avl
• fix panic in gno so we don't try to assign an ID to non realm (see commit 336edab)

I was able to run the tests successfully ✔️

Note that gno.land/r/gnodaos is actually a realm, but it's not detected as is because the pkg path in the code is ./r/gnodaos, and again a realm must start with gno.land/r/. I think this will be mitigated by reading the pkg path in the gno.mod, and @harry-hov already has something in the oven for that (see conversation).

So the plan for this PR is to wait for #682, then use the pkgPath found in gno.mod, which should avoid the need for stripping the ./examples prefix.

[anarcher]

On the master branch, the following command works fine, but on this PR I get the following error.
(I built a fresh gno with make install. )

$ gno test -verbose ./examples/gno.land/r/demo/tests -run TestPrevRealm
=== RUN   TestAssertOriginCall
panic: cannot modify external-realm or non-realm object

goroutine 1 [running]:
github.com/gnolang/gno/gnovm/pkg/gnolang.(*Realm).DidUpdate(0x140002c6460, {0x1011da070, 0x1400041c000}, {0x0, 0x0}, {0x0, 0x0})
        /Users/anarch/go/blockchain/src/gno/gnovm/pkg/gnolang/realm.go:148 +0x270
github.com/gnolang/gno/gnovm/pkg/gnolang.PointerValue.Assign2({0x14008e094a0, {0x1011c8ea0, 0x1400041c000}, 0x14, 0x0}, 0x3?, {0x1011dac38, 0x14000166120}, 0x140002c6460, {{0x1011cee88, ...}, ...}, ...)
        /Users/anarch/go/blockchain/src/gno/gnovm/pkg/gnolang/values.go:281 +0x530
github.com/gnolang/gno/gnovm/pkg/gnolang.(*Machine).doOpAssign(0x14000122b40)
        /Users/anarch/go/blockchain/src/gno/gnovm/pkg/gnolang/op_assign.go:44 +0xd0
github.com/gnolang/gno/gnovm/pkg/gnolang.(*Machine).Run(0x14000122b40)
        /Users/anarch/go/blockchain/src/gno/gnovm/pkg/gnolang/machine.go:1251 +0xed4

I'm not sure, but could these two IDs be different?

	if po.GetObjectID().PkgID != rlm.ID {
		panic("cannot modify external-realm or non-realm object")
	}

[tbruyelle]

@anarcher wow thank you for pointing that, it appears this only happens when the -run flag is provided !

So the fix for that is the following :

diff --git a/gnovm/stdlibs/testing/match.gno b/gnovm/stdlibs/testing/match.gno
index 5b7f509d..13a7edd4 100644
--- a/gnovm/stdlibs/testing/match.gno
+++ b/gnovm/stdlibs/testing/match.gno
@@ -166,13 +166,13 @@ func isSpace(r rune) bool {
 	return false
 }
 
-var (
-	matchPat string
-	matchRe  *regexp.Regexp
-)
-
 // based on testing/internal/testdeps.TestDeps.MatchString.
 func matchString(pat, str string) (result bool, err error) {
+	var (
+		matchPat string
+		matchRe  *regexp.Regexp
+	)
+
 	if matchRe == nil || matchPat != pat {
 		matchPat = pat
 		matchRe, err = regexp.Compile(matchPat)

(this patch should be reworked, there's no reason to turn the global var into local var like that, but that's for the demo)

The funny thing behind this fix is :

• since this PR turned the test into a realm
• any global variables used in this realm or in any package dependencies are aimed to be stored at the end of the transaction
• when -run is provided, it uses testing/match.go which updates 2 global variables for caching
• those 2 global variables doesn't share the same package ID as the test realm, so the VM panics.

You can have the same behavior by calling a realm that calls a package that mutates its own global variables.

So that means with this PR, we can no longer have global variables in testing or in any imported packages in a test. Do you think that's still acceptable ? (cc @moul)

[moul]

To keep track of the previous discussion:

When persisting the snapshot of the finished state, exclude globals from _test.gno files in realms' store. This offers flexibility in _test.gno files while maintaining stricter controls in realms.

[tbruyelle]

@moul I reverted the last change so now when the tx is finalized, the realm's global variables that come from *_test.gno files are ignored.

That works for test files, and for the template file generated at runtime (because it has been renamed from testmain.go to main_test.gno), but unfortunately that doesn't work for testing/match.gno because the file name doesn't end with *_test.gno.

I didn't find a way to determine that testing/match.gno is a dependency of a *_test.gno file, and so should inherit its property of being ignored by the realm persistency code. And because it could potentially be a dependency of multiple other files (with some that aren't *_test.gno files), I believe this is not straightforward to do so.

On the other hand, I found some other gno code imported from the stdlib, where the global variables are commented with the label masterRNG deleted to make package pure. So maybe we should simply remove/comment those global variables from testing, and make the package pure. WDYT?

[moul]

Thinking out loud, what about ignoring files importing « testing »?

[tbruyelle]

Thinking out loud, what about ignoring files importing « testing »?

I believe no production code would import testing, so that could work. But that doesn't solve the problem, we also need to ignore testing itself.

[tbruyelle]

@moul I was able to ignore the testing package only in test environement, by introducing a condition in the TestStore. Now with the other condition related to _test files, I have something that works, even if it looks like an ugly hack :/

[moul]

yes, and later, gnomod.ParseAt can also support new ways to guess a module when there is no gno.mod file.

[moul]

Suggested change

	if strings.HasSuffix(source.Location.File, "_test.gno") {
	func (v Val) InTestFile() bool {return strings.HasSuffix(source.Location.File, "_test.gno")}
	if val.InTestFile()) {

can be done later

[piux2]

We don't have to change it now. I created an issue so that we can track it for later discussion.

Dose this mean if someone deployed a realm package with a source file named xyz_test.gno it will be ignored by the VM? If that is the case, it means we should include this file naming convention to the VM specs.

Also If we do not allow _test.gno files in the realm package, we should guarded it before we parse the file and load it to VM, instead of at the persistent stage.

If this purely serves the purpose for off chain realm testing, would sticking with realm file test be a better option?