Skip to content

test: improve docker-manager.ts coverage to 80%+#140

Merged
Mossaka merged 2 commits intomainfrom
copilot/increase-test-coverage-core
Dec 19, 2025
Merged

test: improve docker-manager.ts coverage to 80%+#140
Mossaka merged 2 commits intomainfrom
copilot/increase-test-coverage-core

Conversation

Copy link
Contributor

Copilot AI commented Dec 19, 2025

Core components had critically low test coverage: docker-manager.ts at 27%, cli.ts at 47%. These files handle Docker orchestration and CLI entry point—the most critical paths in the codebase.

Changes

docker-manager.ts (27% → 80.27%)

Added comprehensive tests for:

  • Container lifecycle: startContainers(), stopContainers() with mocked execa
  • Command execution: runAgentCommand() exit code propagation, blocked domain detection from Squid access logs
  • Cleanup logic: Log preservation, proxyLogsDir handling, empty directory handling
  • Config generation: writeConfigs() directory creation, generateDockerCompose() DNS and proxy log options
describe('runAgentCommand', () => {
  it('should detect blocked domains from access log', async () => {
    fs.writeFileSync(
      path.join(squidLogsDir, 'access.log'),
      '1760994429.358 172.30.0.20:36274 blocked.com:443 -:- 1.1 CONNECT 403 TCP_DENIED:HIER_NONE blocked.com:443 "curl/7.81.0"\n'
    );
    mockExecaFn.mockResolvedValueOnce({ stdout: '1' } as any);
    
    const result = await runAgentCommand(testDir, ['github.com']);
    expect(result.blockedDomains).toContain('blocked.com');
  });
});

cli.ts (47.08%)

Target of 60% not fully met—lines 350-597 are the action handler using process.exit() which is difficult to unit test. However:

  • Core workflow logic lives in cli-workflow.ts with 100% coverage
  • All exported utility functions (parseDomains, parseDnsServers, parseVolumeMounts, etc.) are fully tested

Coverage Summary

File Before After Target
docker-manager.ts 27% 80.27% 80% ✅
cli.ts 47% 47% 60% ⚠️
cli-workflow.ts 100% 100% — ✅
Original prompt

This section details on the original issue you should resolve

<issue_title>[Testing] Low test coverage for core components - docker-manager.ts 18%, cli.ts 0%</issue_title>
<issue_description>## Priority
P0 - Critical (for code quality)

Summary

Core components have critically low test coverage:

  • docker-manager.ts: 18% (handles all Docker orchestration)
  • cli.ts: 0% (main entry point)

These are the most important files in the codebase, yet they have the least test coverage.

Current Coverage

File Coverage Risk Level
src/docker-manager.ts 18% Critical - core orchestration
src/cli.ts 0% Critical - main entry point
src/squid-config.ts Good Lower risk
src/host-iptables.ts Moderate Medium risk

Impact

  • Regressions go undetected: Changes to Docker orchestration may break without tests
  • Refactoring risky: Cannot safely improve code without comprehensive tests
  • Bug fixes incomplete: Hard to verify fixes without test coverage
  • CI false confidence: Passing CI doesn't mean core functionality works

Target Coverage

File Current Target
src/docker-manager.ts 18% 80%
src/cli.ts 0% 60%

Test Cases Needed

docker-manager.ts

describe('Docker Manager', () => {
  describe('container lifecycle', () => {
    it('should start squid container with correct config');
    it('should start agent container after squid is healthy');
    it('should handle squid startup failure');
    it('should handle agent startup failure');
    it('should propagate exit codes correctly');
    it('should stream logs in real-time');
  });

  describe('network configuration', () => {
    it('should create awf-net with correct subnet');
    it('should assign correct IPs to containers');
    it('should handle network creation failure');
    it('should cleanup network on exit');
  });

  describe('cleanup', () => {
    it('should stop containers on normal exit');
    it('should stop containers on SIGINT');
    it('should stop containers on error');
    it('should preserve logs when keep-containers is set');
  });

  describe('log streaming', () => {
    it('should stream stdout in real-time');
    it('should stream stderr in real-time');
    it('should handle disconnection gracefully');
  });
});

