refactor: split review_test.go into focused test files#379
Merged
Conversation
Extract mockEnqueue and mockWaitableReview helpers to eliminate duplicated mock setup across 8+ tests. Replace unnecessary channel-based captures with direct variable reads (executeReviewCmd is synchronous). Consolidate 5 flag-validation subtests into a single table-driven TestReviewFlagValidation. Remove unused "time" import. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add review_test_helpers_test.go with reusable testModelOption functions (withReview, withFixPanel, withFixPrompt, etc.), semantic assertion helpers (assertFixPanelOpen/Closed, assertFixPanelState), and a mockReviewHandler factory. Rewrite fix panel tests to use initTestModel with option functions and pressKey/pressSpecial instead of raw handleKeyMsg calls. Replace withModelBranch with withBranchName and update setupRenderModel to accept testModelOption. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Split the 3,067-line review_test.go into files by feature area, matching the existing pattern (filter_test.go, queue_test.go, etc.): - review_fetch_test.go: fetch review + SHA fallback (4 tests) - review_navigation_test.go: review nav + selection sync (7 tests) - review_branch_test.go: branch name set/clear/filter (5 tests) - review_addressed_test.go: addressed toggle + rollback (7 tests) - review_views_test.go: escape/commit msg/help views (8 tests) - review_clipboard_test.go: yank/copy + formatClipboardContent (13 tests) - review_log_test.go: log paging/loading/nav (10 tests) - review_fix_panel_test.go: fix panel open/close/submit (17 tests) - review_render_test.go: render views + visible lines (3 tests) Pure file-move refactor — no test logic changed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
roborev: Combined Review (
|
mockEnqueue wrote to a shared capturedEnqueue struct in the HTTP handler goroutine and returned a pointer for tests to read without synchronization, tripping -race. The json.Decode error was also silently discarded. Replace the shared pointer with a buffered channel: the handler decodes (checking the error), sends the value, then responds. Tests receive from the channel after executeReviewCmd returns, giving a clean happens-before edge. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
roborev: Combined Review (
|
Use a non-blocking channel send so a second enqueue call fails the test immediately instead of blocking forever. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
roborev: Combined Review (
|
Collaborator
Author
|
Not valid. Merging |
wesm
added a commit
that referenced
this pull request
Feb 27, 2026
Split the 2,487-line sync_test.go (36 tests) into 7 focused files: - sync_test_helpers_test.go: shared helpers, legacy DDL, syncTestHelper - sync_state_test.go: sync state and worker lifecycle (6 tests) - sync_backfill_test.go: machine ID and repo identity backfill (6 tests) - sync_identity_test.go: repo identity CRUD and schema migrations (8 tests) - sync_queries_test.go: sync query functions and timestamps (5 tests) - sync_ordering_test.go: sync ordering and dependency enforcement (8 tests) - sync_remap_test.go: patch ID round-trip and job remapping (4 tests) No test logic changes. Follows the pattern set by review_test.go split (#379). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
4 tasks
wesm
added a commit
that referenced
this pull request
Feb 27, 2026
## Summary - Split `internal/storage/sync_test.go` (2,487 lines, 36 tests) into 7 focused files following the pattern set by the `review_test.go` split (#379) - One shared helpers file (`sync_test_helpers_test.go`) plus 6 feature-area test files: state, backfill, identity, queries, ordering, remap - No test logic changes — purely mechanical refactor ## Test plan - [x] `go build ./internal/storage/...` passes - [x] `go vet ./internal/storage/...` passes - [x] `go fmt ./internal/storage/...` clean - [x] `go test ./internal/storage/... -count=1` — all tests pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
review_test.gointo 9 focused test files by feature area, matching the existing pattern used for filter and queue testsinitTestModel, option functions, assertion helpers) intoreview_test_helpers_test.goNew files:
review_fetch_test.go,review_navigation_test.go,review_branch_test.go,review_addressed_test.go,review_views_test.go,review_clipboard_test.go,review_log_test.go,review_fix_panel_test.go,review_render_test.goTest plan
go test ./cmd/roborev/tui/... -count=1passesgo vet ./cmd/roborev/tui/...cleango fmt ./cmd/roborev/tui/...clean🤖 Generated with Claude Code