chore: bump Jest to v30 - #319
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning MetaMask internal reviewing guidelines:
Ignoring alerts on:
|
c9e655d to
d0b303d
Compare
d0b303d to
ffd8335
Compare
50dbb76 to
e70e07f
Compare
e70e07f to
29dd8bb
Compare
29dd8bb to
9c11b3e
Compare
The merge-base changed after approval.
9c11b3e to
325f083
Compare
325f083 to
7c6f95b
Compare
|
@SocketSecurity ignore npm/@babel/code-frame@7.29.7 |
7c6f95b to
9812c5c
Compare
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 the version core pairs with Jest 30. Jest 30 pulls in @parcel/watcher through jest-haste-map, which has an install script, so lavamoat needs it declared. Set to false like core does with its equivalent native packages, meaning the script does not run and Jest falls back to its JS file crawler. That only affects --watch. No source or config changes were needed. jest-it-up declares no jest peer and still reports coverage correctly.
areUint8ArraysEqual has a test asserting that comparing arrays which differ at the first byte takes about as long as comparing arrays which differ 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 in either loop skewed the whole ratio. It was already failing intermittently on CI before this branch, at a ratio of 1.31 against a bound of 1.1. Under Jest 30 it went to 1.60 and failed on every Node version, which is what forced the fix now. Now takes seven interleaved samples per side and compares the medians. The 1.1 bound is unchanged, so the property under test is exactly as strict as before; only the noise rejection improved. Passes 6 out of 6 locally where it previously failed roughly 1 in 4.
Jest 30 ships a newer istanbul, which accounts for the coverage ignore directives differently. In coercers.ts the `catch` blocks carry `/* istanbul ignore next */` on the trailing `throw error`. That still excludes the statement, but it 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 */` on 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. No change in what is actually tested. Branch coverage is back to 100%.
Taking the median of seven samples cut the ratio from 1.60 to 1.12, but the bound is 1.1, so CI still failed by a hair on the slower runners. areUint8ArraysEqual is genuinely constant time: it walks the full length and accumulates with |=, never returning early. The true ratio is therefore exactly 1.0 and everything above it is measurement noise. That noise is strictly additive, since scheduling, garbage collection and cache pressure only ever make a run slower. The fastest of several samples is consequently the best estimate of the real cost, which is the usual approach in benchmarking. Switching from median to minimum moves the observed ratio from 1.12 to between 1.0002 and 1.0040 across five local runs, roughly 25x headroom against the unchanged 1.1 bound. The test takes 0.57s.
The fastest-sample estimator gets the ratio to 1.0002 on a quiet machine but CI still measures around 1.14, so the runners carry a systematic component that no choice of estimator removes. Chasing it further is not worthwhile, because the bound was false precision to begin with. What this test guards against is the comparison regaining an early return on the first differing byte. If that regressed, `early` would stop after one byte while `late` still walked all 100,000, so the ratio lands in the hundreds. Verified by reintroducing an early return locally: the ratio came out at 556, against the new bound of 2. So a bound of 2 keeps 278x margin against the real failure mode while tolerating the ~1.14 that shared runners produce. The old 1.1 bound bought no extra sensitivity to that regression, only flakiness.
9812c5c to
2bab777
Compare
2 participants
Stacked on #318.
jest^29.2.2^30.4.2@types/jest^28.1.7^30.0.0ts-jest^29.0.3^29.4.11ts-jesthas to move with it:29.0.3peer requiresjest@^29, so Jest 30 cannot land alone.29.4.11is what core pairs with Jest 30.Jest 30 pulls in
@parcel/watcherviajest-haste-map, which has an install script, so lavamoat needs it declared. Set tofalselike 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
areUint8ArraysEqualhas 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
earlywould stop after one byte whilelatewalked all 100,000. Reintroducing an early return locally produces a ratio of 556.So the bound moves from
1.1to2, 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.tsthecatchblocks carry/* istanbul ignore next */on the trailingthrow error. That still excludes the statement, but no longer excludes the implicit else path of theif (error instanceof StructError)above it, so branch coverage fell to 99.12% and the 100% threshold failed.Adds
/* istanbul ignore else */to the threeifs whose else path is not exercised.createNumberis deliberately left alone: its else path is covered by a real test, and ignoring it there would hide a future regression.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, andts-jestso 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.tsstabilizes the constant-time timing check forareUint8ArraysEqual: 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.tsaddsistanbul ignore elseon three coercioncatchpaths so Jest 30’s istanbul still hits the 100% branch coverage threshold; runtime behavior is unchanged.Reviewed by Cursor Bugbot for commit 2bab777. Bugbot is set up for automated code reviews on this repo. Configure here.