Align docker-manager tests with source-module import contract#3960
Conversation
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (2 files)
Coverage comparison generated by |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
This PR restores the test import layering contract by updating docker-manager-related unit tests to import directly from their owning source modules rather than the src/docker-manager.ts backwards-compatibility re-export barrel.
Changes:
- Updated
docker-manager-*.test.tsimports to referencehost-env,config-writer,container-lifecycle,docker-host, andcontainer-cleanupdirectly. - Reduced unit-test coupling to the public compatibility surface (
./docker-manager) so refactors don’t force test churn.
Show a summary per file
| File | Description |
|---|---|
| src/docker-manager-utils.test.ts | Imports parseDifcProxyHost from ./host-env instead of the compatibility barrel. |
| src/docker-manager-lifecycle.test.ts | Splits lifecycle/host/cleanup imports to their owning modules. |
| src/docker-manager-cleanup.test.ts | Imports writeConfigs from ./config-writer and cleanup/diagnostics from ./container-cleanup. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 2
| readEnvFile, | ||
| } from './github-env'; | ||
| import { parseDifcProxyHost } from './docker-manager'; | ||
| import { parseDifcProxyHost } from './host-env'; |
| import { startContainers, runAgentCommand, fastKillAgentContainer } from './container-lifecycle'; | ||
| import { setAwfDockerHost, getLocalDockerEnv } from './docker-host'; | ||
| import { stopContainers } from './container-cleanup'; | ||
| import { containerLifecycleTestHelpers } from './container-lifecycle'; |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@copilot address review feedback |
Addressed. I consolidated duplicate imports in both flagged test files in commit |
Smoke Test (Claude) — ✅ PASS
Total: PASS
|
🔥 Smoke Test: Copilot BYOK (Offline) Mode
Running in BYOK offline mode ( PR: "Align docker-manager tests with source-module import contract" — author: Overall: PASS ✅
|
🧪 Smoke Test Results
Overall: PASS PR: "Align docker-manager tests with source-module import contract" — author
|
🧪 Chroot Smoke Test Results
|
Smoke Test Results
Overall: FAIL — service containers are not reachable from this runner environment.
|
Smoke Test Results: FAIL (2/4 passed)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
|
|
#3960 Align docker-manager tests with source-module import contract
|
Three docker-manager test files had reverted to importing via the
docker-managercompatibility barrel, violating its explicit contract that tests import from source modules directly. This PR restores direct imports so tests target the owning modules instead of the public re-export surface.Import contract restoration
src/docker-manager-utils.test.ts:parseDifcProxyHostnow imports from./host-envsrc/docker-manager-cleanup.test.ts:writeConfigsnow imports from./config-writer; cleanup/diagnostic helpers from./container-cleanupsrc/docker-manager-lifecycle.test.ts: lifecycle ops import from./container-lifecycle; docker host helpers from./docker-host;stopContainersfrom./container-cleanupLayering clarity in tests
src/docker-manager.ts)