-
-
Notifications
You must be signed in to change notification settings - Fork 287
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
test(prover): improve test coverage #5648
Conversation
Performance Report✔️ no performance regression detected Full benchmark results
|
5778399
to
e86bfe1
Compare
696782d
to
a5080c4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactoring looks pretty good but this PR is really hard to review, git diff is lost for a lot of files.
Might want to split this up into two PRs
- one that refactors existing tests and extracts utils to @lodestar/test-util
- and a 2nd PR that adds prover tests
Would suggest a 2nd review from someone else
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, keeping this PR open to give others the opportunity for a final review
It looks like running a custom devnet with proxy is behaving differently than the mainnet. Making this PR to draft temporarily to fix it. |
export async function spawnCliCommand( | ||
command: string, | ||
args: string[], | ||
opts?: SpawnChildProcessOptions & {runWith?: "node" | "ts-node"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can't run with ts-node anymore, we have to do node --loader ts-node/esm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was running with ts-node
, though now I have changed the implementation to use node --loader ts-node/esm
.
🎉 This PR is included in v1.10.0 🎉 |
Motivation
Improve the test coverage for the prover package.
Description
Closes #5647
Run all tests.