Skip to content

Refactor log command tests to centralize empty-stats harness setup#3142

Merged
lpcox merged 3 commits into
mainfrom
copilot/duplicate-code-logs-tests
May 14, 2026
Merged

Refactor log command tests to centralize empty-stats harness setup#3142
lpcox merged 3 commits into
mainfrom
copilot/duplicate-code-logs-tests

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 14, 2026

✨ Enhancement

logs-stats.test.ts and logs-summary.test.ts contained the same 22-line empty-stats + mock wiring block, flagged by duplicate-code detection. This change consolidates that shared setup into the existing log command test utilities to reduce drift and keep both suites aligned.

  • What does this improve?

    • Removes duplicated test infrastructure from both command test files.
    • Makes the shared “no data / empty aggregation” harness setup a single source of truth.
  • Why is this valuable?

    • Future updates to this common setup now happen once instead of in multiple tests.
    • Keeps command-specific tests focused on behavior assertions rather than boilerplate wiring.
  • Implementation approach:

    • Added EMPTY_STATS (AggregatedStats) in src/commands/test-helpers.test-utils.ts.
    • Added setupEmptyStatsHarness(harness, source) in the same helper module.
    • Updated:
      • src/commands/logs-stats.test.ts
      • src/commands/logs-summary.test.ts
        to call the shared helper instead of duplicating inline setup.
// shared helper
export function setupEmptyStatsHarness(harness: LogCommandTestHarness, source: LogSource): void {
  harness.mockedDiscovery.discoverLogSources.mockResolvedValue([source]);
  harness.mockedDiscovery.selectMostRecent.mockReturnValue(source);
  harness.mockedAggregator.loadAndAggregate.mockResolvedValue(EMPTY_STATS);
  harness.mockedFormatter.formatStats.mockReturnValue('');
}

// in command tests
setupEmptyStatsHarness(harness, mockSource);

Copilot AI changed the title [WIP] Refactor duplicate setup in logs-stats.test.ts and logs-summary.test.ts Refactor log command tests to centralize empty-stats harness setup May 14, 2026
Copilot finished work on behalf of lpcox May 14, 2026 13:34
Copilot AI requested a review from lpcox May 14, 2026 13:34
@lpcox lpcox marked this pull request as ready for review May 14, 2026 13:38
@lpcox lpcox requested a review from Mossaka as a code owner May 14, 2026 13:38
Copilot AI review requested due to automatic review settings May 14, 2026 13:38
@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 duplicated empty-stats test setup for the log command test suites into shared command test utilities.

Changes:

  • Adds shared EMPTY_STATS and setupEmptyStatsHarness test helper.
  • Updates logs-stats.test.ts and logs-summary.test.ts to use the helper instead of duplicating mock setup.
Show a summary per file
File Description
src/commands/test-helpers.test-utils.ts Adds shared empty aggregation data and harness setup helper.
src/commands/logs-summary.test.ts Replaces inline empty-stats mock wiring with shared helper.
src/commands/logs-stats.test.ts Replaces inline empty-stats mock wiring with shared helper.

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: 1

Comment thread src/commands/test-helpers.test-utils.ts Outdated
export function setupEmptyStatsHarness(harness: LogCommandTestHarness, source: LogSource): void {
harness.mockedDiscovery.discoverLogSources.mockResolvedValue([source]);
harness.mockedDiscovery.selectMostRecent.mockReturnValue(source);
harness.mockedAggregator.loadAndAggregate.mockResolvedValue(EMPTY_STATS);
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented May 14, 2026

@copilot address review feedback

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 14, 2026

@copilot address review feedback

Addressed in commit dfdb2ca. setupEmptyStatsHarness now returns a fresh empty stats object on each loadAndAggregate mock invocation, preventing mutable state bleed across calls/tests.

Copilot finished work on behalf of lpcox May 14, 2026 13:50
@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test Results

❌ GitHub API — HTTP 401 authentication failed
✅ Playwright — github.com navigation confirmed, title verified
✅ File verify — smoke test file readable

Result: FAIL (2/3 tests passed)

💥 [THE END] — Illustrated by Smoke Claude

@github-actions
Copy link
Copy Markdown
Contributor

🔬 Smoke Test Results

Test Result
GitHub MCP connectivity ❌ (401 Bad credentials)
GitHub.com HTTP connectivity ❌ (template vars not substituted)
File write/read ❌ (template vars not substituted)

Overall: FAIL

Pre-computed step outputs (${{ steps.smoke-data.outputs.* }}) were not resolved — the workflow template variables were passed literally to the agent. GitHub MCP tools also returned 401 errors in this context.

📰 BREAKING: Report filed by Smoke Copilot

@github-actions
Copy link
Copy Markdown
Contributor

🔥 Smoke Test: Copilot BYOK (Offline) Mode

Test Result
GitHub MCP connectivity ❌ 401 Bad credentials (MCP unauthenticated)
GitHub.com HTTP connectivity ⚠️ Pre-step data not injected (template vars unexpanded)
File write/read smoke-test-copilot-byok-25863839607.txt verified
BYOK inference (api-proxy → api.githubcopilot.com) ✅ Responding now

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

Overall: PARTIAL PASS — BYOK inference and file I/O pass; GitHub MCP read requires auth not available in this environment.

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions
Copy link
Copy Markdown
Contributor

fix: skip node --version check under QEMU emulation in agent Dockerfile
fix: remove unused public exports and normalize test-only access
GitHub PR review: ✅
Safe Inputs GH CLI: ❌
Playwright title: ✅
Tavily search: ❌
File/bash: ✅
Discussion: ✅
Build: ✅
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

Smoke test results: FAIL

Warning

Firewall blocked 1 domain

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

  • localhost

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

network:
  allowed:
    - defaults
    - "localhost"

See Network Configuration for more information.

💎 Faceted by Smoke Gemini

@github-actions
Copy link
Copy Markdown
Contributor

🏗️ 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 #3142 · ● 11.9M ·

@github-actions
Copy link
Copy Markdown
Contributor

Chroot Smoke Test Results

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

Overall: ❌ FAILED — Python and Node.js versions differ between host and chroot.

Tested by Smoke Chroot

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test Results — FAIL

Check Result
Redis PING ❌ Timeout (no response)
PostgreSQL pg_isready ❌ No response on port 5432
PostgreSQL SELECT 1 ❌ Not attempted (pg_isready failed)

Overall: FAILhost.docker.internal is not reachable from this environment. Service containers appear unavailable.

🔌 Service connectivity validated by Smoke Services

@lpcox lpcox merged commit 33e338f into main May 14, 2026
64 of 68 checks passed
@lpcox lpcox deleted the copilot/duplicate-code-logs-tests branch May 14, 2026 14:37
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.

[Duplicate Code] logs-stats.test.ts and logs-summary.test.ts duplicate 22-line test setup infrastructure

3 participants