From 126bf616ab077ce172fd381d55a07105b779b0a0 Mon Sep 17 00:00:00 2001 From: Ben Wisecup Date: Wed, 5 Aug 2026 17:47:50 -0400 Subject: [PATCH 1/3] test(packman,config): pin that gitignored artifacts do not dirty a cached import MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both cache-dirtiness checks deliberately run `git status --porcelain` without --ignored, so a pack's own gitignored artifacts (__pycache__/*.pyc, .DS_Store, a runtime state directory recreated after install) are not treated as local edits. Neither call site had a test, so re-adding the flag would pass CI while wedging every city behind a perpetual "run gc import install" gate that no .gitignore can escape -- --ignored prints a `!! ` line for exactly the files the ignore rule was meant to neutralize, and the city loses every pack-provided subcommand. Adds coverage at both call sites: - internal/packman cachedRepoDirty, against a real git repo. One test asserts a gitignored .runtime/ artifact leaves the cache clean, guarded by a positive control that fails the fixture if `--ignored` would not have seen it. A table test asserts untracked, edited, and deleted files still report dirty. - internal/config validateLockedRemoteCache, using the package's existing runRepoCacheGit stub, asserting the status invocation carries no --ignored and that a modified worktree is still rejected. Verified by injection: re-adding --ignored at either call site turns that site's test red, and reverting turns it green again. Generated by the operator's software factory. City: factory-main · Agent: local-core.builder-5 On behalf of: @benw5483 Co-Authored-By: --- .../config/pack_include_cache_dirty_test.go | 62 ++++++++ internal/packman/cache_dirty_test.go | 135 ++++++++++++++++++ 2 files changed, 197 insertions(+) create mode 100644 internal/config/pack_include_cache_dirty_test.go create mode 100644 internal/packman/cache_dirty_test.go diff --git a/internal/config/pack_include_cache_dirty_test.go b/internal/config/pack_include_cache_dirty_test.go new file mode 100644 index 0000000000..eb766215e7 --- /dev/null +++ b/internal/config/pack_include_cache_dirty_test.go @@ -0,0 +1,62 @@ +package config + +import ( + "slices" + "testing" +) + +// validateLockedRemoteCache decides whether a cached import counts as locally +// modified. It deliberately runs `git status --porcelain` WITHOUT --ignored: +// --ignored prints a `!! ` line for every gitignored file, so a pack's own +// gitignored build artifacts (__pycache__/*.pyc, .DS_Store, a runtime state +// directory) would fail the check and no .gitignore could prevent it. The city +// then wedges behind a perpetual "run gc import install" gate with every +// pack-provided subcommand dropped. +// +// This test pins the argument list, because the argument list is the defect. + +func TestValidateLockedRemoteCacheStatusOmitsIgnored(t *testing.T) { + const commit = "abc123def456" + + var statusArgs []string + prev := runRepoCacheGit + runRepoCacheGit = func(_ string, args ...string) (string, error) { + if len(args) > 0 && args[0] == "status" { + statusArgs = append([]string(nil), args...) + return "", nil + } + return commit, nil + } + t.Cleanup(func() { runRepoCacheGit = prev }) + + if err := validateLockedRemoteCache("https://example.com/tools.git", t.TempDir(), commit); err != nil { + t.Fatalf("validateLockedRemoteCache on a clean cache: %v", err) + } + + if statusArgs == nil { + t.Fatal("validateLockedRemoteCache never ran git status, so this test asserts nothing") + } + if slices.Contains(statusArgs, "--ignored") { + t.Errorf("git status ran with --ignored (%v); gitignored artifacts must not mark a cached import dirty", statusArgs) + } +} + +// The companion assertion: omitting --ignored must not stop a genuinely modified +// cache from failing validation. +func TestValidateLockedRemoteCacheRejectsModifiedWorktree(t *testing.T) { + const commit = "abc123def456" + + prev := runRepoCacheGit + runRepoCacheGit = func(_ string, args ...string) (string, error) { + if len(args) > 0 && args[0] == "status" { + return " M packs/local-core/pack.toml\n", nil + } + return commit, nil + } + t.Cleanup(func() { runRepoCacheGit = prev }) + + err := validateLockedRemoteCache("https://example.com/tools.git", t.TempDir(), commit) + if err == nil { + t.Fatal("validateLockedRemoteCache accepted a cache with a modified tracked file") + } +} diff --git a/internal/packman/cache_dirty_test.go b/internal/packman/cache_dirty_test.go new file mode 100644 index 0000000000..b517f827f0 --- /dev/null +++ b/internal/packman/cache_dirty_test.go @@ -0,0 +1,135 @@ +package packman + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +// cachedRepoDirty deliberately runs `git status --porcelain` WITHOUT --ignored, +// so gitignored build artifacts that land in a cache clone in place (Python +// __pycache__/*.pyc from running a cached pack's scripts, a stray .DS_Store, a +// pack's own gitignored .runtime/ state directory) do not count as local +// modifications. Re-adding --ignored wedges the city behind a perpetual "run gc +// import install" gate that no .gitignore can escape, because --ignored prints a +// `!! ` line for exactly the files the ignore rule was meant to neutralize. +// +// These tests pin that behavior. They use a real git repo rather than a stub so +// they assert the observable outcome, not just the argument list. + +func dirtyTestGit(t *testing.T, dir string, args ...string) string { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + return string(out) +} + +// newDirtyTestRepo returns a committed repo that gitignores .runtime/ and holds +// one tracked file, so callers can perturb it in a controlled way. +func newDirtyTestRepo(t *testing.T) string { + t.Helper() + repo := t.TempDir() + dirtyTestGit(t, repo, "init", "--quiet") + dirtyTestGit(t, repo, "config", "user.email", "test@example.invalid") + dirtyTestGit(t, repo, "config", "user.name", "packman test") + if err := os.WriteFile(filepath.Join(repo, ".gitignore"), []byte(".runtime/\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(repo, "tracked.txt"), []byte("v1\n"), 0o644); err != nil { + t.Fatal(err) + } + dirtyTestGit(t, repo, "add", ".") + dirtyTestGit(t, repo, "commit", "--quiet", "-m", "base") + + dirty, err := newDirtyTestRepoBaseline(repo) + if err != nil { + t.Fatal(err) + } + if dirty { + t.Fatalf("fixture is dirty before any perturbation") + } + return repo +} + +func newDirtyTestRepoBaseline(repo string) (bool, error) { return cachedRepoDirty(repo) } + +func TestCachedRepoDirtyIgnoresGitignoredArtifacts(t *testing.T) { + repo := newDirtyTestRepo(t) + + // The artifact observed in the field: a pack's gitignored runtime state + // directory recreated inside the cache clone after `gc import install`. + if err := os.MkdirAll(filepath.Join(repo, ".runtime", "reminders"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(repo, ".runtime", "reminders", ".lock"), nil, 0o644); err != nil { + t.Fatal(err) + } + + // Positive control. If --ignored sees nothing here, the fixture does not + // model the regression and a pass below would be meaningless. + withIgnored := dirtyTestGit(t, repo, "status", "--porcelain", "--ignored") + if strings.TrimSpace(withIgnored) == "" { + t.Fatal("fixture does not model the regression: `git status --porcelain --ignored` reported nothing") + } + + dirty, err := cachedRepoDirty(repo) + if err != nil { + t.Fatal(err) + } + if dirty { + t.Errorf("gitignored artifact reported the cache dirty; `git status --porcelain --ignored` saw %q, and cachedRepoDirty must not", strings.TrimSpace(withIgnored)) + } +} + +// The companion to the test above. Dropping --ignored must not blind the gate to +// changes that really are local edits to the pack's content. +func TestCachedRepoDirtyStillCatchesRealChanges(t *testing.T) { + for _, tc := range []struct { + name string + perturb func(t *testing.T, repo string) + }{ + { + name: "untracked file", + perturb: func(t *testing.T, repo string) { + if err := os.WriteFile(filepath.Join(repo, "stray.txt"), []byte("x\n"), 0o644); err != nil { + t.Fatal(err) + } + }, + }, + { + name: "tracked file edited", + perturb: func(t *testing.T, repo string) { + if err := os.WriteFile(filepath.Join(repo, "tracked.txt"), []byte("v2\n"), 0o644); err != nil { + t.Fatal(err) + } + }, + }, + { + name: "tracked file deleted", + perturb: func(t *testing.T, repo string) { + if err := os.Remove(filepath.Join(repo, "tracked.txt")); err != nil { + t.Fatal(err) + } + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + repo := newDirtyTestRepo(t) + tc.perturb(t, repo) + + dirty, err := cachedRepoDirty(repo) + if err != nil { + t.Fatal(err) + } + if !dirty { + t.Error("a real local change did not report the cache dirty") + } + }) + } +} From b07966bb9a7a2080488d2361b6ed4a995194452e Mon Sep 17 00:00:00 2001 From: Ben Wisecup Date: Wed, 5 Aug 2026 18:05:54 -0400 Subject: [PATCH 2/3] test(packman): stub git in the cache-dirtiness test instead of shelling out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first version of this test drove a real git repo. That tripped the checked resource ledger in internal/testpolicy/resourcecensus: it is an anti-growth ratchet on subprocess use in test source, and the file pushed the untagged Small baseline from 391 calls / 110 files to 392 / 111. Clearing that would have meant declaring a Medium owner and bumping audit baselines that belong to another tracking owner, which is a policy decision this change has no reason to make. Stubbing runGit is also the established pattern in this package -- every other test in internal/packman already does it, and the real-git version was the lone exception. The assertion does not weaken. The stub models git's own behavior, returning a `!! ` line when --ignored is present and nothing when it is not, so the test still pins the outcome: an ignored artifact leaves the cache clean. It also now asserts directly that the status invocation carries no --ignored, which is the defect itself. The companion table gains a staged-addition case. Re-verified by injection after the rewrite: adding --ignored back to cachedRepoDirty turns this test red, and reverting turns it green. Generated by the operator's software factory. City: factory-main · Agent: local-core.builder-5 On behalf of: @benw5483 Co-Authored-By: --- internal/packman/cache_dirty_test.go | 146 ++++++++++----------------- 1 file changed, 53 insertions(+), 93 deletions(-) diff --git a/internal/packman/cache_dirty_test.go b/internal/packman/cache_dirty_test.go index b517f827f0..b2fe1b2533 100644 --- a/internal/packman/cache_dirty_test.go +++ b/internal/packman/cache_dirty_test.go @@ -1,9 +1,7 @@ package packman import ( - "os" - "os/exec" - "path/filepath" + "slices" "strings" "testing" ) @@ -11,124 +9,86 @@ import ( // cachedRepoDirty deliberately runs `git status --porcelain` WITHOUT --ignored, // so gitignored build artifacts that land in a cache clone in place (Python // __pycache__/*.pyc from running a cached pack's scripts, a stray .DS_Store, a -// pack's own gitignored .runtime/ state directory) do not count as local -// modifications. Re-adding --ignored wedges the city behind a perpetual "run gc -// import install" gate that no .gitignore can escape, because --ignored prints a -// `!! ` line for exactly the files the ignore rule was meant to neutralize. +// pack's own gitignored runtime state directory) do not count as local edits. // -// These tests pin that behavior. They use a real git repo rather than a stub so -// they assert the observable outcome, not just the argument list. - -func dirtyTestGit(t *testing.T, dir string, args ...string) string { - t.Helper() - cmd := exec.Command("git", args...) - cmd.Dir = dir - out, err := cmd.CombinedOutput() - if err != nil { - t.Fatalf("git %v: %v\n%s", args, err, out) - } - return string(out) -} +// Re-adding the flag wedges the city behind a perpetual "run gc import install" +// gate that no .gitignore can escape: --ignored prints a `!! ` line for +// exactly the files an ignore rule was meant to neutralize, so ignoring the path +// turns a `??` line into a `!!` line and changes nothing the check sees. The +// city then loses every pack-provided subcommand until the next install, and the +// artifact reappears. +// +// The argument list is the defect, so that is what these tests pin. -// newDirtyTestRepo returns a committed repo that gitignores .runtime/ and holds -// one tracked file, so callers can perturb it in a controlled way. -func newDirtyTestRepo(t *testing.T) string { +// gitStatusStub reports the status arguments cachedRepoDirty passed, and models +// git's own behavior: --ignored adds a `!! ` line for an ignored artifact, +// and a plain --porcelain run does not report it at all. +func gitStatusStub(t *testing.T, ignoredArtifact string) *[]string { t.Helper() - repo := t.TempDir() - dirtyTestGit(t, repo, "init", "--quiet") - dirtyTestGit(t, repo, "config", "user.email", "test@example.invalid") - dirtyTestGit(t, repo, "config", "user.name", "packman test") - if err := os.WriteFile(filepath.Join(repo, ".gitignore"), []byte(".runtime/\n"), 0o644); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(repo, "tracked.txt"), []byte("v1\n"), 0o644); err != nil { - t.Fatal(err) - } - dirtyTestGit(t, repo, "add", ".") - dirtyTestGit(t, repo, "commit", "--quiet", "-m", "base") - - dirty, err := newDirtyTestRepoBaseline(repo) - if err != nil { - t.Fatal(err) - } - if dirty { - t.Fatalf("fixture is dirty before any perturbation") + var seen []string + prev := runGit + runGit = func(_ string, args ...string) (string, error) { + if len(args) > 0 && args[0] == "status" { + seen = append([]string(nil), args...) + if slices.Contains(args, "--ignored") { + return "!! " + ignoredArtifact + "\n", nil + } + return "", nil + } + return "", nil } - return repo + t.Cleanup(func() { runGit = prev }) + return &seen } -func newDirtyTestRepoBaseline(repo string) (bool, error) { return cachedRepoDirty(repo) } - func TestCachedRepoDirtyIgnoresGitignoredArtifacts(t *testing.T) { - repo := newDirtyTestRepo(t) + const artifact = "packs/local-core/.runtime/" + seen := gitStatusStub(t, artifact) - // The artifact observed in the field: a pack's gitignored runtime state - // directory recreated inside the cache clone after `gc import install`. - if err := os.MkdirAll(filepath.Join(repo, ".runtime", "reminders"), 0o755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(repo, ".runtime", "reminders", ".lock"), nil, 0o644); err != nil { + dirty, err := cachedRepoDirty(t.TempDir()) + if err != nil { t.Fatal(err) } - // Positive control. If --ignored sees nothing here, the fixture does not - // model the regression and a pass below would be meaningless. - withIgnored := dirtyTestGit(t, repo, "status", "--porcelain", "--ignored") - if strings.TrimSpace(withIgnored) == "" { - t.Fatal("fixture does not model the regression: `git status --porcelain --ignored` reported nothing") + if *seen == nil { + t.Fatal("cachedRepoDirty never ran git status, so this test asserts nothing") } - - dirty, err := cachedRepoDirty(repo) - if err != nil { - t.Fatal(err) + if slices.Contains(*seen, "--ignored") { + t.Errorf("git status ran with --ignored (%v); a gitignored artifact must not mark a cache clone dirty", *seen) } if dirty { - t.Errorf("gitignored artifact reported the cache dirty; `git status --porcelain --ignored` saw %q, and cachedRepoDirty must not", strings.TrimSpace(withIgnored)) + t.Errorf("gitignored artifact %q reported the cache dirty", artifact) } } -// The companion to the test above. Dropping --ignored must not blind the gate to +// The companion to the test above. Dropping --ignored must not blind the check to // changes that really are local edits to the pack's content. func TestCachedRepoDirtyStillCatchesRealChanges(t *testing.T) { for _, tc := range []struct { - name string - perturb func(t *testing.T, repo string) + name string + status string }{ - { - name: "untracked file", - perturb: func(t *testing.T, repo string) { - if err := os.WriteFile(filepath.Join(repo, "stray.txt"), []byte("x\n"), 0o644); err != nil { - t.Fatal(err) - } - }, - }, - { - name: "tracked file edited", - perturb: func(t *testing.T, repo string) { - if err := os.WriteFile(filepath.Join(repo, "tracked.txt"), []byte("v2\n"), 0o644); err != nil { - t.Fatal(err) - } - }, - }, - { - name: "tracked file deleted", - perturb: func(t *testing.T, repo string) { - if err := os.Remove(filepath.Join(repo, "tracked.txt")); err != nil { - t.Fatal(err) - } - }, - }, + {name: "untracked file", status: "?? packs/local-core/stray.txt\n"}, + {name: "tracked file edited", status: " M packs/local-core/pack.toml\n"}, + {name: "tracked file deleted", status: " D packs/local-core/pack.toml\n"}, + {name: "staged addition", status: "A packs/local-core/new.toml\n"}, } { t.Run(tc.name, func(t *testing.T) { - repo := newDirtyTestRepo(t) - tc.perturb(t, repo) + prev := runGit + runGit = func(_ string, args ...string) (string, error) { + if len(args) > 0 && args[0] == "status" { + return tc.status, nil + } + return "", nil + } + t.Cleanup(func() { runGit = prev }) - dirty, err := cachedRepoDirty(repo) + dirty, err := cachedRepoDirty(t.TempDir()) if err != nil { t.Fatal(err) } if !dirty { - t.Error("a real local change did not report the cache dirty") + t.Errorf("status %q did not report the cache dirty", strings.TrimSpace(tc.status)) } }) } From cd73d046f3cb50c04fcfc5fb9063db7def7f2111 Mon Sep 17 00:00:00 2001 From: Ben Wisecup Date: Wed, 5 Aug 2026 18:53:23 -0400 Subject: [PATCH 3/3] test: match --ignored by prefix, so --ignored=matching cannot slip past MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review caught a real hole. The guards matched the flag with an exact string comparison against "--ignored", but git accepts several spellings of the same behaviour, and they are not interchangeable to an equality check. Verified against git 2.50.1 on a repo with a gitignored .runtime/: --ignored, --ignored=traditional and --ignored=matching each print the identical "!! .runtime/" line, and only --ignored=no suppresses it. So --ignored=matching reintroduced the exact defect these tests exist to ratchet against while all four of them stayed green. The packman stub made it worse by modelling git with the same too-narrow predicate, so the outcome assertion agreed with the flag assertion instead of checking it independently. Both files now share a statusShowsIgnored helper that matches on the "--ignored" prefix, exempts only --ignored=no, and fails closed on unrecognized modes. The packman stub and the assertion both route through it. Two follow-ups from the same review: - TestValidateLockedRemoteCacheRejectsModifiedWorktree asserted only that an error came back. validateLockedRemoteCache fails earlier when rev-parse HEAD disagrees with the locked commit, so a reorder could have kept the test green with the status check rejecting nothing. It now asserts that status actually ran and that the error is the worktree one. Confirmed by forcing a HEAD mismatch: the test fails with "never ran git status" rather than passing. - gitStatusStub returns a closure accessor instead of a pointer to a slice. Injection matrix re-run at this head, one spelling at a time, both call sites: --ignored, --ignored=traditional and --ignored=matching all fail; --ignored=no passes, which is the negative control showing the guard is not simply refusing every argument. Production files reverted after each run. Generated by the operator's software factory. City: factory-main · Agent: local-core.builder-5 On behalf of: @benw5483 Co-Authored-By: --- .../config/pack_include_cache_dirty_test.go | 29 ++++++++++++++++++- internal/packman/cache_dirty_test.go | 29 ++++++++++++++----- 2 files changed, 50 insertions(+), 8 deletions(-) diff --git a/internal/config/pack_include_cache_dirty_test.go b/internal/config/pack_include_cache_dirty_test.go index eb766215e7..f288c6de9f 100644 --- a/internal/config/pack_include_cache_dirty_test.go +++ b/internal/config/pack_include_cache_dirty_test.go @@ -2,6 +2,7 @@ package config import ( "slices" + "strings" "testing" ) @@ -15,6 +16,19 @@ import ( // // This test pins the argument list, because the argument list is the defect. +// statusShowsIgnored reports whether a `git status` argument list would print +// `!! ` lines. git accepts the flag in several spellings and they are not +// interchangeable to a naive equality check: bare --ignored means +// --ignored=traditional, and --ignored=matching prints the same `!!` lines +// (verified against git 2.50.1). Only --ignored=no suppresses them. Matching on +// the exact string "--ignored" alone would let --ignored=matching reintroduce +// the defect with this test still green, so unrecognized modes fail closed. +func statusShowsIgnored(args []string) bool { + return slices.ContainsFunc(args, func(arg string) bool { + return strings.HasPrefix(arg, "--ignored") && arg != "--ignored=no" + }) +} + func TestValidateLockedRemoteCacheStatusOmitsIgnored(t *testing.T) { const commit = "abc123def456" @@ -36,19 +50,26 @@ func TestValidateLockedRemoteCacheStatusOmitsIgnored(t *testing.T) { if statusArgs == nil { t.Fatal("validateLockedRemoteCache never ran git status, so this test asserts nothing") } - if slices.Contains(statusArgs, "--ignored") { + if statusShowsIgnored(statusArgs) { t.Errorf("git status ran with --ignored (%v); gitignored artifacts must not mark a cached import dirty", statusArgs) } } // The companion assertion: omitting --ignored must not stop a genuinely modified // cache from failing validation. +// +// validateLockedRemoteCache fails earlier if rev-parse HEAD disagrees with the +// locked commit, so asserting only err != nil would keep this test green if a +// reorder made the status check stop rejecting anything. Assert that status ran +// and that the error is the worktree one. func TestValidateLockedRemoteCacheRejectsModifiedWorktree(t *testing.T) { const commit = "abc123def456" + statusRan := false prev := runRepoCacheGit runRepoCacheGit = func(_ string, args ...string) (string, error) { if len(args) > 0 && args[0] == "status" { + statusRan = true return " M packs/local-core/pack.toml\n", nil } return commit, nil @@ -56,6 +77,12 @@ func TestValidateLockedRemoteCacheRejectsModifiedWorktree(t *testing.T) { t.Cleanup(func() { runRepoCacheGit = prev }) err := validateLockedRemoteCache("https://example.com/tools.git", t.TempDir(), commit) + if !statusRan { + t.Fatal("validateLockedRemoteCache never ran git status, so this test asserts nothing") + } + if err != nil && !strings.Contains(err.Error(), "local worktree changes") { + t.Fatalf("rejected for the wrong reason: %v", err) + } if err == nil { t.Fatal("validateLockedRemoteCache accepted a cache with a modified tracked file") } diff --git a/internal/packman/cache_dirty_test.go b/internal/packman/cache_dirty_test.go index b2fe1b2533..f33736def1 100644 --- a/internal/packman/cache_dirty_test.go +++ b/internal/packman/cache_dirty_test.go @@ -20,17 +20,32 @@ import ( // // The argument list is the defect, so that is what these tests pin. +// statusShowsIgnored reports whether a `git status` argument list would print +// `!! ` lines. git accepts the flag in several spellings and they are not +// interchangeable to a naive equality check: bare --ignored means +// --ignored=traditional, and --ignored=matching prints the same `!!` lines +// (verified against git 2.50.1). Only --ignored=no suppresses them. Matching on +// the exact string "--ignored" alone would let --ignored=matching reintroduce +// the defect with every test in this file still green, so unrecognized modes +// fail closed. +func statusShowsIgnored(args []string) bool { + return slices.ContainsFunc(args, func(arg string) bool { + return strings.HasPrefix(arg, "--ignored") && arg != "--ignored=no" + }) +} + // gitStatusStub reports the status arguments cachedRepoDirty passed, and models // git's own behavior: --ignored adds a `!! ` line for an ignored artifact, // and a plain --porcelain run does not report it at all. -func gitStatusStub(t *testing.T, ignoredArtifact string) *[]string { +// The returned accessor reports the status arguments seen so far. +func gitStatusStub(t *testing.T, ignoredArtifact string) func() []string { t.Helper() var seen []string prev := runGit runGit = func(_ string, args ...string) (string, error) { if len(args) > 0 && args[0] == "status" { seen = append([]string(nil), args...) - if slices.Contains(args, "--ignored") { + if statusShowsIgnored(args) { return "!! " + ignoredArtifact + "\n", nil } return "", nil @@ -38,23 +53,23 @@ func gitStatusStub(t *testing.T, ignoredArtifact string) *[]string { return "", nil } t.Cleanup(func() { runGit = prev }) - return &seen + return func() []string { return seen } } func TestCachedRepoDirtyIgnoresGitignoredArtifacts(t *testing.T) { const artifact = "packs/local-core/.runtime/" - seen := gitStatusStub(t, artifact) + statusArgs := gitStatusStub(t, artifact) dirty, err := cachedRepoDirty(t.TempDir()) if err != nil { t.Fatal(err) } - if *seen == nil { + if statusArgs() == nil { t.Fatal("cachedRepoDirty never ran git status, so this test asserts nothing") } - if slices.Contains(*seen, "--ignored") { - t.Errorf("git status ran with --ignored (%v); a gitignored artifact must not mark a cache clone dirty", *seen) + if statusShowsIgnored(statusArgs()) { + t.Errorf("git status ran with --ignored (%v); a gitignored artifact must not mark a cache clone dirty", statusArgs()) } if dirty { t.Errorf("gitignored artifact %q reported the cache dirty", artifact)