refactor: extract shared service test imports into service-test-setup.test-utils#3324
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors the duplicated 3-line import block found in all 13 src/services/*.test.ts files into a single shared re-export module, so future changes to generateDockerCompose, WrapperConfig, or the test fixtures only require updating one file.
Changes:
- Adds
src/services/service-test-setup.test-utils.tsre-exportinggenerateDockerCompose,WrapperConfig,baseConfig,mockNetworkConfig, anduseTempWorkDir. - Updates all 13 service test files to import these symbols from the new shared module.
- Documents (via JSDoc) why
jest.mock('execa', ...)and themockConfigbinding must remain per-file.
Show a summary per file
| File | Description |
|---|---|
| src/services/service-test-setup.test-utils.ts | New shared re-export module with JSDoc explaining the jest.mock hoisting constraint. |
| src/services/squid-service.test.ts | Replaces 3 imports with the shared one. |
| src/services/doh-proxy-service.test.ts | Replaces 3 imports with the shared one. |
| src/services/cli-proxy-service.test.ts | Replaces 3 imports with the shared one. |
| src/services/api-proxy-service-rate-limit.test.ts | Replaces 3 imports with the shared one. |
| src/services/api-proxy-service-key-isolation.test.ts | Replaces 3 imports with the shared one. |
| src/services/api-proxy-service-env-forwarding.test.ts | Replaces 3 imports with the shared one. |
| src/services/api-proxy-service-config.test.ts | Replaces 3 imports with the shared one. |
| src/services/agent-volumes.test.ts | Replaces 3 imports with the shared one. |
| src/services/agent-service.test.ts | Replaces 3 imports with the shared one. |
| src/services/agent-environment-runtime.test.ts | Replaces 3 imports with the shared one. |
| src/services/agent-environment-proxy.test.ts | Replaces 3 imports with the shared one. |
| src/services/agent-environment-options.test.ts | Replaces 3 imports with the shared one. |
| src/services/agent-environment-credentials.test.ts | Replaces 3 imports with the shared one. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 14/14 changed files
- Comments generated: 0
🧪 Smoke Test: Copilot BYOK (Offline) Mode
Running in BYOK offline mode ( Overall:
|
Smoke Test Results❌ GitHub API: Bad credentials Result: FAIL (1/3 tests failed)
|
🔬 Smoke Test Results
Overall: FAIL — GitHub MCP returned 401; template variables were not substituted before agent invocation. No PR author/assignee info available (MCP auth unavailable).
|
Smoke TestPR titles: fix: resolve test failures on macOS; fix: postprocess claude-token-optimizer lock file to use local awf build 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.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
Smoke Test: GitHub Actions Services Connectivity
Overall: FAIL
|
|
Gemini Smoke Test: PASS 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.
|
Chroot Smoke Test Results
Overall: ❌ Not all tests passed — Python and Node.js versions differ between host and chroot.
|
The same 3-line import block was duplicated verbatim across all 13
src/services/*.test.tsfiles, meaning any change togenerateDockerCompose,WrapperConfig, or the fixture exports required touching every file.Changes
src/services/service-test-setup.test-utils.ts— re-exportsgenerateDockerCompose,WrapperConfig,baseConfig,mockNetworkConfig, anduseTempWorkDirfrom their respective source modulesBefore / After
The
jest.mock('execa', ...)call andlet mockConfig: WrapperConfigdeclaration remain per-file — Jest hoistsjest.mock()before imports are resolved, so the factory cannot reference variables from an imported module. This constraint is documented in the new shared module's JSDoc.