refactor(tests): extract shared harness for network-isolation workflow tests - #6496
Conversation
Removes ~80 lines of repeated test setup across 5 nearly-identical blocks in the
`onNetworkReady static DNS pre-registration` describe block. The new
`runNetworkIsolationWorkflow` helper centralises:
- mockedRuntimeNeedsStaticDns.mockReturnValue(...)
- mockedGetTopologyContainerIps.mockResolvedValue(peerIps)
- mockedPatchComposeWithTopologyHosts.mockImplementation(() => {})
- building the WrapperConfig with networkIsolation: true
- creating the startContainers stub that invokes onNetworkReady
- calling runMainWorkflow
Each test now only declares its unique inputs (needsStaticDns, peerIps,
configOverrides, connectTopologyContainers) and its specific assertions.
Closes #6490
There was a problem hiding this comment.
Pull request overview
Refactors network-isolation workflow tests by extracting repeated setup into a shared helper.
Changes:
- Adds a configurable network-isolation test harness.
- Simplifies five topology and static-DNS test cases.
Show a summary per file
| File | Description |
|---|---|
src/cli-workflow.test.ts |
Centralizes repeated workflow mock setup and execution. |
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: 1
- Review effort level: Medium
| * Each test only needs to declare the inputs and assertions unique to it. | ||
| */ | ||
| const runNetworkIsolationWorkflow = async ({ | ||
| needsStaticDns = false, |
|
@copilot address review feedback |
Addressed in I removed the ineffective |
|
✅ Smoke Gemini completed. All facets verified. 💎 |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
✅ Contribution Check completed successfully! Contribution check complete: the PR follows the applicable guidelines in CONTRIBUTING.md. The change is a test refactor with no new functionality, includes tests in the touched test file, stays in the correct source area, and the PR description is clear with a related issue reference. |
|
📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅ |
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓 |
|
🔑 Smoke Copilot PAT PAT auth validated. All systems operational. ✅ |
|
🛡️ Smoke Copilot Network Isolation confirmed the egress allowlist is enforced. ✅ |
|
✅ Build Test Suite completed successfully! |
|
🔌 Smoke Services — All services reachable! ✅ |
|
📰 VERDICT: Smoke Docker Sbx has concluded. All systems operational. This is a developing story. 🎤 |
|
✅ Smoke Copilot BYOK AOAI (api-key) completed. Copilot AOAI BYOK (api-key) mode operational. 🔓 |
|
🚀 Security Guard has started processing this pull request |
|
📰 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 Network Isolation — PASS
Overall: PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "example.com"See Network Configuration for more information.
|
github-actions
Bot
added
the
smoke-copilot-network-isolation
Smoke Test: Claude Engine Validation
Overall result: PASS
|
🔬 Smoke Test Results — PAT Auth
Auth mode: PAT (COPILOT_GITHUB_TOKEN)
Overall: PARTIAL (MCP ✅, pre-step data unavailable)
|
🔬 Smoke Test Results
Overall: INCONCLUSIVE — workflow template variables (
|
Smoke Test
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 Results — Services Connectivity
Overall: FAIL —
|
|
Smoke Test: Copilot BYOK (Direct) Mode ✅ PASS
Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY) via api-proxy → api.githubcopilot.com
|
Smoke Test Results\n- GitHub MCP: ✅ (Filtered by DIFC)\n- GitHub.com: ✅ (via proxy)\n- File Writing: ✅\n- Bash Tool: ✅\n\nOverall status: PASSWarning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "localhost"See Network Configuration for more information.
|
🔍 Smoke Test: API Proxy OpenTelemetry Tracing
All 5 scenarios passed. OTEL tracing integration is functional.
|
|
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
🧪 Smoke Test: Docker Sbx — PASS
Overall: PASS
|
Five test cases in the
onNetworkReady static DNS pre-registrationdescribe block insrc/cli-workflow.test.tsrepeated the same ~16-line setup — mockingruntimeNeedsStaticDns,getTopologyContainerIps, andpatchComposeWithTopologyHosts, constructing anetworkIsolation: trueconfig, and wiring up astartContainersstub that firesonNetworkReady— differing only in peer IPs, config overrides, and assertions.Changes
src/cli-workflow.test.ts— addsrunNetworkIsolationWorkflowhelper inside the describe block, accepting{ needsStaticDns, peerIps, configOverrides, connectTopologyContainers }with sensible defaults; each of the five tests now only supplies what's unique to it:Reduces ~80 lines of boilerplate; future changes to the topology test harness now require a single edit point.