Skip to content

refactor(tests): extract setupDefaultIptablesMocks to eliminate repeated iptables mock sequences#3140

Merged
lpcox merged 2 commits into
mainfrom
copilot/duplicate-code-fix-iptables-mock
May 14, 2026
Merged

refactor(tests): extract setupDefaultIptablesMocks to eliminate repeated iptables mock sequences#3140
lpcox merged 2 commits into
mainfrom
copilot/duplicate-code-fix-iptables-mock

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 14, 2026

✨ Enhancement

26 occurrences of a 3–5 line mockedExeca setup sequence (bridge name lookup → permission check → chain existence check → catch-all) were copy-pasted across host-iptables-setup.test.ts and host-iptables-host-access.test.ts.

What does this improve?

  • Eliminates ~200 lines of duplicated mock boilerplate across the two test files
  • Future changes to the iptables setup call sequence only require updating one place

Implementation approach:

Added setupDefaultIptablesMocks to the existing shared src/test-helpers/host-iptables-test-setup.ts:

export function setupDefaultIptablesMocks(
  opts: { chainExists?: boolean; bridgeName?: string; catchAllStdout?: string } = {}
): void {
  const { chainExists = false, bridgeName = 'fw-bridge', catchAllStdout = '' } = opts;
  mockedExeca
    .mockResolvedValueOnce(execaResult({ stdout: bridgeName, exitCode: 0 }))
    .mockResolvedValueOnce(execaResult({ stdout: '', exitCode: 0 }))
    .mockResolvedValueOnce(execaResult({ exitCode: chainExists ? 0 : 1 }));
  mockedExeca.mockResolvedValue(execaResult({ stdout: catchAllStdout, exitCode: 0 }));
}
  • chainExists — controls whether the FW_WRAPPER chain existence check returns 0 or 1
  • catchAllStdout — overrides the default catch-all stdout for tests that need specific listing output (e.g. skip-jump-rule test)
  • Tests that follow with mockImplementation naturally override the catch-all since mockImplementation takes precedence over mockResolvedValue

Dropped the now-unused execaResult import from host-iptables-host-access.test.ts.

Copilot AI changed the title [WIP] Refactor duplicate mockedExeca mock sequence in iptables tests refactor(tests): extract setupDefaultIptablesMocks to eliminate repeated iptables mock sequences May 14, 2026
Copilot finished work on behalf of lpcox May 14, 2026 13:38
Copilot AI requested a review from lpcox May 14, 2026 13:38
@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
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

Refactors duplicated iptables mock setup sequences across two test files into a shared setupDefaultIptablesMocks helper to reduce ~200 lines of boilerplate.

Changes:

  • Adds setupDefaultIptablesMocks helper to src/test-helpers/host-iptables-test-setup.ts with configurable chainExists, bridgeName, and catchAllStdout options.
  • Replaces 26 inline mockedExeca setup sequences in host-iptables-setup.test.ts and host-iptables-host-access.test.ts with helper calls.
  • Removes unused execaResult import from host-iptables-host-access.test.ts.
Show a summary per file
File Description
src/test-helpers/host-iptables-test-setup.ts New shared helper consolidating bridge/permission/chain-existence mock sequence.
src/host-iptables-setup.test.ts Replaces inline mock sequences with helper calls.
src/host-iptables-host-access.test.ts Replaces inline mock sequences with helper calls; drops unused import.

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

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test Results

Test Result Details
GitHub API (List 2 merged PRs) gh CLI authentication failed (not pre-authenticated)
Playwright (GitHub title check) Successfully navigated to github.com, title contains 'GitHub'
File Verify (smoke-test file) File exists with valid content: 'Smoke test passed for Claude...'

Overall: FAIL (1 of 3 tests failed)

The GitHub API test failure is expected per documentation: gh CLI is not authenticated. The two operational tests (Playwright navigation and file verification) both passed successfully.

💥 [THE END] — Illustrated by Smoke Claude

@github-actions
Copy link
Copy Markdown
Contributor

🔬 Smoke Test Results

Test Status
GitHub MCP connectivity ❌ 401 Bad credentials
GitHub.com HTTP connectivity ❌ Pre-step data not substituted
File write/read ❌ Pre-step data not substituted

Overall: FAIL

The workflow template variables (${{ steps.smoke-data.outputs.* }}) were not expanded before reaching the agent, so pre-computed test data was unavailable. GitHub MCP also returned 401.

📰 BREAKING: Report filed by Smoke Copilot

@github-actions github-actions Bot mentioned this pull request May 14, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test: Copilot BYOK (Offline) Mode

Test Result
GitHub MCP connectivity ❌ (401 Bad credentials)
GitHub.com HTTP connectivity ⚠️ (template var not expanded)
File write/read ⚠️ (template var not expanded)
BYOK inference (api-proxy → api.githubcopilot.com)

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

Overall: FAIL — workflow template variables (${{ steps.smoke-data.outputs.* }}) were not substituted; GitHub MCP returned 401.

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test Results: File Writing ✅, Bash Tool ✅, GitHub MCP ❌, Connectivity ❌. Overall Status: 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

Smoke test: FAIL
Merged PRs: fix: skip node --version check under QEMU emulation in agent Dockerfile; fix: remove unused public exports and normalize test-only access
❌ GitHub safe-input/MCP: safeinputs-gh and github-discussion-query unavailable; gh returned 401
✅ Playwright: GitHub title verified
❌ Tavily: no search tool exposed
✅ File/Bash: temp file created and read
✅ Discussion: #191 commented
✅ Build: npm ci && npm run build

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

🏗️ 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 #3140 · ● 4.3M ·

@github-actions
Copy link
Copy Markdown
Contributor

Chroot Version Comparison — Smoke Test Results

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

Result: FAILED — Python and Node.js versions differ between host and chroot. The chroot environment uses the system-installed binaries (Ubuntu 22.04 defaults), while the host has newer versions installed separately.

Tested by Smoke Chroot

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test Results — FAIL

Check Result
Redis PING (host.docker.internal:6379) ❌ Connection refused / timeout
PostgreSQL pg_isready (host.docker.internal:5432) ❌ No response
PostgreSQL SELECT 1 ❌ No response

host.docker.internal resolves to 172.17.0.1 but no services are listening on ports 6379 or 5432. The GitHub Actions service containers appear to not be running or not accessible from this environment.

Overall: FAIL

🔌 Service connectivity validated by Smoke Services

@lpcox lpcox merged commit a6599d3 into main May 14, 2026
68 of 74 checks passed
@lpcox lpcox deleted the copilot/duplicate-code-fix-iptables-mock branch May 14, 2026 14:36
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] host-iptables-setup.test.ts contains repeated mock sequence for iptables chain setup

3 participants