Skip to content

test(cli): stop two tests from depending on the developer's setup - #2039

Merged
clay-good merged 1 commit into
Fission-AI:mainfrom
vyhuholl:test/isolate-from-dev-environment
Oct 5, 2026
Merged

clay-good merged 1 commit into
Fission-AI:mainfrom
vyhuholl:test/isolate-from-dev-environment

Conversation

@vyhuholl

@vyhuholl vyhuholl commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Closes #2038

What this changes

Two test files read things from the developer's machine that CI doesn't have, so three tests failed locally and passed on CI.

  • completion-tip.test.ts now clears $ZSH and $ZSH_CUSTOM in beforeEach, like zsh-installer.test.ts already does. With Oh My Zsh exported, the zsh installer found the developer's real completions under $ZSH and retired the tip. afterEach already restores the whole environment, so nothing else changes.
  • The workset launch-failure test removes every PATH directory that holds a claude, claude.exe or claude.cmd before putting its broken fake first. Without that, the spawn failed with ENOENT on the fake and went on to the developer's real Claude Code.

Only test files change.

How you verified it

  • Before, on main (2500d6d): pnpm test gave 6,400 passed and 3 failed (the three tests in Three tests fail locally when Oh My Zsh or Claude Code is installed #2038).
  • After: workset.test.ts and completion-tip.test.ts pass 60/60, and the full pnpm test passes 6,403/6,403. pnpm build, pnpm exec tsc --noEmit and pnpm lint pass.
  • Run on macOS only. The workset test already passed on CI, so its runners have no other claude on PATH and the filter removes nothing there.

Notes

  • No changeset: users see no change.
  • Written with Claude Code, model Claude Opus 5.5.

  • Ran pnpm changeset if this affects users, and committed the file
  • If a coding agent wrote this, named the agent and model in the Notes section, and verified the result myself

Summary by CodeRabbit

  • Tests
    • Improved the reliability of checks for failed tool launches by ensuring they use the intended test environment.
    • Reduced interference from existing shell configuration during completion detection checks, helping keep test results consistent across different development environments.

completion-tip sandboxes HOME but not $ZSH. Oh My Zsh exports $ZSH, the
zsh installer looks for completions under it, finds a real install and
retires the tip, so two tests failed on such machines. Clear $ZSH and
$ZSH_CUSTOM the way zsh-installer.test.ts already does.

The workset launch-failure test puts a claude with a missing interpreter
first on PATH. Spawning it fails with ENOENT and the PATH search moves on
to the next entry, so with Claude Code installed the real one ran
instead of failing. Drop the directories that hold a claude from PATH in
that test.
@vyhuholl
vyhuholl requested a review from a team as a code owner October 4, 2026 22:12
@vyhuholl
vyhuholl requested review from clay-good and removed request for a team October 4, 2026 22:12
@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: Fission-AI/OpenSpec/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 2882271b-3ac5-402b-9a1e-656874c1e64d
📥 Commits

Reviewing files that changed from the base of the PR and between 2500d6d and 78a2c0f.

📒 Files selected for processing (2)
  • test/commands/workset.test.ts
  • test/core/completion-tip.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

Two tests now isolate their environments from local Claude Code and Oh My Zsh installations. The changes adjust PATH and clear ZSH-related environment variables.

Changes

Test environment isolation

Layer / File(s) Summary
Isolate tests from local installations
test/commands/workset.test.ts, test/core/completion-tip.test.ts
The workset test removes PATH entries containing Claude executables before adding its broken fake tool. The completion-tip test clears ZSH and ZSH_CUSTOM after setting its temporary home. These changes prevent the tests from using local installations or configuration.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to 78a2c

These changes isolate two tests from local machine settings without changing product behavior; no concrete merge-blocking risk is identified.

Architecture Summary

Architecture risk: 🔵 Low · up to 78a2c

The change affects 1 system.

Changed systems: test

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — test (service) was modified; 2 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in test/commands/workset.test.ts: The test now filters out PATH directories containing claude, claude.exe, or claude.cmd before prepending the broken fake-tool directory. Previously, it only prepended that directory, allowing PATH search to continue to a real Claude executable if the fake failed with ENOENT.
  • observed — Modified behavior in test/core/completion-tip.test.ts: The test setup now deletes ZSH and ZSH_CUSTOM after configuring the temporary home, in addition to the existing environment sandbox.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Issue #2038 requires the affected tests to behave independently of installed Oh My Zsh and Claude Code. completion-tip.test.ts clears ZSH and ZSH_CUSTOM; workset.test.ts removes PATH directori…
Out of Scope Changes check ✅ Passed The changes are limited to the tests named in issue #2038. Both changes isolate tests from developer-machine environment state, which directly supports the issue objective. No unrelated changes are re…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: making two tests independent of the developer’s local setup.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@clay-good
clay-good added this pull request to the merge queue Oct 5, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 5, 2026
@clay-good clay-good changed the title test: stop two tests from depending on the developer's setup test(cli): stop two tests from depending on the developer's setup Oct 5, 2026
@clay-good
clay-good added this pull request to the merge queue Oct 5, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 5, 2026

@alfred-openspec alfred-openspec left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approved. These are narrowly scoped test-isolation fixes: shell-completion tests no longer inherit Oh My Zsh paths, and the launch-failure test cannot fall through to a developer's real Claude binary. The two targeted files pass 60 tests locally, and full cross-platform CI is green.

@clay-good
clay-good added this pull request to the merge queue Oct 5, 2026
Merged via the queue into Fission-AI:main with commit 69cf0a9 Oct 5, 2026
15 checks passed
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.

Three tests fail locally when Oh My Zsh or Claude Code is installed

3 participants