cli.ts

describe('CLI', () => {
  describe('argument parsing', () => {
    it('should parse --allow-domains correctly');
    it('should parse multiple comma-separated domains');
    it('should parse --dns-servers correctly');
    it('should reject invalid domain formats');
    it('should handle --keep-containers flag');
    it('should handle --build-local flag');
    it('should handle --image-tag flag');
  });

  describe('workflow', () => {
    it('should call writeConfigs before startContainers');
    it('should call stopContainers in finally block');
    it('should exit with agent exit code');
    it('should handle SIGINT gracefully');
    it('should handle SIGTERM gracefully');
  });

  describe('error handling', () => {
    it('should show helpful error for missing domains');
    it('should show helpful error for missing command');
    it('should cleanup on error');
  });
});

Files to Create/Modify

  • Create/expand: src/docker-manager.test.ts
  • Create: src/cli.test.ts
  • Modify: jest.config.js - ensure coverage thresholds

Implementation Notes

  • Use mocking for Docker commands (execa)
  • Use Jest's mock filesystem for config files
  • Test signal handling carefully (may need child processes)
  • Integration tests separate from unit tests

Acceptance Criteria

  • docker-manager.ts coverage ≥ 80%
  • cli.ts coverage ≥ 60%
  • All tests pass in CI
  • No regression in existing tests</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Co-authored-by: Mossaka <5447827+Mossaka@users.noreply.github.com>
Copilot AI changed the title [WIP] Add unit tests for core components coverage test: improve docker-manager.ts coverage to 80%+ Dec 19, 2025
Copilot AI requested a review from Mossaka December 19, 2025 09:46
@Mossaka Mossaka added the smoke label Dec 19, 2025
@github-actions
Copy link

💥 WHOOSH! Smoke Claude springs into action on this pull request! [Panel 1 begins...]

@github-actions
Copy link

📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing...

@github-actions
Copy link

github-actions bot commented Dec 19, 2025

Test Coverage Report

Metric Coverage Covered/Total
Lines 66.1% 708/1071
Statements 66.27% 729/1100
Functions 70.73% 87/123
Branches 60.89% 232/381
Coverage Thresholds

The project has the following coverage thresholds configured:

  • Lines: 38%
  • Statements: 38%
  • Functions: 35%
  • Branches: 30%

Coverage report generated by `npm run test:coverage`

@Mossaka Mossaka marked this pull request as ready for review December 19, 2025 17:52
@github-actions
Copy link

Smoke Test Results

Last 2 merged PRs:

Test Results:

  • ✅ GitHub MCP: Retrieved PR metadata
  • ✅ File Writing: Created /tmp/gh-aw/agent/smoke-test-copilot-20378116964.txt
  • ✅ Bash Tool: Verified file content

Status: PASS

📰 BREAKING: Report filed by Smoke Copilot fer issue #140 🗺️

@github-actions
Copy link

Smoke Test Results

Last 2 merged PRs:

Test Results:

  • ✅ GitHub MCP: Fetched PRs successfully
  • ✅ File Writing: Created test file at /tmp/gh-aw/agent/smoke-test-claude-20378117039.txt
  • ✅ Bash Tool: Verified file contents
  • ❌ Playwright MCP: Connection failures (ERR_TUNNEL_CONNECTION_FAILED)

Overall Status: PARTIAL PASS

Note: Playwright connectivity issues detected - page navigation failed.

💥 [THE END] — Illustrated by Smoke Claude fer issue #140 🗺️

@Mossaka Mossaka merged commit b4b6a99 into main Dec 19, 2025
29 checks passed
@Mossaka Mossaka deleted the copilot/increase-test-coverage-core branch December 19, 2025 23:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Testing] Low test coverage for core components - docker-manager.ts 18%, cli.ts 0%

2 participants