VM test runner rewrite #4088
VM test runner rewrite #4088
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
packages/tx/test/types.ts
Outdated
| pre: Record<string, AccountState> | ||
| post: Partial<Record<ForkName, PostReceipt[]>> | ||
| transaction: BlobTransaction | ||
| } |
There was a problem hiding this comment.
These types are added with the following commit message "Add types for vm state test data", so with "vm" in the text, while the types here are in "tx", so there seems to be something off.
There was a problem hiding this comment.
Yes, good point, they would have had to have been moved over to the vm package along with making getTests and other helpers in testLoader.ts available there too for use with the new runners. But we could leave that as a potential followup.
There was a problem hiding this comment.
Side note if we/you tackle later: these tx specific things should never make it over, there should definitely no unified runner, only a very very lightweight unified 20 line file loader (as outlined in the chat) helper function, which is completely independent of whatever VM or TX code and can therefore be placed in a generic place (testdata package).
am1r021
force-pushed
the
tape-to-vitest-refactor-vm-testrunners
branch
from
7592482 to
e39ed30
Compare
|
@holgerd77 this seems like a good candidate for wrapping up and merging in. Wdyt? Could look ino it and see if we can get it done, the |
|
Yes, that's the one I was talking about. |
ScottyPoi
force-pushed
the
tape-to-vitest-refactor-vm-testrunners
branch
from
6900cc7 to
18dbe45
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR modernizes the VM test runner infrastructure by migrating from tape to vitest, introducing environment variable-based configuration, and streamlining test execution. The changes enable running state tests through vitest while maintaining backward compatibility with tape-based tests.
- Migration from tape testing framework to vitest for state tests
- Implementation of environment variable-based configuration (VITE_* prefixed variables)
- Addition of dual test framework support (tape and Chai assertions) in test runners
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/vm/vitest.config.coverage.ts | Excludes stateRunner.spec.ts from coverage to avoid running integration tests during coverage collection |
| packages/vm/vitest.config.browser.mts | Excludes stateRunner.spec.ts from browser tests since test runners are CI-only |
| packages/vm/test/tester/stateRunner.spec.ts | New vitest-based test runner that reads environment variables and executes state tests with test count verification |
| packages/vm/test/tester/runners/GeneralStateTestsRunner.ts | Adds support for both tape and Chai assertions via type guards, removes console.log debugging, implements test count tracking |
| packages/vm/test/tester/config.ts | Updates getSkipTests parameter type to accept undefined for better type safety |
| packages/vm/package.json | Updates test scripts to use vitest with environment variables and pins vitest browser package versions |
| package.json | Changes vitest browser dependencies from caret ranges to exact versions |
| .github/workflows/vm-pr.yml | Updates CI workflow to use vitest directly with environment variables instead of npm scripts |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ScottyPoi
force-pushed
the
tape-to-vitest-refactor-vm-testrunners
branch
from
a11058d to
8b490b1
Compare
This reverts commit 5c06661.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
ScottyPoi
force-pushed
the
tape-to-vitest-refactor-vm-testrunners
branch
from
8b490b1 to
92c9767
Compare
holgerd77
left a comment
holgerd77
left a comment
There was a problem hiding this comment.
Thanks so much Scotty for bringing this work from Amir over the finish line! 🙏 I did some checks to gain some confidence that this is more or less doing what it should, and I think it mostly does.
That said: the whole test code is close to impossible to review, I did some test runs with the VSCode debugger but gave up cause even in a step-by-step following it is close to not possible to retrace what the code is actually doing and why.
We should keep this high-priority and throw this whole code away mid-term (let's say: next 3-4 months). And then start fresh with a 50-100 liner only containing what we need right now newly building up on execution-specs/execution-spec-tests.
Let's talk a bit about this on the call.
Will merge this in though, so that we can continue here!
This change aims to modernize the vm test runners by switching over from using tape to vitest, refining the test file reader, and dropping any unused features or parameters of the current test runners.