Recommendation relevance, the rollback unit, and a version that names what shipped #131
@@ -76,6 +76,28 @@ readonly SHIPPED=(
|
|||||||
':!.dockerignore'
|
':!.dockerignore'
|
||||||
':!renovate.json'
|
':!renovate.json'
|
||||||
':!.golangci.yml'
|
':!.golangci.yml'
|
||||||
|
|
||||||
|
# TESTS DO NOT SHIP, so they must not re-version an artifact.
|
||||||
|
#
|
||||||
|
# Named as globs rather than a directory because this repo has no tests/
|
||||||
|
# tree to exclude: Go tests sit inline beside the code they cover, and the
|
||||||
|
# web suite sits beside its modules. `go build` drops *_test.go outright and
|
||||||
|
# the Vite build never imports a .test.ts, so neither reaches an artifact.
|
||||||
|
#
|
||||||
|
# A commit touching a test AND its source still moves the version — the
|
||||||
|
# source path matches on its own. Only a test-ONLY commit is inert, which is
|
||||||
|
# the whole intent.
|
||||||
|
#
|
||||||
|
# Patterns match what exists today and nothing speculative: there are no
|
||||||
|
# .spec.* files, no __tests__/ directories and no androidTest/ tree. If any
|
||||||
|
# appear they will re-version until named here, which is the harmless
|
||||||
|
# direction and the reason this list is a denylist.
|
||||||
|
':!*_test.go' # 158 files, inline beside the code
|
||||||
|
':!*.test.ts' # 114 files
|
||||||
|
':!*.test.js'
|
||||||
|
':!android/app/src/test' # JVM unit tests; no androidTest tree exists
|
||||||
|
':!web/vitest.config.ts' # test-harness config, not build config
|
||||||
|
':!web/vitest.setup.ts'
|
||||||
)
|
)
|
||||||
|
|
||||||
commit_epoch="${MINSTREL_COMMIT_EPOCH:-}"
|
commit_epoch="${MINSTREL_COMMIT_EPOCH:-}"
|
||||||
|
|||||||
@@ -496,7 +496,11 @@ func commitFile(t *testing.T, dir, path, epoch string) {
|
|||||||
if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil {
|
if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil {
|
||||||
t.Fatal(err)
|
t.Fatal(err)
|
||||||
}
|
}
|
||||||
if err := os.WriteFile(full, []byte("x\n"), 0o644); err != nil {
|
// Content must DIFFER from any earlier write to the same path, or git
|
||||||
|
// records nothing and the commit silently covers fewer files than the
|
||||||
|
// test believes. That is not hypothetical: it made the
|
||||||
|
// source-alongside-test case pass vacuously.
|
||||||
|
if err := os.WriteFile(full, []byte("content @"+epoch+"\n"), 0o644); err != nil {
|
||||||
t.Fatal(err)
|
t.Fatal(err)
|
||||||
}
|
}
|
||||||
for _, args := range [][]string{{"add", "-A"}, {"commit", "-q", "-m", path}} {
|
for _, args := range [][]string{{"add", "-A"}, {"commit", "-q", "-m", path}} {
|
||||||
@@ -599,3 +603,116 @@ func TestVersionScript_RefusesWhenNothingShippedIsInRange(t *testing.T) {
|
|||||||
t.Errorf("script emitted a version name while refusing — that value could still be consumed\n%s", out)
|
t.Errorf("script emitted a version name while refusing — that value could still be consumed\n%s", out)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// commitFiles is commitFile for more than one path in a single commit.
|
||||||
|
func commitFiles(t *testing.T, dir, epoch string, paths ...string) {
|
||||||
|
t.Helper()
|
||||||
|
for _, rel := range paths {
|
||||||
|
full := filepath.Join(dir, rel)
|
||||||
|
if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if err := os.WriteFile(full, []byte("content @"+epoch+"\n"), 0o644); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
for _, args := range [][]string{{"add", "-A"}, {"commit", "-q", "-m", strings.Join(paths, " ")}} {
|
||||||
|
cmd := exec.Command("git", args...)
|
||||||
|
cmd.Dir = dir
|
||||||
|
cmd.Env = append(os.Environ(),
|
||||||
|
"GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_SYSTEM=/dev/null",
|
||||||
|
"GIT_AUTHOR_DATE=@"+epoch+" +0000",
|
||||||
|
"GIT_COMMITTER_DATE=@"+epoch+" +0000",
|
||||||
|
)
|
||||||
|
if out, err := cmd.CombinedOutput(); err != nil {
|
||||||
|
t.Fatalf("git %v: %v\n%s", args, err, out)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// gitOutput runs a git command in dir and returns its combined output.
|
||||||
|
func gitOutput(t *testing.T, dir string, args ...string) string {
|
||||||
|
t.Helper()
|
||||||
|
cmd := exec.Command("git", args...)
|
||||||
|
cmd.Dir = dir
|
||||||
|
cmd.Env = append(os.Environ(), "GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_SYSTEM=/dev/null")
|
||||||
|
out, err := cmd.CombinedOutput()
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("git %v: %v\n%s", args, err, out)
|
||||||
|
}
|
||||||
|
return string(out)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Tests do not ship, so a test-only commit must not re-version an artifact.
|
||||||
|
//
|
||||||
|
// `go build` drops *_test.go outright and the Vite build never imports a
|
||||||
|
// .test.ts, so neither reaches an image or an APK. This repo has no tests/
|
||||||
|
// tree — Go tests sit inline beside the code — so the exclusions are globs,
|
||||||
|
// and a glob is easy to get subtly wrong in a way that still looks right.
|
||||||
|
func TestVersionName_TestsDoNotReVersionTheArtifact(t *testing.T) {
|
||||||
|
const (
|
||||||
|
shipped = "1757443736" // 2025.09.09.1848
|
||||||
|
later = "1789000920" // 2026.09.10.0042
|
||||||
|
)
|
||||||
|
for _, tc := range []struct{ name, path string }{
|
||||||
|
{"go test beside its source", "internal/server/thing_test.go"},
|
||||||
|
{"web unit test", "web/src/lib/api/admin.test.ts"},
|
||||||
|
{"web script test", "web/scripts/tokens-to-css.test.js"},
|
||||||
|
{"android JVM unit test", "android/app/src/test/java/A.kt"},
|
||||||
|
{"vitest harness config", "web/vitest.config.ts"},
|
||||||
|
} {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
dir := gitRepo(t)
|
||||||
|
commitFile(t, dir, "internal/server/thing.go", shipped)
|
||||||
|
commitFile(t, dir, tc.path, later)
|
||||||
|
|
||||||
|
out, err := versionIn(t, dir)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("version.sh failed: %v\n%s", err, out)
|
||||||
|
}
|
||||||
|
if !strings.Contains(out, "name=2025.09.09.1848") {
|
||||||
|
t.Errorf("a commit touching only %s moved the version; tests do not ship\n%s", tc.path, out)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The direction that actually costs something, and the reason the exclusions
|
||||||
|
// above are globs over FILES rather than over their directories.
|
||||||
|
//
|
||||||
|
// `':!internal'` would satisfy every assertion in the test above while
|
||||||
|
// silently excluding the entire server. This pins the opposite: a commit that
|
||||||
|
// changes a test AND the source under it must still move the version, because
|
||||||
|
// the source path matches on its own. Without this, a too-broad exclusion
|
||||||
|
// reads as a passing test suite and ships an artifact under a stale version.
|
||||||
|
func TestVersionName_ATestAlongsideItsSourceStillCounts(t *testing.T) {
|
||||||
|
const (
|
||||||
|
shipped = "1757443736"
|
||||||
|
later = "1789000920"
|
||||||
|
)
|
||||||
|
dir := gitRepo(t)
|
||||||
|
commitFile(t, dir, "internal/server/thing.go", shipped)
|
||||||
|
commitFiles(t, dir, later,
|
||||||
|
"internal/server/thing.go",
|
||||||
|
"internal/server/thing_test.go",
|
||||||
|
)
|
||||||
|
|
||||||
|
// The commit must actually contain BOTH paths. Rewriting a file with
|
||||||
|
// identical bytes records nothing, and this assertion would then be
|
||||||
|
// checking a test-only commit while appearing to check a mixed one.
|
||||||
|
touched := gitOutput(t, dir, "show", "--name-only", "--format=", "HEAD")
|
||||||
|
for _, want := range []string{"internal/server/thing.go", "internal/server/thing_test.go"} {
|
||||||
|
if !strings.Contains(touched, want) {
|
||||||
|
t.Fatalf("fixture is wrong: HEAD does not contain %s\n%s", want, touched)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
out, err := versionIn(t, dir)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("version.sh failed: %v\n%s", err, out)
|
||||||
|
}
|
||||||
|
if !strings.Contains(out, "name=2026.09.10.0042") {
|
||||||
|
t.Errorf("a source change accompanied by a test change did NOT move the version — "+
|
||||||
|
"the exclusions are matching directories rather than test files\n%s", out)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user