mirror of
https://github.com/dtzp555-max/ocp.git
synced 2026-07-27 07:55:07 +00:00
fix: correct the PR body, kill a comment I rotted, and get the counts right
The review's blocker was not in the code — it was that my PR body still carried
both retracted claims VERBATIM while my commit message asserted they had been
"corrected in the PR body". They had only been corrected in a trailing comment.
In a PR whose entire thesis is claim accuracy, that is the finding that matters.
The body is rewritten. Both claims now appear only as struck-through
corrections with the true values:
- "ltDiag in EVERY assertion message" -> 8 call sites, two target tests only
- "39330+i was SHARED across a loop" -> it wasn't; i is 0/1/2, ports differ.
The real risk is cross-process, which the control arm's 246 EADDRINUSE shows.
Comment rot I introduced: test-features.mjs:1000 said "See ltAssertBoot below"
and no such function exists — grep returned exactly one hit, the comment itself.
Removed.
Counts corrected after re-measuring with the definition line excluded, which is
where my earlier numbers came from:
ltDiag call sites 7 -> 8
_ltRmRetry sites 10 -> 11
_ltRmRetry's catch now honours LT_DEBUG instead of being unconditionally
silent. It must still never throw — it runs in a finally, where a throw would
REPLACE the real assertion error and turn a flake into an apparent regression —
but an opt-in warn costs nothing and stops it swallowing genuine programming
errors too.
Recording the reviewer's control-arm numbers, since they are what actually
justifies this PR (full suite x 4 concurrent x 50 rounds, per arm):
main this branch
clean runs 42/200 200/200
#199 7 0
:1092 8 0
EADDRINUSE 246 0
ENOTEMPTY 1 0
#203 0 0
Tests: 457 passed, 0 failed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017gbqUZ8HfBZpjjbzQ85oH8
This commit is contained in:
+6
-2
@@ -997,7 +997,7 @@ function ltBoot(env, dir, nodeArgs = []) {
|
||||
// 'exit' fires when the process terminates, but its stdio pipes may still hold unread data —
|
||||
// 'close' is the one that guarantees both are drained. A test that terminates the child and
|
||||
// then asserts on buf.err/buf.out must wait for `closed`, not `exit != null`, or it can read
|
||||
// an empty buffer. See ltAssertBoot below.
|
||||
// an empty buffer.
|
||||
child.on("exit", (code, signal) => { buf.exit = code; buf.signal = signal; });
|
||||
child.on("close", () => { buf.closed = true; });
|
||||
// Without a listener, a spawn 'error' is re-thrown as an uncaught exception and takes down the
|
||||
@@ -1010,7 +1010,11 @@ function ltBoot(env, dir, nodeArgs = []) {
|
||||
// as an intermittent ENOTEMPTY (4/200 in review). Node's own retry loop handles the window.
|
||||
function _ltRmRetry(dir) {
|
||||
try { _ltRm(dir, { recursive: true, force: true, maxRetries: 5, retryDelay: 50 }); }
|
||||
catch { /* a leaked temp dir must never fail a test — the OS reaps it */ }
|
||||
catch (e) {
|
||||
// Never throw: this runs in a finally, so a throw here would REPLACE the real assertion
|
||||
// error and make a flake look like a regression. LT_DEBUG surfaces it without that risk.
|
||||
if (process.env.LT_DEBUG) console.warn(` [ltRmRetry] ${dir}: ${e.code || e.message}`);
|
||||
}
|
||||
}
|
||||
// Every ltBoot assertion failure should be self-diagnosing. The historical failure text was
|
||||
// `expected a local-tools FATAL, got: ` — an empty string, which says nothing about whether the
|
||||
|
||||
Reference in New Issue
Block a user