refactor: extract shared Docker test fixture constants to eliminate copy-paste across 6 test files - #2618
Conversation
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
✨ New Files (1 files)
Coverage comparison generated by |
Smoke Test Results✅ GitHub MCP: Listed 2 merged PRs Status: PASS
|
🔬 Smoke Test Results
PR: "refactor: extract shared Docker test fixture constants to eliminate copy-paste across 6 test files" Overall Status:
|
🔥 Smoke Test: Copilot BYOK (Offline) Mode
Running in BYOK offline mode ( Overall: PARTIAL — BYOK inference and MCP pass; pre-step template vars were not substituted. PR author:
|
There was a problem hiding this comment.
Pull request overview
This PR reduces duplication in the unit-test suite by extracting shared Docker compose test fixtures into a single reusable test-utils module, keeping service/compose tests consistent and easier to maintain.
Changes:
- Added a shared test-utils module exporting
baseConfigandmockNetworkConfig. - Updated compose-generator and service unit tests to import the shared fixtures instead of duplicating inline constants.
- Added per-test-file comments clarifying why
jest.mock('execa', ...)must remain local (hoisting behavior).
Show a summary per file
| File | Description |
|---|---|
| src/test-helpers/docker-test-fixtures.test-utils.ts | New shared source of truth for baseConfig and mockNetworkConfig used by multiple Docker compose-related tests. |
| src/services/squid-service.test.ts | Replaced inline fixture constants with imports from the shared test-utils module. |
| src/services/doh-proxy-service.test.ts | Replaced inline fixture constants with imports from the shared test-utils module. |
| src/services/cli-proxy-service.test.ts | Replaced inline fixture constants with imports from the shared test-utils module. |
| src/services/api-proxy-service.test.ts | Replaced inline fixture constants with imports from the shared test-utils module. |
| src/services/agent-service.test.ts | Replaced inline fixture constants with imports from the shared test-utils module. |
| src/compose-generator.test.ts | Replaced inline fixture constants with imports from the shared test-utils module. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 7/7 changed files
- Comments generated: 0
Smoke TestPRs: feat(api-proxy): OIDC authentication for Azure OpenAI (Entra-only); Enable Copilot BYOK provider-env fallback, base-path routing, and hardened token isolation 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.
|
Chroot Smoke Test Results
Result: ❌ Not all tests passed. Python and Node.js versions differ between host and chroot environments.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
Smoke Test Results
Overall: FAIL —
|
baseConfigandmockNetworkConfigwere copy-pasted verbatim across 6 test files (~102 duplicate lines). Any change to the shared mock shape required edits in 6 places, with inconsistencies already creeping in.Changes
src/test-helpers/docker-test-fixtures.test-utils.ts— single source of truth forbaseConfig(Omit<WrapperConfig, 'workDir'>) andmockNetworkConfigsrc/services/{squid,doh-proxy,cli-proxy,api-proxy,agent}-service.test.tsandsrc/compose-generator.test.ts— replace inline declarations with imports from the shared moduleThe
jest.mock('execa', ...)block withmockExecaFn/mockExecaSyncintentionally remains per-file — Jest hoistsjest.mock()before imports are resolved, so the factory closure cannot reference variables from an external module. Each file now carries a comment explaining this constraint.The
.test-utils.tssuffix follows the existing convention (src/commands/test-helpers.test-utils.ts) and is excluded from the TypeScript build output viatsconfig.json.