Repository navigation
test(mocactl): stop the large-tree push test timing out in CI - #642
Conversation
`the push list > splits a large tree into several Execs` carries a 120 s
timeout and has been hitting it in CI while passing locally. It is not
contention and it is not the push: measured per phase, planPush takes
0.6 s and pushTree 1.3 s, while the assertions take 19.6 s.
Nearly all of that is one line:
expect(readFileSync(join(ws, 'big.bin'))).toEqual(readFileSync(join(r, 'big.bin')))
`toEqual` walks two ~12 MiB Buffers as structures, element by element.
Timed on its own: 18688 ms, against 0 ms for `Buffer.equals`, which
asserts the same byte-for-byte identity. The `.length` check goes first
so a failure still reports something useful, since `equals` collapses to
a bare boolean.
Also replaces the test's data generation. `Buffer.from(Array.from({
length: n }, () => Math.floor(Math.random() * 256)))` builds an
n-element JS array before the Buffer: ~1 s of CPU and a 27-million-
element allocation for the ~27 MiB this test writes, against ~3 ms for
`randomFillSync` into a pre-allocated Buffer. Secondary to the `toEqual`
fix, but worth having on the one test in the package with that timeout.
The test still fails if a single byte of the pushed copy differs
(verified by flipping one), and the suite stays at 129 passing. The test
itself goes from 23967 ms to 2610 ms, a 9x reduction, which should take
it well clear of the CI budget.
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Aleksander Slominski <aslom@us.ibm.com>
ingpaolodettori-dev
left a comment
ingpaolodettori-dev
left a comment
There was a problem hiding this comment.
LGTM. Buffer.equals with the .length check first keeps the byte-for-byte identity assertion and its useful failure message, and randomFillSync removes the 27M-element intermediate array. The only change is to the test, and the 120 s timeout still applies. One nit is inline.
Author: aslom (MEMBER — maintainer)
Areas reviewed: TypeScript tests (mocactl)
Agent/IDE config (.claude/.vscode): none
Commits: 1 commit, all signed-off: yes
CI status: passing
| // two ~12 MiB buffers as structures, element by element, which measured 18.7 s against 0 ms | ||
| // here -- on its own more than enough to blow a CI budget. The `.length` check first so a | ||
| // failure still says something useful, since `equals` collapses to a bare true/false. | ||
| const pushed = readFileSync(join(ws, 'big.bin')); |
There was a problem hiding this comment.
nit: const pushed shadows the module-level pushed() helper (line 96), which every later test in this file calls. It's harmless here, but a rename (copy/got) avoids confusion. Optionally, give the assertion a message, e.g. expect(got.equals(original), 'pushed big.bin differs').toBe(true), so a failure says more than expected false to be true.
Fix for one of CI tests failing taking too long, for example https://github.com/rossoctl/moca/actions/runs/40533836790/job/122293591458?pr=620
the push list > splits a large tree into several Execscarries a 120 s timeout and has been hitting it in CI while passing locally. It is not contention and it is not the push: measured per phase, planPush takes 0.6 s and pushTree 1.3 s, while the assertions take 19.6 s.Nearly all of that is one line:
toEqualwalks two ~12 MiB Buffers as structures, element by element. Timed on its own: 18688 ms, against 0 ms forBuffer.equals, which asserts the same byte-for-byte identity. The.lengthcheck goes first so a failure still reports something useful, sinceequalscollapses to a bare boolean.Also replaces the test's data generation.
Buffer.from(Array.from({ length: n }, () => Math.floor(Math.random() * 256)))builds an n-element JS array before the Buffer: ~1 s of CPU and a 27-million- element allocation for the ~27 MiB this test writes, against ~3 ms forrandomFillSyncinto a pre-allocated Buffer. Secondary to thetoEqualfix, but worth having on the one test in the package with that timeout.The test still fails if a single byte of the pushed copy differs (verified by flipping one), and the suite stays at 129 passing. The test itself goes from 23967 ms to 2610 ms, a 9x reduction, which should take it well clear of the CI budget.
Assisted-By: Claude (Anthropic AI) noreply@anthropic.com