Skip to content

Stop tests from clobbering the OS clipboard#604

Merged
drn merged 2 commits into
masterfrom
argus/copied-clipboard-regularly
May 16, 2026
Merged

Stop tests from clobbering the OS clipboard#604
drn merged 2 commits into
masterfrom
argus/copied-clipboard-regularly

Conversation

@drn
Copy link
Copy Markdown
Owner

@drn drn commented May 16, 2026

A TUI test (TestCopyStagedClipboard_ClearError_LoggedNotPanicked) set
app.clipboardPending = "x" and called copyStagedClipboard(), which
shelled to pbcopy and wrote "x" to the developer's real macOS
clipboard every test run. TestApp_CopyToClipboard did the same with
"hello".

Inject the OS clipboard writer as a function field on App so tests
can swap in a no-op:

  • App.clipboardWriter func(string) error, set by New() to
    pbcopyWriter. Production path unchanged.
  • pbcopyWriter moved to internal/tui/clipboard_pbcopy.go and added
    to coverage-ignore.txt (privileged/external-resource policy).
  • The three tests that exercise the copy path inject a no-op writer.
  • New TestCopyToClipboard_WriterError covers the early-return branch
    when the writer fails.
  • Field-level doc on App.clipboardWriter explicitly states that test
    authors who skip the override will silently clobber the real
    clipboard.

Verified: pbpaste survives go test -race -count=1 ./internal/tui/
unchanged across multiple consecutive runs.

Co-Authored-By: Claude noreply@anthropic.com

drn and others added 2 commits May 15, 2026 18:51
TestCopyStagedClipboard_ClearError_LoggedNotPanicked set
clipboardPending = "x" and called copyStagedClipboard, which shelled to
pbcopy and wrote "x" to the developer's real macOS clipboard on every
run. TestApp_CopyToClipboard did the same with "hello".

Inject the OS clipboard writer via App.clipboardWriter so tests can swap
in a no-op. The production path (New() -> pbcopyWriter) is unchanged.
pbcopyWriter lives in its own file so it can be cleanly excluded from
the coverage gate as privileged/external-resource glue.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Drop redundant header.Notice() assertion in TestCopyToClipboard_WriterError
  (the goroutine never writes header on the error path, so the read was a
  no-op; removing it sidesteps any theoretical -race concern).
- Clarify nil-panic location in App.clipboardWriter doc — the panic surfaces
  inside copyToClipboard's goroutine.
- Update copyStagedClipboard doc to say "via the configured clipboard writer"
  instead of "via pbcopy" (now goes through the injection seam).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@drn drn merged commit 82750a2 into master May 16, 2026
@github-actions
Copy link
Copy Markdown

Merging this branch will increase overall coverage

Impacted Packages Coverage Δ 🤖
github.com/drn/argus/internal/tui 81.51% (+0.08%) 👍

Coverage by file

Changed files (no unit tests)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/drn/argus/internal/tui/app.go 68.35% (ø) 1627 1112 515
github.com/drn/argus/internal/tui/clipboard.go 92.50% (+16.89%) 40 (-1) 37 (+6) 3 (-7) 🎉
github.com/drn/argus/internal/tui/clipboard_pbcopy.go 0.00% (ø) 3 (+3) 0 3 (+3)

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

Changed unit test files

  • github.com/drn/argus/internal/tui/app_test.go
  • github.com/drn/argus/internal/tui/clipboard_test.go

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