Skip to content

fix(b-0528): gate plutil tests on darwin (PR #3423 cross-platform follow-up)#3426

Merged
AceHack merged 1 commit into
mainfrom
fix/b0528-gate-plutil-tests-darwin-otto-cli-2026-05-15
May 15, 2026
Merged

fix(b-0528): gate plutil tests on darwin (PR #3423 cross-platform follow-up)#3426
AceHack merged 1 commit into
mainfrom
fix/b0528-gate-plutil-tests-darwin-otto-cli-2026-05-15

Conversation

@AceHack
Copy link
Copy Markdown
Member

@AceHack AceHack commented May 15, 2026

Summary

Post-merge fix for PR #3423. Two P0/P1 portability findings:

  • Codex P1: plutilLint called in-process hard-exits the test runner on non-darwin (ENOENT → process.exit(1)).
  • Copilot P0: subprocess plutil calls fail on non-darwin (line 98 + 167 + 229), breaking cross-platform test runs.

Both real. plutil is macOS-only, and the install-launchagent.ts target (launchd) is also macOS-only — so the pure-helper tests should run cross-platform but the plutil integration tests should skip.

Approach

const IS_DARWIN = process.platform === "darwin";
const itDarwin = IS_DARWIN ? it : it.skip;

Applied itDarwin to the 4 plutil-touching test cases. Pure helpers (xmlEscape, substitutePlaceholders, requireAbsolute, tryDetect, argument-validation via subprocess) remain it for cross-platform coverage.

Expected results

Platform Pass Skip Fail
macOS 19 0 0
Linux 15 4 0

Verified on macOS: 19/19 pass.

Test plan

  • bunx tsc --noEmit clean
  • bun test on macOS: 19 pass / 0 fail (unchanged)
  • CI green on Linux runners
  • Auto-merge fires

🤖 Generated with Claude Code

Two cross-platform portability findings from PR #3423 review:

- Codex P1 line 234: `plutilLint` called in-process hard-exits the
  test runner on non-darwin hosts when `plutil` is missing (ENOENT
  → plutilLint's catch calls process.exit(1)).
- Copilot P0 line 98 + 167 + 229: subprocess `plutil` calls fail on
  non-darwin hosts, breaking the cross-platform test suite.

Both real. `plutil` is a macOS-only system binary, and the
LaunchAgent install path the test file exercises is also macOS-only
(launchd is macOS). The pure-helper tests (xmlEscape,
substitutePlaceholders, requireAbsolute, tryDetect,
argument-validation via subprocess) run cross-platform.

Fix: introduce `const IS_DARWIN = process.platform === "darwin"` +
`const itDarwin = IS_DARWIN ? it : it.skip` and route the 4
plutil-touching test cases through `itDarwin`:

- "substituted values containing & < > produce plutil-valid plist"
- "writes rendered plist to stdout, not to ~/Library/LaunchAgents/"
- "returns without throwing for a valid plist" (plutilLint)
- "exits with code 1 (via subprocess) for an invalid plist" (plutilLint)

Result on macOS: 19 pass / 0 fail (unchanged).
Result on Linux (expected): 15 pass / 4 skip / 0 fail (suite remains
green and the rest of the suite is reachable).

Co-Authored-By: Claude <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 15, 2026 11:08
@AceHack AceHack enabled auto-merge (squash) May 15, 2026 11:08
Copy link
Copy Markdown

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

Post-merge cross-platform follow-up to PR #3423: gates the four plutil-dependent tests in install-launchagent.test.ts to darwin only, so non-darwin CI runners don't fail (subprocess plutil ENOENT) or hard-exit (in-process plutilLint calls process.exit(1)). Pure-helper tests remain cross-platform.

Changes:

  • Introduces IS_DARWIN / itDarwin (skip-on-non-darwin variant of it).
  • Applies itDarwin to the 4 plutil-touching cases (xmlEscape integration, --dry-run, plutilLint valid, plutilLint invalid).
  • Adds a comment explaining the macOS-only scope.

@AceHack AceHack merged commit 5aeaffb into main May 15, 2026
29 checks passed
@AceHack AceHack deleted the fix/b0528-gate-plutil-tests-darwin-otto-cli-2026-05-15 branch May 15, 2026 11:11
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.

2 participants