chore: improve vitest output #6786
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## unstable #6786 +/- ##
============================================
+ Coverage 61.88% 62.20% +0.32%
============================================
Files 562 571 +9
Lines 59331 60015 +684
Branches 1916 1976 +60
============================================
+ Hits 36715 37335 +620
- Misses 22573 22637 +64
Partials 43 43 |
nflaig
reviewed
May 15, 2024
vitest.base.unit.config.ts
Outdated
| @@ -20,7 +20,7 @@ export default defineConfig({ | |||
| ], | |||
| reporters: process.env.GITHUB_ACTIONS | |||
| ? ["verbose", "hanging-process", "github-actions"] | |||
| : ["verbose", "hanging-process"], | |||
| : [process.env.TEST_COMPACT_OUTPUT === "true" ? "basic" : "verbose", "hanging-process"], | |||
There was a problem hiding this comment.
we need a more consistent approach on how we check env variables, something we check strictly for "true" and other times we just check if the env variable is set
There was a problem hiding this comment.
Makes sense. Any preference?
There was a problem hiding this comment.
I have no hard opinion about it but what ever we do should do consistently. And if that rule can be enforced by linter then let it be individual choice.
nazarhussain
requested changes
May 24, 2024
nazarhussain
requested changes
May 24, 2024
vitest.base.unit.config.ts
Outdated
| @@ -42,5 +42,6 @@ export default defineConfig({ | |||
| ], | |||
| }, | |||
| diff: process.env.TEST_COMPACT_DIFF ? path.join(import.meta.dirname, "./scripts/vitest/vitest.diff.ts") : undefined, | |||
| onConsoleLog: () => process.env.TEST_QUIET_CONSOLE !== "true", | |||
There was a problem hiding this comment.
This will be expensive check, as for every log message this function will be called. Not fully against it, but we should document somewhere that it's a expensive.
There was a problem hiding this comment.
It should be neglectable compared to the cost of logging a line to the console. I haven't notices any slowdown.
There was a problem hiding this comment.
shouldn't be checking against "true" here
Co-authored-by: Nazar Hussain <nazarhussain@gmail.com>
nflaig
reviewed
May 24, 2024
| | ENV variable | Effect | Impact | | ||
| | ------------------- | ------ | ------------------------------------------------------------------------------------------------------------------------- | | ||
| | TEST_COMPACT_DIFF | All | Will strip down the object difference rendered during test failures. Very useful for large object matching. | | ||
| | TEST_QUIET_CONSOLE | All | Will strip down console output. Reduce console flickering. | |
There was a problem hiding this comment.
why do you mean by "strip down" here? doesn't this disable console logging if set?
There was a problem hiding this comment.
There's still the vitest output, this disables lodestar console output. Open to description clarifications.
vitest.base.unit.config.ts
Outdated
| @@ -42,5 +42,6 @@ export default defineConfig({ | |||
| ], | |||
| }, | |||
| diff: process.env.TEST_COMPACT_DIFF ? path.join(import.meta.dirname, "./scripts/vitest/vitest.diff.ts") : undefined, | |||
| onConsoleLog: () => process.env.TEST_QUIET_CONSOLE !== "true", | |||
There was a problem hiding this comment.
shouldn't be checking against "true" here
* chore: upgrade classic-level * chore: address comments
…ainSafe#6727) * Restructure the assertions * Add an inspector to run the logic to detect providers * Update web3 provdier logic to use inspetor * Fix the types for proxy * Make the default type for mutation * Rename elrpc to elrpcprovider * Apply suggestions from code review Co-authored-by: Julien <jeluard@users.noreply.github.com> * Fix build error * Update the readme doc * Apply suggestions from code review Co-authored-by: Julien <jeluard@users.noreply.github.com> * Fix the docs linting * Add missing words --------- Co-authored-by: Julien <jeluard@users.noreply.github.com>
* docs: added debugging section * chore: spell checks * chore: added extra docs * chore: address comments * chore: added extra configuration * chore: updated docs * chore: added extra configuration * chore: fix lint * chore: fix typos * chore: .gitignore * chore: address comments * chore: address comments
* Rename simulation test to crucible * Rename SimulationEnvironment to Simulation * Use consistent function names * Update readme * Rename interfaces for consistent pattern * Fix lint error
* Add grandine * add grandine to wordlist
* chore: refactor sleep(0) usage * chore: refactor setTimeout * chore: address comments * chore: cleanup * Apply suggestions from code review --------- Co-authored-by: Cayman <caymannava@gmail.com>
…afe#6814) * Check get_proposer_head * Fix enum typo * lint
* chore: do not rely on leveldown * chore: replace level with classic-level
* chore: added docker support for osx * chore: address comments * chore: address comments * Update docker-compose.yml Co-authored-by: Nico Flaig <nflaig@protonmail.com> * chore: address comments --------- Co-authored-by: Nico Flaig <nflaig@protonmail.com>
…inSafe#6817) Fix vitest performance with spec tests in place
* feat: use @chainsafe/blst directly * chore: update to blst@1.0.1 * refactor: remove randomBytesNonZero and user blst exported version * chore: update blst references * test: catch invalid deserialization in spec tests and return false * feat: create signatureFromBytes and signatureFromBytesNoCheck in utils package * feat: implement signatureFromBytes from utils package * feat: implement signatureFromBytes everywhere * fix: light-client empty module for blst --------- Co-authored-by: matthewkeil <me@matthewkeil.com>
* fix: avoid Buffer.from copies * chore: simplify shuffling * fix: use subarray instead of slice in shuffling * chore: remove unnecessary devDependencies * chore: rely on fastify 4.x behavior * chore: avoid copy in verifyMerkleBranch * use toBase64 * relax assertions in shuffle function * Update packages/state-transition/src/util/shuffle.ts Co-authored-by: twoeths <tuyen@chainsafe.io> --------- Co-authored-by: twoeths <tuyen@chainsafe.io>
* Upgrade node js version to 22 * Update node types * Revert action config * Add package from git hash * Fix the build error * Update the docs for node-22 * Update docker version for Nodejs to 22 * Update package.json * Update package.json Co-authored-by: Nico Flaig <nflaig@protonmail.com> * Update readme docs * Add word in dictionary * Add word in dictionary --------- Co-authored-by: Cayman <caymannava@gmail.com> Co-authored-by: Nico Flaig <nflaig@protonmail.com>
* test: increase timeout of keystore cache tests * Increase hook timeout * Consistent number formatting
|
Replaced by #6850 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Current tests output can be quite verbose, and display interleaved
consoleoutput.Description
Default behavior is unchanged. 2 env variables allow to tweak
vitestoutput:TEST_COMPACT_OUTPUTto produce a basic tests summaryTEST_QUIET_CONSOLEto strip console output from tests summarye.g.
TEST_QUIET_CONSOLE=true TEST_COMPACT_OUTPUT=true yarn test:spec:mainnetwill produce the following output: