Skip to content

test(nvx): gate Linux-specific cases - #8815

Merged
lpcox merged 2 commits into
mainfrom
fix/nvx-macos-tests
Sep 20, 2026
Merged

lpcox merged 2 commits into
mainfrom
fix/nvx-macos-tests

Conversation

@lpcox

@lpcox lpcox commented Sep 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • gate NVX filesystem staging tests that require Linux descriptor traversal
  • gate the NVX process-group escalation test that requires /proc enumeration
  • preserve all platform-independent NVX test coverage on macOS and other hosts

Validation

  • npm test -- --runInBand (377 suites passed; 6046 tests passed, 4 Linux-only tests skipped)
  • npm run type-check
  • npm run lint -- --quiet

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings September 20, 2026 18:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The mixed filesystem test must be split so platform-independent constructor validation remains covered on non-Linux hosts.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Gates NVX tests that depend on Linux-specific /proc and descriptor traversal behavior.

Changes:

  • Adds reusable linuxIt test guards.
  • Skips four Linux-dependent NVX tests on other platforms.
  • One mixed test also unintentionally skips platform-independent validation.
File Description
src/​nvx/​one-shot-adapter.test.ts Gates /proc-dependent process-group escalation test.
src/​nvx/​filesystem-builder.test.ts Gates descriptor-traversal filesystem tests.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/nvx/filesystem-builder.test.ts Outdated
});

it('rejects escaping symlinks, duplicate roles, and unsafe run identifiers', async () => {
linuxIt('rejects escaping symlinks, duplicate roles, and unsafe run identifiers', async () => {
@github-actions

github-actions Bot commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 93.28% 93.29% 📈 +0.01%
Statements 91.95% 91.97% 📈 +0.02%
Functions 92.00% 92.00% ➡️ +0.00%
Branches 85.78% 85.79% 📈 +0.01%
📁 Per-file Coverage Changes (1 files)
File Lines (Before → After) Statements (Before → After)
src/log-directory-setup.ts 96.2% → 100.0% (+3.78%) 96.3% → 100.0% (+3.71%)

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

@lpcox

lpcox commented Sep 20, 2026

Copy link
Copy Markdown
Collaborator Author

@copilot address review feedback

Copilot AI commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

@copilot address review feedback

Addressed in da013f5: split the mixed test into a platform-independent it covering the constructor-only assertions (unsafe run id, duplicate role) and a linuxIt limited to the escaping-symlink prepare() check.

@github-actions

Copy link
Copy Markdown
Contributor

✅ Copilot review passed with no inline comments.

@lpcox Add the ready-for-aw label to this PR to trigger agentic CI smoke tests.

@lpcox
lpcox merged commit 8de5c9c into main Sep 20, 2026
25 of 26 checks passed
@lpcox
lpcox deleted the fix/nvx-macos-tests branch September 20, 2026 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants