Repository navigation
Conversation
rekmarks
commented
Dec 2, 2020
rekmarks
commented
Contributor
- Minor version bump
- Update changelog
Member
|
This release is still missing a tag and a GitHub release page |
Mrtenz
pushed a commit
to Mrtenz/core
that referenced
this pull request
Jun 17, 2026
Naz-Ovh
pushed a commit
to 0x-fork/metamask-core
that referenced
this pull request
Sep 13, 2026
Stacked on MetaMask#318. | Dep | From | To | | --- | --- | --- | | `jest` | `^29.2.2` | `^30.4.2` | | `@types/jest` | `^28.1.7` | `^30.0.0` | | `ts-jest` | `^29.0.3` | `^29.4.11` | `ts-jest` has to move with it: `29.0.3` peer requires `jest@^29`, so Jest 30 cannot land alone. `29.4.11` is what core pairs with Jest 30. Jest 30 pulls in `@parcel/watcher` via `jest-haste-map`, which has an install script, so lavamoat needs it declared. Set to `false` like core does with its equivalent native packages, so the script does not run and Jest falls back to its JS file crawler. That only affects `--watch`. ## Fixing a flaky timing test `areUint8ArraysEqual` has an assertion that comparing arrays differing at the first byte takes about as long as comparing arrays differing at the last, guarding the constant time property. It took a **single** timing sample per side, so one garbage collection pause or descheduled CPU slice skewed the ratio. It was already failing intermittently before this branch (1.31 against a bound of 1.1). Under Jest 30 it reached 1.60 and failed on every Node version. The function is genuinely constant time: it walks the full length accumulating with `|=` and never returns early. So the true ratio is exactly 1.0 and everything above is measurement noise. Taking the **fastest** of seven samples per side (noise is strictly additive, so the quickest run is closest to real cost) gets the ratio to 1.0002 locally. CI still measured ~1.14 though, so the runners carry a systematic component no estimator removes. The bound was false precision anyway: what this test guards against is the comparison regaining an early return, and if that regressed `early` would stop after one byte while `late` walked all 100,000. Reintroducing an early return locally produces a ratio of **556**. So the bound moves from `1.1` to `2`, keeping 278x margin against the real failure mode while tolerating the ~1.14 shared runners produce. ## And a coverage fix Jest 30 ships a newer istanbul that accounts for ignore directives differently. In `coercers.ts` the `catch` blocks carry `/* istanbul ignore next */` on the trailing `throw error`. That still excludes the statement, but no longer excludes the implicit else path of the `if (error instanceof StructError)` above it, so branch coverage fell to 99.12% and the 100% threshold failed. Adds `/* istanbul ignore else */` to the three `if`s whose else path is not exercised. `createNumber` is deliberately left alone: its else path is covered by a real test, and ignoring it there would hide a future regression. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Dev-only dependency and test/coverage annotation changes; no published library API or runtime logic changes. > > **Overview** > Upgrades the test stack to **Jest 30** (`jest`, `@types/jest`, and `ts-jest` so peer deps stay aligned), with a large lockfile refresh. **Lavamoat** now blocks the install script on Jest’s transitive `@parcel/watcher` (same pattern as other native deps), so watch mode falls back to the JS file crawler. > > **`bytes.test.ts`** stabilizes the constant-time timing check for `areUint8ArraysEqual`: seven trials per side, min-of-samples timing, and a ratio cap relaxed from **1.1 → 2** so shared CI noise (~1.14) does not flake while still catching a reintroduced early-return (which would spike the ratio into the hundreds). > > **`coercers.ts`** adds `istanbul ignore else` on three coercion `catch` paths so Jest 30’s istanbul still hits the **100% branch coverage** threshold; runtime behavior is unchanged. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 2bab777add2c84d1ef5783734efbbfd42655f042. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.