Skip to content

refactor: split cli.test.ts by feature area into co-located test files#2557

Merged
lpcox merged 3 commits intomainfrom
copilot/refactor-split-cli-test-by-feature
May 5, 2026
Merged

refactor: split cli.test.ts by feature area into co-located test files#2557
lpcox merged 3 commits intomainfrom
copilot/refactor-split-cli-test-by-feature

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 5, 2026

src/cli.test.ts had grown to 2,909 lines testing 15+ unrelated concerns under a single describe('cli') block, including functions already extracted to separate modules (domain-utils.ts, option-parsers.ts, api-proxy-config.ts).

Changes

  • src/domain-utils.test.ts (new) — parseDomains, parseDomainsFile, isValidIPv4/IPv6, validateAgentImage, processAgentImageOption, AGENT_IMAGE_PRESETS, DEFAULT_*_API_TARGET constants
  • src/option-parsers.test.ts (new) — parseEnvironmentVariables, parseVolumeMounts, escapeShellArg/joinShellArgs, parseDnsServers, parseDnsOverHttps, processLocalhostKeyword, buildRateLimitConfig, validateRateLimitFlags, validateAllowHostPorts/ServicePorts, applyHostServicePortsConfig, parseMemoryLimit, parseAgentTimeout, checkDockerHost, formatItem, and related validators
  • src/api-proxy-config.test.ts (new) — validateApiProxyConfig, validateAnthropicCacheTailTtl, validateApiTargetInAllowedDomains, emitApiProxyTargetWarnings, emitCliProxyStatusLogs, warnClassicPATWithCopilotModel, resolveApiTargetsToAllowedDomains (including GHEC/GHES variants), extractGhes/GhecDomains*
  • src/dns-resolver.test.ts (extended) — added describe('DEFAULT_DNS_SERVERS') block, importing directly from ./dns-resolver instead of via dynamic ./cli import
  • src/cli.test.ts (reduced 2,909 → 327 lines) — retains only tests for things genuinely in cli.ts: secret redaction, log level validation, Commander.js program configuration, argument parsing, work directory generation, validateFormat, help text formatting, handlePredownloadAction

The two duplicate describe blocks (hasRateLimitOptions and formatItem) were merged into single blocks with all unique test cases preserved.

Copilot AI changed the title [WIP] Refactor to split src/cli.test.ts by feature area refactor: split cli.test.ts by feature area into co-located test files May 5, 2026
Copilot finished work on behalf of lpcox May 5, 2026 13:56
Copilot AI requested a review from lpcox May 5, 2026 13:56
@lpcox lpcox marked this pull request as ready for review May 5, 2026 18:27
@lpcox lpcox requested a review from Mossaka as a code owner May 5, 2026 18:27
Copilot AI review requested due to automatic review settings May 5, 2026 18:27
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

⚠️ Coverage Regression Detected

This PR decreases test coverage. Please add tests to maintain coverage levels.

Overall Coverage

Metric Base PR Delta
Lines 86.45% 86.53% 📈 +0.08%
Statements 86.38% 86.46% 📈 +0.08%
Functions 90.61% 80.51% 📉 -10.10%
Branches 79.23% 79.27% 📈 +0.04%
📁 Per-file Coverage Changes (1 files)
File Lines (Before → After) Statements (Before → After)
src/container-lifecycle.ts 86.1% → 87.2% (+1.09%) 86.3% → 87.4% (+1.08%)

Coverage comparison generated by scripts/ci/compare-coverage.ts

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the CLI unit test suite by splitting the previously monolithic src/cli.test.ts into smaller, feature-focused test files co-located with their corresponding modules. This improves test organization and makes it easier to maintain and extend coverage as the CLI continues to grow.

Changes:

  • Added dedicated test suites for domain-utils, option-parsers, and api-proxy-config, moving module-specific tests out of cli.test.ts.
  • Simplified cli.test.ts to only cover behavior that lives directly in cli.ts (e.g., Commander config, help formatting, secret redaction, handlePredownloadAction).
  • Extended dns-resolver.test.ts with direct coverage of the DEFAULT_DNS_SERVERS constant and updated import style.
Show a summary per file
File Description
src/option-parsers.test.ts New, module-scoped tests for option parsing/validation helpers previously tested via cli.test.ts.
src/domain-utils.test.ts New, module-scoped tests for domain parsing, IP validation, agent image validation/presets, and default API targets.
src/api-proxy-config.test.ts New, module-scoped tests for API proxy configuration/validation and GHEC/GHES domain extraction helpers.
src/dns-resolver.test.ts Adds direct assertion for DEFAULT_DNS_SERVERS and imports it directly from dns-resolver.
src/cli.test.ts Reduced to CLI-owned responsibilities (program/help/config, redaction, validateFormat, handlePredownloadAction).

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 4/5 changed files
  • Comments generated: 1

Comment thread src/cli.test.ts Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

⚠️ Coverage Regression Detected

