refactor: extract shared parameterised test factory for log sub-commands#2563
refactor: extract shared parameterised test factory for log sub-commands#2563
Conversation
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
There was a problem hiding this comment.
Pull request overview
This PR refactors the duplicated logs-stats and logs-summary unit tests by extracting their shared source-selection / formatting assertions into a reusable test factory. It fits the codebase’s existing command-test structure by keeping command-specific behavior tests local while centralizing the common log-command contract.
Changes:
- Added
createLogCommandTests<TOptions>()intest-helpers.test-utils.tsto register the shared test cases for log subcommands. - Replaced duplicated shared tests in
logs-stats.test.tswith a single factory call plus the stats-specific logging-behavior test. - Replaced duplicated shared tests in
logs-summary.test.tswith a single factory call plus the summary-specific markdown/logging tests.
Show a summary per file
| File | Description |
|---|---|
src/commands/test-helpers.test-utils.ts |
Adds the shared parameterized Jest test factory for common log-command behaviors. |
src/commands/logs-summary.test.ts |
Switches summary tests to the shared factory and retains summary-specific assertions. |
src/commands/logs-stats.test.ts |
Switches stats tests to the shared factory and retains stats-specific logging assertions. |
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: 0
🔬 Smoke Test Results
PR: "refactor: extract shared parameterised test factory for log sub-commands" — author Overall: PARTIAL — MCP ✅, pre-computed test data unavailable (workflow template substitution did not occur).
|
Smoke Test: Copilot BYOK (Offline Mode)
Running in BYOK offline mode ( PR: "refactor: extract shared parameterised test factory for log sub-commands" by Overall: PARTIAL — BYOK inference ✅, pre-step smoke data was not interpolated (template expressions unevaluated).
|
🧪 Chroot Version Comparison Results
Overall: ❌ Not all tests passed — Python and Node.js versions differ between host and chroot environments.
|
Smoke Test Results
Overall: FAIL —
|
|
api-proxy: inject X-Initiator: agent default on all Copilot-bound requests to prevent billing inflation 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 Results ✅ PASS Recent merged PRs (last 2):
Tests:
|
logs-stats.test.tsandlogs-summary.test.tswere structurally near-identical — ~160 lines duplicated across 6 clone pairs, differing only in the command under test and its default format.Changes
test-helpers.test-utils.ts— addscreateLogCommandTests<TOptions>(): a generic factory that registers the 6 common test cases (source discovery, explicit source, no-source error, invalid-source error, aggregation error, format forwarding) under their owndescribeblock. AcceptscommandName,runCommand,defaultFormat, and amakeOptionsfactory for typed option construction.logs-stats.test.ts— reduced to acreateLogCommandTests(...)call + one stats-specificdescribecovering the info-log suppression behaviour (logs forpretty/markdown, suppresses forjson).logs-summary.test.ts— reduced to acreateLogCommandTests(...)call + onedescribecovering the two summary-specific cases: markdown-default assertion and stricter info-log suppression (onlyprettylogs;markdownandjsonare both silent).Command-specific edge cases remain in their own
describeblocks. All 15 original tests continue to pass.