Skip to content

fix: use module-level mock for fs.existsSync in config-writer test#3766

Merged
lpcox merged 1 commit into
mainfrom
fix/config-writer-test-existsSync
May 25, 2026
Merged

fix: use module-level mock for fs.existsSync in config-writer test#3766
lpcox merged 1 commit into
mainfrom
fix/config-writer-test-existsSync

Conversation

@lpcox
Copy link
Copy Markdown
Collaborator

@lpcox lpcox commented May 25, 2026

Problem

The config-writer.test.ts seccomp test (added in #3739) uses jest.spyOn(fs, 'existsSync'), which fails on Node 22+ with:

TypeError: Cannot redefine property: existsSync

This is because the file already uses jest.mock('fs') with a spread of requireActual('fs') — the resulting mock object has non-configurable properties that jest.spyOn cannot redefine.

The file's own comment on line 9 notes this exact issue for chownSync, but the bot-generated test in #3739 didn't follow the same pattern for existsSync.

Fix

  • Add existsSync to the jest.mock('fs') factory (with passthrough to the real implementation by default)
  • Replace jest.spyOn(fs, 'existsSync') with mockImplementation() on the pre-existing mock

Verification

All 18 tests in config-writer.test.ts pass locally on Node 24.

Fixes #3761

fs.existsSync is non-configurable on Node 22+ and cannot be overridden
with jest.spyOn when the fs module is already mocked via jest.mock().
This caused TypeError: Cannot redefine property: existsSync in CI.

Fix: add existsSync to the jest.mock('fs') factory (with passthrough to
the real implementation by default), matching the existing pattern for
chownSync. The seccomp test now uses mockImplementation() on the
pre-existing mock instead of jest.spyOn().

Fixes #3761

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 25, 2026 15:07
@github-actions
Copy link
Copy Markdown
Contributor

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 96.41% 96.58% 📈 +0.17%
Statements 96.26% 96.42% 📈 +0.16%
Functions 97.99% 97.99% ➡️ +0.00%
Branches 90.80% 90.92% 📈 +0.12%
📁 Per-file Coverage Changes (1 files)
File Lines (Before → After) Statements (Before → After)
src/config-writer.ts 85.1% → 90.9% (+5.78%) 85.1% → 90.9% (+5.78%)

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

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 fixes src/config-writer.test.ts failing on Node 22+ by avoiding jest.spyOn(fs, 'existsSync') on a non-configurable fs.existsSync export when fs is already being partially mocked. The test now uses a module-level existsSync mock (defaulting to passthrough) and overrides it via mockImplementation() within the seccomp test.

Changes:

  • Extend the existing module-level jest.mock('fs', ...) to include a passthrough existsSync wrapper.
  • Replace jest.spyOn(fs, 'existsSync') with mockImplementation() against the pre-defined mock, restoring the original implementation afterward.
Show a summary per file
File Description
src/config-writer.test.ts Switches existsSync mocking to a module-level jest mock to avoid Node 22+ “Cannot redefine property” errors in the seccomp-profile test.

Copilot's findings

Tip

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

  • Files reviewed: 1/1 changed files
  • Comments generated: 0

@github-actions
Copy link
Copy Markdown
Contributor

✅ Smoke test validation passed

  • ✅ GitHub API: 2 recent PRs verified
  • ✅ Playwright check: PASS
  • ✅ File verification: Claude engine validation successful

All checks passed.

💥 [THE END] — Illustrated by Smoke Claude

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test Results: Copilot BYOK Offline Mode

Author: @lpcox | PR: #3766

✅ GitHub MCP: Retrieved PR #3766 ("fix: use module-level mock for fs.existsSync in config-writer test")
❌ File Write/Read: /tmp/smoke-test-file.txt not found
✅ GitHub.com Connectivity: (pre-step verified)
✅ BYOK Inference: Agent successfully responding via api-proxy → api.githubcopilot.com

Mode: COPILOT_OFFLINE=true (BYOK offline mode)

Overall: ❌ FAIL (file test failed)

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Codex: FAIL
Merged PRs: ✅ refactor: extract tryMiddlePowerFallback helper in model-resolver; refactor: deduplicate provider API target/basePath env configuration
safeinputs-gh PR query: ❌ tool unavailable; gh fallback returned PR titles
Playwright title: ✅ GitHub
Tavily search: ❌ no Tavily tools exposed
File/bash: ✅ smoke-test-codex-26407152620.txt
Discussion query/comment: ❌ github-discussion-query unavailable; oracle comment posted via fallback
Build: ✅ npm ci && npm run 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

Chroot Runtime Version Comparison

The chroot environment test compared runtime versions between the host and chroot:

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

Result: Tests did not pass. Python and Node.js versions differ between host and chroot environments.

Tested by Smoke Chroot

@github-actions
Copy link
Copy Markdown
Contributor

Gemini Smoke Test Results

  • GitHub MCP Testing: ❌ (Command mcpscripts not found)
  • GitHub.com Connectivity: ❌ (SSL error 35)
  • File Writing Testing: ✅
  • Bash Tool Testing: ✅

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: Services Connectivity

Status: ❌ FAIL

  • ❌ Redis (host.docker.internal:6379) — connection timeout
  • ❌ PostgreSQL (host.docker.internal:5432) — no response

Services running on the host are not reachable from the AWF sandbox.

🔌 Service connectivity validated by Smoke Services

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test Results

✅ GitHub MCP: PR #3755 (refactor: extract tryMiddlePowerFallback helper)
❌ GitHub connectivity: No HTTP status provided
❌ File test: /tmp/smoke-test.txt not found

Status: FAIL

cc @lpcox

📰 BREAKING: Report filed by Smoke Copilot

@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 1/1 passed ✅ PASS
Node.js execa 1/1 passed ✅ PASS
Node.js p-limit 1/1 passed ✅ PASS
Rust fd 1/1 passed ✅ PASS
Rust zoxide 1/1 passed ✅ PASS

Overall: 8/8 ecosystems passed — ✅ PASS

All build and test operations completed successfully across all ecosystems.

Generated by Build Test Suite for issue #3766 · ● 14.8M ·

@lpcox lpcox merged commit d3d2199 into main May 25, 2026
63 of 65 checks passed
@lpcox lpcox deleted the fix/config-writer-test-existsSync branch May 25, 2026 15:31
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.

[aw] Test Coverage Reporter failed

2 participants