Skip to content

fix: prevent tests from spawning real OS processes - #25

Merged
jongio merged 1 commit into
mainfrom
fix/test-side-effects
Mar 21, 2026
Merged

jongio merged 1 commit into
mainfrom
fix/test-side-effects

Conversation

@jongio

@jongio jongio commented Mar 21, 2026

Copy link
Copy Markdown
Owner

Problem

Running go test ./... caused real OS-level side effects on Windows:

  1. Copilot CLI terminal window — TestEnsureStarted_SetsStartedFlag used copilot.NewClient(nil) which launches the real Copilot CLI process
  2. Windows error dialog ("Windows cannot find...") — TestOpenInEditor_PlatformDefaultWindows spawns cmd /c start
  3. cmd.exe terminal window — TestOpenInTerminal_WindowsPlatform spawns cmd /c start cmd /k
  4. Browser tab to 127.0.0.1:1 — TestOpenInBrowser_WindowsPlatform spawns rundll32

Fix

Copilot provider test (internal/ai/provider_copilot_test.go)

  • Use copilot.NewClient(&copilot.ClientOptions{CLIPath: "/nonexistent/copilot-cli"}) so the SDK fails immediately without spawning a real process
  • Added assert.Error to verify the expected failure

Platform tests (internal/panels/)

  • Converted startDetached from a function to a package-level var startDetachedFn — standard Go pattern for test dependency injection
  • Updated all 10+ call sites from startDetached( to startDetachedFn(
  • Rewrote platform tests to use stubStartDetachedCapture(t) helper that captures the *exec.Cmd without launching it
  • Tests now assert on captured command args (e.g., contains "rundll32", "start", "cmd")
  • t.Cleanup restores the original function after each test

Verification

  • All 49 test packages pass with zero side effects
  • go build ./... clean
  • go vet ./... clean

- Use fake CLIPath in CopilotProvider test to avoid launching copilot CLI
- Make startDetached a swappable var (startDetachedFn) for test injection
- Rewrite platform tests to capture exec.Cmd without spawning processes
- Eliminates terminal windows, error dialogs, and browser tabs during testing

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jongio
jongio force-pushed the fix/test-side-effects branch from 110f662 to 86bebe4 Compare March 21, 2026 15:07
@jongio
jongio merged commit 92a51f6 into main Mar 21, 2026
2 checks passed
@jongio
jongio deleted the fix/test-side-effects branch March 21, 2026 15:19
jongio added a commit that referenced this pull request May 16, 2026
Squash merge of swarm/2026-05-15 branch resolving 55 GitHub issues.

Closes #4, #5, #6, #7, #8, #9, #10, #11, #12, #13, #14, #15, #16, #17, #18, #19, #20, #21, #22, #23, #24, #25, #26, #27, #28, #29, #30, #31, #32, #33, #34, #35, #36, #37, #38, #39, #40, #41, #42, #43, #44, #45, #46, #47, #48, #49, #50, #51, #52, #53, #54, #55, #56, #57, #58

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

1 participant