This PR decreases test coverage. Please add tests to maintain coverage levels.

Overall Coverage

Metric Base PR Delta
Lines 86.45% 86.53% 📈 +0.08%
Statements 86.38% 86.46% 📈 +0.08%
Functions 90.61% 80.51% 📉 -10.10%
Branches 79.23% 79.27% 📈 +0.04%
📁 Per-file Coverage Changes (2 files)
File Lines (Before → After) Statements (Before → After)
src/container-cleanup.ts 77.3% → 77.5% (+0.12%) 77.0% → 77.1% (+0.11%)
src/container-lifecycle.ts 86.1% → 87.2% (+1.09%) 86.3% → 87.4% (+1.08%)

Coverage comparison generated by scripts/ci/compare-coverage.ts

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

Smoke Test Results

✅ GitHub MCP: api-proxy X-Initiator fix, one-shot-token musl fix
✅ Playwright: GitHub page loaded
✅ File I/O: Test file created
✅ Bash: File verified

Status: PASS

💥 [THE END] — Illustrated by Smoke Claude

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

🔥 Smoke Test: Copilot BYOK — PASS

Test Result
GitHub MCP (list PRs)
GitHub.com connectivity
File write/read (smoke-test-copilot-byok-25394997804.txt)
BYOK inference (api-proxy → api.githubcopilot.com)

Running in BYOK offline mode (COPILOT_OFFLINE=true) via api-proxy → api.githubcopilot.com

Author: @Copilot · Assignees: @lpcox, @Copilot · Overall: PASS

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

🔬 Smoke Test Results

Test Status
GitHub MCP connectivity
GitHub.com HTTP ❌ (template vars unresolved)
File write/read ❌ (template vars unresolved)

PR: refactor: split cli.test.ts by feature area into co-located test files
Author: @Copilot | Assignees: @lpcox, @Copilot

Overall: FAIL — pre-step outputs (SMOKE_HTTP_CODE, SMOKE_FILE_PATH, SMOKE_FILE_CONTENT) were not substituted before agent execution.

📰 BREAKING: Report filed by Smoke Copilot

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

🏗️ Build Test Suite Results

Ecosystem Project Build/Install Tests Status
Bun elysia 1/1 passed ✅ PASS
Bun hono 1/1 passed ✅ PASS
C++ fmt N/A ✅ PASS
C++ json N/A ✅ PASS
Deno oak N/A 1/1 passed ✅ PASS
Deno std N/A 1/1 passed ✅ PASS
.NET hello-world N/A ✅ PASS
.NET json-parse N/A ✅ PASS
Go color 1/1 passed ✅ PASS
Go env 1/1 passed ✅ PASS
Go uuid 1/1 passed ✅ PASS
Java gson 1/1 passed ✅ PASS
Java caffeine 1/1 passed ✅ PASS
Node.js clsx All passed ✅ PASS
Node.js execa All passed ✅ PASS
Node.js p-limit All passed ✅ PASS
Rust fd 1/1 passed ✅ PASS
Rust zoxide 1/1 passed ✅ PASS

Overall: 8/8 ecosystems passed — ✅ PASS

Generated by Build Test Suite for issue #2557 · ● 396.1K ·

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

🔮 Codex smoke: FAIL
✅ api-proxy: inject X-Initiator: agent default on all Copilot-bound requests to prevent billing inflation
✅ fix: one-shot-token.so fails to load on musl/Alpine hosts (ARC runners)
❌ Safe Inputs GH / discussion safe-input: commands unavailable; gh fallback succeeded
✅ Playwright title, file/bash, discussion comment, build
❌ Tavily search: server exposed no tools
Overall status: FAIL

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • registry.npmjs.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "registry.npmjs.org"

See Network Configuration for more information.

🔮 The oracle has spoken through Smoke Codex

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

Smoke Test Results

Check Result
Redis PING ❌ timeout (no response)
PostgreSQL pg_isready ❌ no response
PostgreSQL SELECT 1 ❌ no response

host.docker.internal resolves to 172.17.0.1 but ports 6379 and 5432 are unreachable from this runner.

Overall: FAIL — service containers are not accessible from this environment.

🔌 Service connectivity validated by Smoke Services

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

Chroot Version Comparison — Smoke Test Results

Runtime Host Version Chroot Version Match?
Python Python 3.12.13 Python 3.12.3 ❌ NO
Node.js v24.14.1 v20.20.2 ❌ NO
Go go1.22.12 go1.22.12 ✅ YES

Overall: ❌ Not all tests passed (Python and Node.js versions differ between host and chroot environment).

Tested by Smoke Chroot

@lpcox lpcox merged commit 57478ed into main May 5, 2026
64 of 68 checks passed
@lpcox lpcox deleted the copilot/refactor-split-cli-test-by-feature branch May 5, 2026 20:20
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.

[Refactoring] Split src/cli.test.ts (2,909 lines) by feature area

3 participants