fix: reflect manual runs in status - #3
Conversation
![umpire-bot[bot]](https://avatars.githubusercontent.com/in/3739611?s=60&v=4){kind=small}
| } | ||
| } | ||
|
|
||
| func TestAtomicWriteNoTmpFile(t *testing.T) { |
There was a problem hiding this comment.
Test name no longer matches implementation (T2). The test at line 89–92 only verifies that path + ".tmp" never appears, which is now trivially always true. Rename to reflect the actual invariant (no stale temp files after atomic write) and consider asserting the status file content is correct.
| repo := setupRunRepo(t) | ||
| stateDir := paths.StateDir() | ||
| if err := os.MkdirAll(stateDir, 0755); err != nil { | ||
| t.Fatal(err) |
There was a problem hiding this comment.
Integration test uses removeStuckFlag (defined in unstick.go:66) as a test helper, creating a cross-file implementation dependency. If removeStuckFlag changes its error behavior or side effects, this test breaks even though it's testing runRepo behavior. Consider defining the helper directly in the test file or using only exported APIs.
There was a problem hiding this comment.
Declined. That's the point of an integration test!
| return repo | ||
| } | ||
|
|
||
| func TestRunAfterUnstickBypassesDebounceAndPreservesStatus(t *testing.T) { |
There was a problem hiding this comment.
TestRunAfterUnstickBypassesDebounceAndPreservesStatus is an expensive end-to-end test (multiple exec.Command("git", ...) calls, real git repos, filesystem state). The "preserves status" portion tests MergeAndWrite behavior, which could be verified with a cheaper unit test on the status package. The unstick + run interaction is valid to test end-to-end, but the test name and scope are overloaded.
There was a problem hiding this comment.
End to end tests are like that by nature
| } | ||
| } | ||
|
|
||
| func TestAtomicWriteNoTmpFile(t *testing.T) { |
There was a problem hiding this comment.
newerOrEqual (status.go:144) has no dedicated unit test. This is the core comparison function for the merge-and-write stale-snapshot protection. Its behavior — handling empty strings, invalid timestamps, and exact-equal timestamps — is only tested implicitly through TestMergeAndWritePreservesNewerForegroundResult. A dedicated unit test would catch regressions in the comparison logic without spinning up a full status file.
| return fmt.Errorf("sync failed (%s): %s", result.State, result.Error) | ||
| } | ||
|
|
||
| func recordRunStatus(statusPath, repoPath, mode string, result sync.SyncResult) error { |
There was a problem hiding this comment.
recordRunStatus (run.go:56) has no dedicated unit test. The function's "offline" and "stuck" switch branches are production code that modifies RepoStatus fields (sets LastError, increments ConsecutiveOfflineFailures, sets OfflineSince/StuckSince timestamps). Only the "ok" path is indirectly tested via the integration test. A unit test with a mock RepoStatus update function would verify each branch independently.
There was a problem hiding this comment.
Testing via an integration test is superior
1 participant
Summary
git tend runrequests bypass read-write debounce.git tend runcommands without allowing stale daemon snapshots to overwrite them.run .updates the scanned repository entry.Testing
go test ./...go test -race ./internal/status ./internal/sync ./cmd/git-tend ./internal/daemon ./internal/pathsgo vet ./...