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

[GnoVM] Reset machine context & realm state after each Test function #1982

Closed
leohhhn opened this issue Apr 25, 2024 · 4 comments · Fixed by #2975
Closed

[GnoVM] Reset machine context & realm state after each Test function #1982

leohhhn opened this issue Apr 25, 2024 · 4 comments · Fixed by #2975
Assignees
Labels
🐞 bug Something isn't working

Comments

@leohhhn
Copy link
Contributor

leohhhn commented Apr 25, 2024

Description

As title says, this is one of the issues with gno test. Currently, the context and realm state between test functions is kept instead of being discarded, making testing things difficult.

If we want to keep close to Go behavior, we could implement a Gno-specific function that would allow us to wipe the state at the top of each test.

@harry-hov
Copy link
Contributor

harry-hov commented May 7, 2024

@leohhhn why do you want to reset the context after each test function? Current behaviour seems to be a valid Go behaviour.

If you try to run the following code in Go:

// counter.go
package counter

var count int

func Increment() {
	count++
}
// counter_test.go
package counter

import "testing"

func TestIncrement1(t *testing.T) {
	Increment()
	if count != 1 {
		t.Fail()
	}
}

func TestIncrement2(t *testing.T) {
	Increment()
	if count != 1 {
		t.Fail()
	}
}

TestIncrement2 will fail. As count has been already incremented by TestIncrement1

@leohhhn
Copy link
Contributor Author

leohhhn commented May 21, 2024

In blockchain context, it's very useful to have a clean slate to test things on. To keep expected behavior with Go, I suggest we add a ResetMachineState function or similar, that you can call at the top of the test function and have it reset the machine running it.

cc @thehowl @zivkovicmilos WDYT?

@thehowl
Copy link
Member

thehowl commented Jun 28, 2024

In #2425, this is already somewhat improved for what concerns the test context; ie. avoiding TestSetXXX calls and instead have a closure in which a given context is applied.

However, one angle which I think is important and is not (yet) tackled by this issue is resetting the state of realms. This is a feature that Go's testing doesn't have (ie. if you change a global variable in a test, it stays changed), but it makes sense for us. (Guaranteeing that tests have a clean state also opens up the doors to eventually parallelizing test execution, but I digress :P)

I think that one will be trickier, but also important to implement, and partly what Hariom was talking about in his comment.

@leohhhn
Copy link
Contributor Author

leohhhn commented Jun 28, 2024

resetting the state of realm

This is basically what I wanted to achieve with the issue 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working
Projects
Development

Successfully merging a pull request may close this issue.

7 participants