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

Discussion: check _test.gno in realm.go #1032

Open
piux2 opened this issue Aug 8, 2023 · 4 comments
Open

Discussion: check _test.gno in realm.go #1032

piux2 opened this issue Aug 8, 2023 · 4 comments

Comments

@piux2
Copy link
Contributor

piux2 commented Aug 8, 2023

          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?

Originally posted by @piux2 in #896 (comment)

@piux2 piux2 changed the title check _test.gno in realm.go Discussion: check _test.gno in realm.go Aug 8, 2023
@tbruyelle
Copy link
Contributor

tbruyelle commented Aug 8, 2023

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.

I think this isn't something new if you look at machine.RunMemPackage(), *_test.gno and *_filetest.gno are already filtered out.

@piux2
Copy link
Contributor Author

piux2 commented Aug 31, 2023

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.

I think this isn't something new if you look at machine.RunMemPackage(), *_test.gno and *_filetest.gno are already filtered out.

Here is my thought process.

A. node.ParseMemPackage() is used for on chain. It does not load _test.gno in the memory package

https://github.com/gnolang/gno/pull/896/files#r1283457569, if we have to filtered the _test.gno file at this point, that means somewhere else has an serious issue, since no _test.gno should be loaded in the MemPackage on Chain at the first place.

B. node.ParseMemPackageTests() is used for off chain testing. That loads _test.gno in memory package. The entire method is copied to cmd/gno/test.go to load testing files in memory packages.

At later stage both using the same VM code vm/realm.go. We should not change vm/realm.go because we want to make certain _test.gno testing case pass for B. Instead, it should be fixed in cmd/gno/test.go, or somewhere else, but not in vm/realm.go

@tbruyelle
Copy link
Contributor

These are very good points, I really tried to not have this _test.gno check in realm.go, but failed.

@moul
Copy link
Member

moul commented Sep 5, 2023

  1. We should keep letting users upload "test scripts" with their code on gno.land. It makes things clear and helps with forking.
  2. Right now, when contracts run on-chain, we skip the test files. Let's keep doing that.
  3. Maybe we can have a service or experts who check these contracts for safety. We could then show badges on the trusted ones.

So, if I've got it right, we don't need to change anything for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🔵 Not Needed for Launch
Development

No branches or pull requests

3 participants