Skip to content

Commit 9d31f36

Browse files
MaxGhenisclaude
andauthored
Make checks green: vet lock-copies + flaky binary test (#29)
* Fix vet: stop copying BackfillProgress's mutex in snapshots snapshot() and GetBackfillProgress() copied the whole struct including its sync.Mutex. Split the data fields into a mutex-free BackfillSnapshot embedded in BackfillProgress, and return that from snapshot paths. go vet ./... is now clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Fix flaky pair-command binary test The test killed the process after 3s and then treated "exited non-zero with no output" as a crash — but the timer kill itself produces exactly that signature whenever pairing is quiet for 3s (it contacts Google before printing the QR, so slow or sandboxed networks hit this every time). Distinguish self-exit from grace-period kill: surviving the grace period now passes outright. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent ee53818 commit 9d31f36

2 files changed

Lines changed: 32 additions & 20 deletions

File tree

cmd/binary_test.go

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -48,23 +48,29 @@ func TestBuiltBinaryAcceptsPairCommand(t *testing.T) {
4848
t.Fatalf("failed to start binary: %v", err)
4949
}
5050

51-
// Wait briefly then kill — we just want to confirm it doesn't crash immediately
52-
timer := time.AfterFunc(3*time.Second, func() {
53-
cmd.Process.Kill()
54-
})
55-
defer timer.Stop()
56-
57-
err := cmd.Wait()
58-
output := out.String()
51+
// We only want to confirm the binary doesn't crash on startup. Surviving
52+
// the grace period is a pass in itself: pairing may legitimately produce
53+
// no output for a while (it contacts Google before printing the QR), so
54+
// a kill-then-inspect approach can't tell a quiet healthy process from a
55+
// crashed one.
56+
done := make(chan error, 1)
57+
go func() { done <- cmd.Wait() }()
5958

60-
// The process should either still be running (killed by our timer) or
61-
// have produced some output before failing. A zero-length output + immediate
62-
// exit means the binary crashed.
63-
if err != nil && len(output) == 0 {
64-
t.Errorf("binary exited immediately with no output: %v", err)
59+
select {
60+
case err := <-done:
61+
// Exited on its own — fine if it said something first (e.g. a
62+
// pairing error without a phone), a crash if it was silent.
63+
if err != nil && len(out.String()) == 0 {
64+
t.Errorf("binary exited immediately with no output: %v", err)
65+
}
66+
case <-time.After(3 * time.Second):
67+
// Still running after the grace period: startup succeeded.
68+
_ = cmd.Process.Kill()
69+
<-done
6570
}
6671

67-
// Should not contain panic traces
72+
// Should not contain panic traces in either outcome.
73+
output := out.String()
6874
if strings.Contains(output, "panic:") || strings.Contains(output, "runtime error") {
6975
t.Errorf("binary panicked:\n%s", output)
7076
}

internal/app/app.go

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,9 @@ const (
3333

3434
const maxErrorDetails = 100
3535

36-
// BackfillProgress tracks the current state of a deep backfill operation.
37-
type BackfillProgress struct {
38-
mu sync.Mutex
36+
// BackfillSnapshot is a point-in-time copy of backfill progress, safe to
37+
// pass and marshal by value.
38+
type BackfillSnapshot struct {
3939
Running bool `json:"running"`
4040
Phase BackfillPhase `json:"phase"`
4141
FoldersScanned int `json:"folders_scanned"`
@@ -46,6 +46,12 @@ type BackfillProgress struct {
4646
ErrorDetails []string `json:"error_details,omitempty"`
4747
}
4848

49+
// BackfillProgress tracks the current state of a deep backfill operation.
50+
type BackfillProgress struct {
51+
mu sync.Mutex
52+
BackfillSnapshot
53+
}
54+
4955
// reset clears all fields for a fresh backfill run.
5056
func (p *BackfillProgress) reset() {
5157
p.mu.Lock()
@@ -95,10 +101,10 @@ func (p *BackfillProgress) add(conversations, messages, contacts, folders int) {
95101
p.FoldersScanned += folders
96102
}
97103

98-
func (p *BackfillProgress) snapshot() BackfillProgress {
104+
func (p *BackfillProgress) snapshot() BackfillSnapshot {
99105
p.mu.Lock()
100106
defer p.mu.Unlock()
101-
cp := *p
107+
cp := p.BackfillSnapshot
102108
if len(p.ErrorDetails) > 0 {
103109
cp.ErrorDetails = append([]string(nil), p.ErrorDetails...)
104110
}
@@ -576,7 +582,7 @@ func (a *App) IsDeepBackfillRunning() bool {
576582
}
577583

578584
// GetBackfillProgress returns a snapshot of the current backfill progress.
579-
func (a *App) GetBackfillProgress() BackfillProgress {
585+
func (a *App) GetBackfillProgress() BackfillSnapshot {
580586
snap := a.BackfillProgress.snapshot()
581587
// A shallow Backfill (startup catch-up) holds the same mutual-exclusion
582588
// guard (backfillRunning) without populating the deep-backfill progress

0 commit comments

Comments
 (0)