test: fix exit code validation in test runner fixture#792
Conversation
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Claude
AI
changed the title
- Normalize exitCode before evaluating success condition - Previously compared original result.exitCode (possibly undefined) to 0 - Now normalizes to 0 first, then uses normalized value for success check - Fixes issue where exit code 0 was incorrectly marked as failure Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
Smoke test (Codex)
|
|
Smoke Test Results Last 2 merged PRs:
✅ GitHub MCP Status: PASS
|
There was a problem hiding this comment.
Pull request overview
This PR adjusts the test runner fixture (AwfRunner) result shaping to ensure the success field is computed from the same normalized exit code value that is returned to tests, addressing failures where exitCode normalization and success could disagree.
Changes:
- Normalize
result.exitCodeinto a localexitCodevariable inrun()andrunWithSudo(). - Compute
successfrom the normalizedexitCoderather than the rawresult.exitCode.
Comments suppressed due to low confidence (1)
tests/fixtures/awf-runner.ts:299
- Same issue here: the
|| 0normalization can turn a missing/invalid exit code into 0 and thereforesuccess: true. Prefer nullish coalescing for exit code normalization and ensuresuccesscan’t become true when the process ends without a real 0 exit code (e.g. signal termination).
// Normalize exit code (undefined/null becomes 0)
const exitCode = result.exitCode || 0;
return {
exitCode,
stdout: result.stdout || '',
stderr: result.stderr || '',
success: exitCode === 0,
timedOut: false,
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/fixtures/awf-runner.ts
Outdated
| // Normalize exit code (undefined/null becomes 0) | ||
| const exitCode = result.exitCode || 0; | ||
|
|
||
| return { | ||
| exitCode: result.exitCode || 0, | ||
| exitCode, | ||
| stdout: result.stdout || '', | ||
| stderr: result.stderr || '', | ||
| success: result.exitCode === 0, | ||
| success: exitCode === 0, |
There was a problem hiding this comment.
result.exitCode || 0 does not only normalize undefined/null—it also coerces other falsy values (e.g. NaN) to 0, which can incorrectly mark the run as success. Use nullish coalescing for the normalization, and consider guarding success against cases where the process terminated without an exit code (e.g. due to a signal) so it can’t be reported as successful.
This issue also appears on line 291 of the same file.
See below for a potential fix:
const rawExitCode = result.exitCode;
const exitCode = rawExitCode ?? 0;
const hasValidExitCode =
typeof rawExitCode === 'number' && Number.isFinite(rawExitCode);
return {
exitCode,
stdout: result.stdout || '',
stderr: result.stderr || '',
success: hasValidExitCode && exitCode === 0,
tests/fixtures/awf-runner.ts
Outdated
| // Normalize exit code (undefined/null becomes 0) | ||
| const exitCode = result.exitCode || 0; | ||
|
|
There was a problem hiding this comment.
PR title is scoped as docs: but this change modifies test fixture logic. Consider renaming the PR (and/or using a non-docs conventional commit scope) so release notes/changelogs and reviewers correctly categorize it.
C++ Build Test Results
Overall: PASS All C++ projects built successfully.
|
Go Build Test Results
Overall: PASS ✅ All Go projects successfully downloaded dependencies and passed tests.
|
Deno Build Test Results ✅
Overall: PASS All Deno tests completed successfully!
|
Build Test: Rust - ResultsAll Rust projects built and tested successfully! ✅
Overall: PASS ✅ Both projects compiled successfully and all tests passed.
|
|
@claude[agent] merge this branch into claude/extract-rust-one-shot-token-lib |
Resolved conflicts in favor of main's C-based one-shot-token implementation (replacing PR's Rust-based version) and main's nullish coalescing operator for exit code normalization in awf-runner.ts. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
|
Smoke Test Results (Claude) - Run #21978273270 ✅ GitHub MCP: Retrieved last 2 merged PRs
✅ Playwright: Navigated to github.com (title: "GitHub · Change is constant. GitHub keeps you ahead. · GitHub") ✅ File Writing: Created ✅ Bash Tool: Verified file content Status: PASS ✅
|
C++ Build Test Results
Overall: PASS ✅ All C++ projects built successfully.
|
Go Build Test Results
Overall: PASS ✅ All Go projects successfully downloaded dependencies and passed tests.
|
Node.js Build Test Results ✅All Node.js projects tested successfully!
Overall: PASS ✅
|
Smoke Test Results - PASS ✅Last 2 Merged PRs:
Test Results:
Overall Status: PASS cc
|
🦀 Rust Build Test Results
Overall: PASS ✅ All Rust projects built and tested successfully.
|
Bun Build Test Results
Overall: PASS ✅ All Bun projects built and tested successfully.
|
Deno Build Test Results
Overall: ✅ PASS All Deno tests completed successfully.
|
.NET Build Test Results
Overall: PASS ✅ All .NET projects successfully restored, built, and executed.
|
|
Smoke test results:
|
🧪 Chroot Version Comparison Test Results
Overall Status: ❌ Test Failed Chroot mode is working correctly (binaries from host are accessible), but version mismatches were detected for Python and Node.js. This is expected when the host system has different versions installed than what's available in the agent container's Ubuntu base image.
|
Build Test: Java ✅All Java projects compiled and tested successfully!
Overall: PASS ✅ All tests passed successfully through the AWF firewall with Maven proxy configuration.
|
3 participants
Changes Made
Fixed the test runner fixture (
AwfRunner) to properly validate exit codes before marking runs as successful:||operator to nullish coalescing (??) for exit code normalization to avoid incorrectly coercing falsy values likeNaNto 0hasValidExitCodevalidation to ensure the exit code is a number and finite before marking as successrun()andrunWithSudo()methods✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.