refactor(tests): deduplicate signal-handler tests with shared helper and it.each#5660
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors registerSignalHandlers unit tests to remove repeated per-signal boilerplate by introducing a shared runSignalScenario helper and parameterizing paired SIGINT/SIGTERM test cases. This keeps the test behavior coverage the same while making future signal-cleanup semantics changes easier to update in one place.
Changes:
- Introduces
runSignalScenarioto centralize mock wiring, handler registration, signal dispatch, and promise flushing. - Collapses duplicated SIGINT/SIGTERM assertions into
it.each-parameterized tests for fast-kill behavior and error-swallowing behavior. - Updates remaining unique tests to use the shared helper while preserving their intent.
Show a summary per file
| File | Description |
|---|---|
| src/commands/signal-handler.test.ts | Deduplicates SIGINT/SIGTERM handler tests using a shared scenario helper and it.each parameterization. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Low
|
✅ Copilot review passed with no inline comments. @copilot Add the |
|
🔌 Smoke Services — All services reachable! ✅ |
|
✅ Contribution Check completed successfully! Contribution check complete for PR #5660: the test-only refactor follows the applicable CONTRIBUTING.md guidelines, with no important missing items requiring a PR comment. |
|
🔑 Smoke Copilot PAT PAT auth validated. All systems operational. ✅ |
|
✅ Smoke Gemini completed. All facets verified. 💎 Smoke test completed with partial success (File/Bash passed, Network/MCP failed). |
|
✅ Smoke Copilot BYOK AOAI (api-key) completed. Copilot AOAI BYOK (api-key) mode operational. 🔓 |
|
📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅ |
|
✅ Smoke Copilot BYOK AOAI (Entra) completed. Copilot AOAI BYOK (Entra) mode operational. 🔓 |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
🚀 Security Guard has started processing this pull request |
|
✅ Build Test Suite completed successfully! |
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓 |
|
✅ Smoke Claude passed |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
|
Smoke Test: Copilot BYOK (Direct) Results ✅ MCP Connectivity — Listed 2 recent merged PRs from gh-aw-firewall Status: PASS | Mode: Direct BYOK via api-proxy
|
🔥 Smoke Test — Auth mode: PAT (COPILOT_GITHUB_TOKEN)
Overall: FAIL
|
Smoke Test: Claude Engine Validation
Overall result: PASS
|
🤖 Smoke Test ResultsPR: refactor(tests): deduplicate signal-handler tests with shared helper and it.each
Overall: PARTIAL — MCP verified ✅; pre-step template variables unexpanded (workflow infra issue, not engine failure)
|
Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
|
|
🔍 Smoke Test: API Proxy OpenTelemetry Tracing
All 5 scenarios passed. OTEL tracing integration is functional.
|
|
✅ GitHub MCP Testing Running in direct BYOK mode (AWF_AUTH_TYPE=github-oidc + AWF_AUTH_AZURE_* + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw) authenticated via Microsoft Entra Overall: PASS
|
Chroot Version Comparison Results
Overall: ❌ Not all tests passed — Python and Node.js versions differ between host and chroot environments.
|
Smoke Test Results (Gemini)
Overall status: FAIL Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "localhost"See Network Configuration for more information.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
Smoke Test: Services Connectivity
Overall: FAIL —
|
SIGINT and SIGTERM signal-handler tests had identical mock setup, dependency wiring, handler dispatch, and promise-flushing repeated across five test cases — any change to cleanup behaviour required updating multiple nearly-identical blocks.
Changes
runSignalScenariohelper — inner async function that accepts{ signal, containersStarted, keepContainers, fastKillRejects? }, wires up mocks + deps, callsregisterSignalHandlers, dispatches the signal, flushes promises, and returns{ fastKill, performCleanup }for assertions.it.eachfor paired SIGINT/SIGTERM cases — two duplicate test pairs collapsed into parameterised tests:fast-kills agent container on %s when containers are started and keepContainers is falseswallows errors thrown during %s handlingregisters handlers,skips fast-kill when containers not started,skips fast-kill when keepContainers is true) rewritten to callrunSignalScenario, eliminating inline boilerplate.Net: ~60 lines removed, same coverage, single place to update when cleanup semantics change.