Repository navigation
test(cli): the shared-name install-local jobs test lets each install run to the setup deadline, not a fixed 120 s - #25547
Merged
Conversation
…up deadline, not a fixed 120 s The three `os package install` children each had their own fixed 120 s SIGKILL, set on an idle machine; under a full `pnpm test` on a 4 vCPU box the first install outlived it and six assertions went red against correct code. Each install now runs until the beforeAll's own deadline (its budget less a 10 s reserve), in its own process group, and a kill rejects naming the install, its wall time and the budget instead of surfacing as a bare exit null. Claude-Session: https://claude.ai/code/session_01CxBKycaiUpA1yGx1XyoGqX Co-authored-by: Claude <noreply@anthropic.com>
Contributor
📓 Docs Drift CheckNothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs. What this run could not see
Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
Contributor
Author
席位核对ACCEPT ·
Generated by Claude Code |
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
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.
Requested by Sales · project thread
Fixes #25499
Clause-②: no
Metadata-protocol: none
packages/cli/test/package-install-local-jobs-shared-name.integration.test.tsgave each of its threeos package installchildren a fixed 120 s SIGKILL. That number was set on an idle machine. Under a fullpnpm teston a 4 vCPU box, the first install outlived it, and 6 of 9 assertions went red against correct code. ThebeforeAllstill had more than 390 s of its 600 s budget left.Test-only.
skip-changeset: the file is underpackages/cli/test/, which@objectstack/cli'sfiles[]does not ship.What changed (one file)
beforeAlltakes one deadline when it starts:SETUP_BUDGET_MS(the hook's existing timeout,2 * BOOT_TIMEOUT_MS + 12 * RUN_WAIT_MS, unchanged at 600 s) less a 10 sKILL_REPORT_RESERVE_MS. Each install may run until that deadline. On a slow box, the installs use the time the boots and run waits did not need. No number went up.detached, so the node process thattsxstarts is killed too, and that process holds the pipesclosewaits on. The promise then rejects, naming the install, its wall time and the budget, with both transcripts. So the red is that message, not a bareexit nulland not vitest'sHook timed out, which names no step.InstallRunnow carrieselapsedMs, and the firstitputs it in its failure message.Why there is no constant and no load factor:
run-dev-unbuilt-workspace.e2e.test.tsin this package records two ceilings that failed this way, a fixed one and one scaled from a calibration. A bigger constant fails again on a busier box. The shared spawner helpers intest/helpers/serve-process.tsexport no install budget and no load-aware ceiling (their only child budget isrunServe's inline 180 s default). So this change adds no export to them, and the seven siblingpackage-install-local-*files keep their own copy ofpackageInstall()(see Acceptance notes).Verification (head
02097ac15b, 4 vCPU container, every run underscripts/pm/os-verify-lock.sh)Test Files 1 passed,Tests 9 passed, 87.10 sintegrationtier,VITEST_MAX_WORKERS=3(same mutation)Test Files 171 passed, 1 skipped (172),Tests 1469 passed, 9 skipped, 2498.96 s./dist/no-such-artifact.json)all three packages installfails withalpha (9838 ms): … Cannot read artifact: ENOENT, exit 1 against expected 0. 9/9 red, no hook timeoutdeadline = Date.now() + 1)beforeAllrejects withos package install in …/shared_alpha was SIGKILLed after 7 ms: the setup budget (600000 ms for the whole beforeAll, less a 10000 ms reserve) was spent.The file fails and its 9 tests are skipped. NoHook timed outThe mutations went through
scripts/ablation-replace.mjs, in WRAP mode, with an EXIT/INT/TERM trap. After the last leg the file's blob equals HEAD (bc1e7d9b3a…) andgit diff HEADis empty. My first attempt at the logging mutation did nothing: its anchor was a substring of its replacement, so the tool refused it (anchor count 1 -> 1). It was re-run with an anchor that is not.NOT MEASURED: a full
pnpm teston this box. The loaded reading above is the cli integration tier at three workers, not the turbo-wide run from the issue. The installs there took 14-16 s, against more than 120 s in the issue's run. So this box's load never came near the old ceiling, and the fix's tolerance of a busier box is argued from the design, not measured.The local gates are listed in the report on #25499.
pnpm lintand the clitypecheck/unittier were run here.Acceptance notes
setTimeout(() => child.kill('SIGKILL'), 120_000)install ceiling is in six sibling files:package-install-local-{jobs,jobs-pull,hooks,handlers,boot-steps,uninstall-cleanups}.integration.test.ts. Not changed here, because the claim's file surface is this one file. No red is on record for them. carrier: none.bootStart's fixedBOOT_TIMEOUT_MSstill does not read the shared deadline. If installs used most of the budget, a hung second boot would show asHook timed outinstead of a named error. Not observed. carrier: none.Generated by Claude Code
🤖 Generated with Claude Code
https://claude.ai/code/session_01CxBKycaiUpA1yGx1XyoGqX