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): test execution refactor + speedup #3119

Open
wants to merge 47 commits into
base: master
Choose a base branch
from

Conversation

thehowl
Copy link
Member

@thehowl thehowl commented Nov 13, 2024

This PR tackles a large amount of improvements in our current internal testing systems for the gnovm, as well as those for gno test. The primary target is the execution of filetests; however many improvements have been implemented to remove dead or duplicate code, and bring over some of the improvements that have been implemented in tests to filetests, when they come at little added "cost".

The biggest headline concerns the execution of filetests. I wrote the specific improvements undertaken in a blog post on "diary of a gnome", but here's a side-by-side comparison of the execution in this PR (left) and the execution on master (right).

filetests

Impact

  • Test context (tests and filetests)
    • Tests and filetests now share the same test.Store when running. Coupled with test.LoadImports, it allows us to "eagerly load" imports ahead of the test execution, and save it in the store. This is the primary performance improvement of this PR, as all pure packages can be run and preprocessed only once, whereas before it was once per package, and once per filetest. (More info in the blog post.)
      • One of the consequences of this is that package initialization happens outside of the test; so a filetest can no longer check the output of a println used in package initialization.
    • The default user no longer has 200 gnot by default. There are two mechanisms that make this unnecessary: // SEND:, and std.TestIssueCoin. Some test balances had to be updated.
  • Filetests
    • Running filetests in gno test -v now also prints their output.
    • Realm tests are no longer executed in main.gno, but in a filename following the name of the filetest; this changed some // Realm: directives.
    • The sytem to read directives and update them in the filetests has been re-written, and should now be more resilient and with fewer "exceptions". This means that leading and trailing whitespace in output now is correctly considered as output, leading to the addition of many empty // in the tests.
      • // Error: directives are now treated the same as everything else; and are updated if the -update-golden-tests flag is passed.
    • Removed the imports metric from the runtime metrics, as it's now no longer straightforward to calculate (given that imports are loaded "eagerly").
    • Removed support for different "modes" of importing stdlibs; further removing support for gonative ([chore] Remove NativeValue and NativeType from the VM entirely #1361). The remaining gonative libraries are os, fmt, encoding/json, internal/os_test and math/big. This removes the -with-native-fallback flag from gno test.
      • Consequently, filetests ending with _native.gno have largely been removed, and those ending with _stdlibs.go have had their suffix removed.
      • Some files testing gonative types and functions which were only used in a small subset of these tests, have been removed.
  • Tests
    • gno test, for testing packages, created a main_test.gno file that is then called directly from the machine on each test. This creates two identifiers, tests and runtest, which could come into conflict with those defined by the tests themselves. We now call testing.RunTest directly, without an intermediary.
    • Exports in the normal tests (ie. defined in pkg) are now visible in the tests of pkg_test. This is most useful for some standard library tests, where there often is an export_test.go with the sole scope of exporting some internal functions and methods for testing in the "real" testing file.
  • gno lint, and other occasions where we use issueFromError (like when parsing errors in gno test), will now also print the column of the error if given.

Summary of internal changes

  • pkg/gnolang
    • TestFiles is the new function running the filetests.
    • eval_test.go has been removed, moving some of its improvements over to TestFiles, and reducing the scope of the corresponding tests in debugger.go, to what it's actually supposed to test for.
    • As a consequence of removing all of type mappings in imports.go, I removed SetStrictGo2GnoMapping in favour of always being "strict", and not allowing defining native types of defined types any more.
      • The tests in gonative_test where primarily related to this, so I removed them. Similarly for preprocess_test.
    • TestFunc / TestMemPackage have been removed as redundant, including some of their features in cmd/go/test.go (like supporting exporting symbols in _test.gno files)
    • The Store no longer has ClearCache and SetStrictGo2GnoMapping; ClearCache now has a better substitute in BeginTransaction.
  • the testing stdlib no longer caches matchPat and matchString as pure packages should not be able to modify their values during execution, and as a result of other changes this was failing.
  • The tests/ directory has been removed / moved to gnovm/pkg/test. This directory should eventually contain the internal code for gno test as well; but for now I wanted to clean tests to ensure it is a directory just for integration tests, rather than code and testing systems.
    • TestMachine and TestStore have been renamed to Machine and Store, to avoid redundancy with the test package name.
  • Removed plenty instructions in gnovm/Makefile as now most tests are just in pkg/gnolang.
  • I removed MsgContext.Msg as unused. It can be re-added later if necessary.
  • In the CI, tests are now run with -short, at least until we figure out how to make the VM efficient enough to run these tests. Aside from some filetests, this now excludes some stdlibs: bytes, strconv, regexp/syntax. I accept other proposals that could make us have fast tests on PR's, while still testing (mostly) everything.

Open Source Saturday

@thehowl thehowl self-assigned this Nov 13, 2024
@github-actions github-actions bot added 🧾 package/realm Tag used for new Realms or Packages. 📦 🤖 gnovm Issues or PRs gnovm related 📦 ⛰️ gno.land Issues or PRs gno.land package related labels Nov 13, 2024
Copy link

codecov bot commented Nov 13, 2024

Codecov Report

Attention: Patch coverage is 77.07317% with 188 lines in your changes missing coverage. Please review.

Project coverage is 63.59%. Comparing base (b3800b7) to head (f1cfc8e).

Files with missing lines Patch % Lines
gnovm/pkg/test/imports.go 59.78% 67 Missing and 7 partials ⚠️
gnovm/pkg/test/filetest.go 77.85% 49 Missing and 11 partials ⚠️
gnovm/pkg/test/test.go 82.57% 38 Missing and 12 partials ⚠️
gnovm/pkg/gnolang/gonative.go 0.00% 3 Missing ⚠️
gnovm/pkg/gnolang/store.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3119      +/-   ##
==========================================
- Coverage   63.80%   63.59%   -0.21%     
==========================================
  Files         549      553       +4     
  Lines       78833    78928      +95     
==========================================
- Hits        50301    50196     -105     
- Misses      25143    25361     +218     
+ Partials     3389     3371      -18     
Flag Coverage Δ
contribs/gnodev 60.54% <ø> (-0.63%) ⬇️
contribs/gnofaucet 14.82% <ø> (ø)
gno.land 73.67% <ø> (-0.03%) ⬇️
gnovm 67.27% <77.07%> (-0.68%) ⬇️
misc/genstd 79.72% <ø> (ø)
tm2 62.41% <ø> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@thehowl thehowl added the in focus Core team is prioritizing this work label Nov 15, 2024
@thehowl thehowl modified the milestone: 🚀 Mainnet launch Nov 15, 2024
@thehowl thehowl changed the title feat: filetest execution refactor feat(gnovm): test execution refactor + speedup Nov 19, 2024
@thehowl thehowl marked this pull request as ready for review November 19, 2024 18:30
@thehowl
Copy link
Member Author

thehowl commented Nov 19, 2024

Marking this as ready for review. Yes, there are a lot of changed files, but I'm thinking we can still do this in one go rather than doing multiple PRs because there are actually very few "tricky" changes (ie., those involving non-test code in pkg/gnolang are very minimal), and the benefits on our mental sanity are quite large (given that this reduces greatly the execution of both examples and tests/files by several minutes).

As usual for my large PRs, I tried updating OP to give it as much information and guidance on how to review the PR. 🥂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in focus Core team is prioritizing this work 📦 ⛰️ gno.land Issues or PRs gno.land package related 📦 🤖 gnovm Issues or PRs gnovm related 🧾 package/realm Tag used for new Realms or Packages.
Projects
1 participant