Files
stack-sync/main_test.go
LyAhn a221c63d94 fix(sync): keep the live progress counters correct and race-free
Two defects in the in-place progress line introduced with the reporting
work:

- started() only ran for repositories that actually synced, but
  finished() decrements for every repository that records a result. Any
  repository skipped by the eligibility or pre-sync checks therefore
  decremented a counter it had never incremented, so the running "active"
  count went negative and was displayed as such. Count a repository as
  active before the skip checks can exit.

- clear() mutates the line width but ran under outputMu, while draw()
  mutates the same field under the progress mutex. With parallel jobs one
  worker could erase the line while another repainted it. Route clear()
  through the same lock, keeping outputMu around the surrounding writes.

Verified by reverting the first fix and watching the new test report
"active = -3".

🤖 Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>
2026-09-28 20:47:44 +01:00

316 lines
10 KiB
Go

package main
import (
"bytes"
"io"
"os"
"os/exec"
"path/filepath"
"strings"
"sync"
"testing"
"time"
)
func git(t *testing.T, dir string, args ...string) {
t.Helper()
cmd := exec.Command("git", append([]string{"-C", dir}, args...)...)
if out, err := cmd.CombinedOutput(); err != nil {
t.Fatalf("git %v: %v\n%s", args, err, out)
}
}
func makeRepo(t *testing.T, path string) {
t.Helper()
if err := os.MkdirAll(path, 0o755); err != nil {
t.Fatal(err)
}
git(t, path, "init", "-q")
git(t, path, "config", "user.name", "Test")
git(t, path, "config", "user.email", "test@example.com")
if err := os.WriteFile(filepath.Join(path, "tracked.txt"), []byte("clean\n"), 0o644); err != nil {
t.Fatal(err)
}
git(t, path, "add", "tracked.txt")
git(t, path, "commit", "-qm", "initial")
git(t, path, "remote", "add", "origin", "https://example.invalid/repo.git")
}
func TestDiscoverNestedRepositories(t *testing.T) {
root := t.TempDir()
parent := filepath.Join(root, "parent")
child := filepath.Join(parent, "apps", "child")
makeRepo(t, parent)
makeRepo(t, child)
paths, err := discover(root, nil)
if err != nil {
t.Fatal(err)
}
if len(paths) != 2 || paths[0] != parent || paths[1] != child {
t.Fatalf("discover() = %#v", paths)
}
}
func TestNestedRepoDoesNotDirtyParent(t *testing.T) {
root := t.TempDir()
parent := filepath.Join(root, "parent")
child := filepath.Join(parent, "child")
makeRepo(t, parent)
makeRepo(t, child)
paths, _ := discover(root, nil)
got := inspect(root, parent, paths)
if !got.Eligible {
t.Fatalf("parent blocked by nested repo: %+v", got)
}
if len(got.NestedRepoEntries) == 0 {
t.Fatalf("expected nested repository entry to be recorded: %+v", got)
}
}
func TestDirtyFileIsReportedButRepositoryRemainsEligible(t *testing.T) {
root := t.TempDir()
path := filepath.Join(root, "repo")
makeRepo(t, path)
if err := os.WriteFile(filepath.Join(path, "tracked.txt"), []byte("changed\n"), 0o644); err != nil {
t.Fatal(err)
}
got := inspect(root, path, []string{path})
if !got.Eligible || len(got.Dirty) != 1 {
t.Fatalf("dirty repository was not eligible with its changes reported: %+v", got)
}
}
func TestUntrackedFileAlongsideNestedRepoIsStillReported(t *testing.T) {
root := t.TempDir()
parent := filepath.Join(root, "parent")
child := filepath.Join(parent, "child")
makeRepo(t, parent)
makeRepo(t, child)
if err := os.WriteFile(filepath.Join(parent, "notes.txt"), []byte("work\n"), 0o644); err != nil {
t.Fatal(err)
}
paths, _ := discover(root, nil)
got := inspect(root, parent, paths)
if !got.Eligible || len(got.Dirty) != 1 {
t.Fatalf("untracked file should be reported without blocking parent: %+v", got)
}
}
func TestSkippedDirectoriesAreNotSearched(t *testing.T) {
root := t.TempDir()
makeRepo(t, filepath.Join(root, "node_modules", "dependency"))
makeRepo(t, filepath.Join(root, "real"))
paths, err := discover(root, nil)
if err != nil {
t.Fatal(err)
}
if len(paths) != 1 || paths[0] != filepath.Join(root, "real") {
t.Fatalf("discover() = %#v", paths)
}
}
func TestStrictSyncRechecksAndSkipsNewlyDirtyRepository(t *testing.T) {
root := t.TempDir()
path := filepath.Join(root, "repo")
makeRepo(t, path)
planned := inspect(root, path, []string{path})
if !planned.Eligible {
t.Fatalf("fixture should initially be eligible: %+v", planned)
}
if err := os.WriteFile(filepath.Join(path, "late-change.txt"), []byte("do not touch\n"), 0o644); err != nil {
t.Fatal(err)
}
var progress []syncProgressEvent
results := syncSelectedWithProgress(root, []repo{planned}, []repo{planned}, 1, time.Minute, io.Discard, true, true, false, nil, func(event syncProgressEvent) {
progress = append(progress, event)
})
if len(results) != 1 || !results[0].Skipped || results[0].Success {
t.Fatalf("syncAll() = %+v", results)
}
if len(progress) != 1 || progress[0].Started || !progress[0].Result.Skipped {
t.Fatalf("progress events = %+v, want one finished skip", progress)
}
}
func TestScanReportsDirtyEligibleRepository(t *testing.T) {
root := t.TempDir()
path := filepath.Join(root, "repo")
makeRepo(t, path)
if err := os.WriteFile(filepath.Join(path, "untracked.txt"), []byte("work\n"), 0o644); err != nil {
t.Fatal(err)
}
var stdout, stderr bytes.Buffer
code := run([]string{"scan", "--root", root}, bytes.NewReader(nil), &stdout, &stderr)
if code != 0 {
t.Fatalf("run() code = %d, stderr = %q", code, stderr.String())
}
if !bytes.Contains(stdout.Bytes(), []byte("dirty worktree (1 changes), checked-out branch protected")) {
t.Fatalf("scan did not report dirty eligible repository:\n%s", stdout.String())
}
}
func TestIndentNormalizesGitProgressOutput(t *testing.T) {
got := indent("first\rsecond\r\nthird", " ")
want := " first\n second\n third"
if got != want {
t.Fatalf("indent() = %q, want %q", got, want)
}
}
// The summary must say what actually changed, not just how many repositories
// ran. "synced" previously counted repositories where nothing was touched.
func TestSummaryReportsPerActionCountsAndDeletions(t *testing.T) {
results := []syncResult{
{Path: "api", Success: true, Branches: []branchSyncResult{
{Branch: "main", Action: "updated"},
{Branch: "stale", Action: "deleted", Message: "upstream was deleted and the branch was merged into main"},
}},
{Path: "web", Success: true, Branches: []branchSyncResult{
{Branch: "wip", Action: "protected"},
}},
{Path: "cli", Success: true},
{Path: "docs", Message: "fetch origin: authentication failed"},
}
var out bytes.Buffer
printSummary(&out, results)
got := out.String()
for _, want := range []string{
"1 changed", "2 unchanged", "1 failed", "0 skipped",
"1 updated · 1 deleted · 1 protected",
"Needs review:",
"DELETED api / stale: upstream was deleted and the branch was merged into main",
"FAILED docs: fetch origin: authentication failed",
} {
if !strings.Contains(got, want) {
t.Fatalf("summary missing %q:\n%s", want, got)
}
}
// A workspace where nothing changed must not claim to have synced anything,
// and must not open a "Needs review" section with nothing to review.
var quiet bytes.Buffer
printSummary(&quiet, []syncResult{{Path: "web", Success: true, Branches: []branchSyncResult{{Branch: "wip", Action: "protected"}}}})
if strings.Contains(quiet.String(), "1 changed") {
t.Fatalf("a protected-only run was reported as changed:\n%s", quiet.String())
}
if strings.Contains(quiet.String(), "Needs review") {
t.Fatalf("a run with no failures or deletions opened a review section:\n%s", quiet.String())
}
}
// The in-place progress line must never be written to a pipe, a buffer, or a
// --json document, where the cursor control would corrupt the captured output.
func TestLiveProgressIsDisabledForNonTerminalOutput(t *testing.T) {
var mu sync.Mutex
if p := newLiveProgress(&bytes.Buffer{}, &mu, 3); p != nil {
t.Fatal("newLiveProgress() enabled the live line for a buffered writer")
}
}
func TestLiveProgressRendersRunningTotals(t *testing.T) {
var mu sync.Mutex
buf := &bytes.Buffer{}
p := &liveProgress{w: buf, mu: &mu, total: 3, counts: map[string]int{}}
p.update(func() { p.started(); p.draw() })
if !strings.Contains(buf.String(), "0/3") || !strings.Contains(buf.String(), "1 active") {
t.Fatalf("start did not render progress: %q", buf.String())
}
buf.Reset()
p.update(func() {
p.finished(syncResult{Path: "api", Success: true, Branches: []branchSyncResult{{Branch: "main", Action: "updated"}}})
p.draw()
})
for _, want := range []string{"1/3", "1 changed", "1 updated"} {
if !strings.Contains(buf.String(), want) {
t.Fatalf("totals missing %q: %q", want, buf.String())
}
}
buf.Reset()
p.update(func() {
p.finished(syncResult{Path: "web", Message: "fetch origin: authentication failed"})
p.draw()
})
for _, want := range []string{"2/3", "1 FAILED"} {
if !strings.Contains(buf.String(), want) {
t.Fatalf("failure not surfaced live: %q", buf.String())
}
}
if strings.Contains(buf.String(), "2 changed") {
t.Fatalf("a failed repository was counted as changed: %q", buf.String())
}
}
// End-to-end guard: a non-interactive run must emit no carriage returns at all.
func TestSyncOutputToBufferHasNoCarriageReturns(t *testing.T) {
root := t.TempDir()
path := filepath.Join(root, "repo")
makeRepo(t, path)
var stdout, stderr bytes.Buffer
code := run([]string{"sync", "--root", root, "--yes"}, bytes.NewReader(nil), &stdout, &stderr)
if code > 1 {
t.Fatalf("run() code = %d, stderr = %q", code, stderr.String())
}
if strings.Contains(stdout.String(), "\r") {
t.Fatalf("buffered output contained cursor control:\n%q", stdout.String())
}
if !strings.Contains(stdout.String(), "Summary:") {
t.Fatalf("run did not print a summary:\n%q", stdout.String())
}
}
// A repository skipped before it ever syncs must still be counted active and
// then finished. Counting it as active only after the skip checks left the
// running "active" total negative, because finished() decrements it regardless.
func TestLiveProgressBalancesSkippedRepositories(t *testing.T) {
root := t.TempDir()
var repos []repo
for _, name := range []string{"blocked-a", "blocked-b", "blocked-c"} {
path := filepath.Join(root, name)
if err := os.MkdirAll(path, 0o755); err != nil {
t.Fatal(err)
}
// Not Eligible, so the worker takes the early-exit skip path.
repos = append(repos, repo{Path: path, RelativePath: name, BlockReason: "no remotes"})
}
var mu sync.Mutex
buf := &bytes.Buffer{}
live := &liveProgress{w: buf, mu: &mu, total: len(repos), counts: map[string]int{}}
results := syncSelectedWithProgress(root, repos, repos, 2, time.Minute, io.Discard, true, false, false, live, nil)
if len(results) != len(repos) {
t.Fatalf("results = %d, want %d", len(results), len(repos))
}
for _, result := range results {
if !result.Skipped {
t.Fatalf("%s was not skipped: %+v", result.Path, result)
}
}
if live.done != len(repos) {
t.Fatalf("done = %d, want %d", live.done, len(repos))
}
if live.active != 0 {
t.Fatalf("active = %d after every repository finished, want 0", live.active)
}
if live.skipped != len(repos) {
t.Fatalf("skipped = %d, want %d", live.skipped, len(repos))
}
// Intermediate frames legitimately report active repositories; only the
// last one must be settled.
frames := strings.Split(buf.String(), "\r")
if last := frames[len(frames)-1]; strings.Contains(last, "active") {
t.Fatalf("a settled run still reports active repositories: %q", last)
}
}