Read the trunk's copy only from MESH_CHECK_BESIDE/mesh-catalog: unset skips the comparison saying NOT COMPARED, set without the copy fails (issue 1293, E5a)

checks/config/realtree_test.go

@@ -2,15 +2,16 @@ package main
 
 import (
 	"path/filepath"
-	"strings"
 	"testing"
 
 	"checks/trunk"
 )
 
-// The script's run of this check over the real tree (novox/hq ADR 0529 clause 2): the list of modules not yet moved gains none
-// the trunk's copy at the merge base does not hold. Without a trunk copy the modules in seed are the trunk's.
+// The script's comparison of this list with the trunk's (novox/hq ADR 0529 clause 2): the list of modules not yet moved gains none
+// the trunk's copy at the merge base does not hold. The catalogue itself is judged against the list by
+// TestTheCatalogueSaysConfigInEveryMovedModule. Skipped, and said, where no trunk is beside.
 func TestTheUnmovedListHoldsNoModuleTheTrunksDoesNot(t *testing.T) {
+	path, ok := trunk.Copy(t, "checks/config/unmoved")
 	found, err := Check(filepath.Join("..", ".."))
 	if err != nil {
 		t.Fatal(err)
@@ -20,18 +21,12 @@ func TestTheUnmovedListHoldsNoModuleTheTrunksDoesNot(t *testing.T) {
 		t.Fatal(err)
 	}
 	base := seed
-	if path, ok := trunk.Copy(t, "checks/config/unmoved"); ok {
+	if ok {
 		if base, err = Unmoved(path); err != nil {
 			t.Fatal(err)
 		}
 	}
-	refused, stale, added := Judge(found, unmoved, base)
-	for _, m := range refused {
-		t.Errorf("%s says %s and is not on checks/config/unmoved", m, strings.Join(found[m], ", "))
-	}
-	for _, m := range stale {
-		t.Errorf("checks/config/unmoved names %s, which says no old name: take it out", m)
-	}
+	_, _, added := Judge(found, unmoved, base)
 	for _, m := range added {
 		t.Errorf("checks/config/unmoved gains %s, which the trunk's copy does not hold: the list only shrinks", m)
 	}

checks/owned/realtree_test.go

@@ -6,11 +6,23 @@ import (
 	"checks/trunk"
 )
 
-// The script's run of this check over the real tree (novox/hq ADR 0529 clause 2): every rule holds, and the system-known list
-// holds no line the trunk's copy at the merge base does not. A trunk with none had no list: its first copy.
-func TestTheCatalogueKeepsEveryRuleOfOwnedPathsAndTheSystemListOnlyShrinks(t *testing.T) {
+// The script's run of this check over the real tree (novox/hq ADR 0529 clause 2): every rule holds.
+func TestTheCatalogueKeepsEveryRuleOfOwnedPaths(t *testing.T) {
+	found, err := Check("../..", nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, f := range found {
+		t.Errorf("%s: %s: %s (rule %d, novox/hq ADR 0422 §8)", f.Module, f.ID, f.Why, f.Rule)
+	}
+}
+
+// And the list of system directories' known exceptions holds no line the trunk's copy at the merge base does not (rule 5). A trunk
+// with none had no list: its first copy. Skipped, and said, where no trunk is beside.
+func TestTheSystemListHoldsNoLineTheTrunksDoesNot(t *testing.T) {
+	path, ok := trunk.Copy(t, "checks/owned/system-known")
 	var base []string
-	if path, ok := trunk.Copy(t, "checks/owned/system-known"); ok {
+	if ok {
 		lines, err := readLines(path)
 		if err != nil {
 			t.Fatal(err)
@@ -22,6 +34,8 @@ func TestTheCatalogueKeepsEveryRuleOfOwnedPathsAndTheSystemListOnlyShrinks(t *te
 		t.Fatal(err)
 	}
 	for _, f := range found {
-		t.Errorf("%s: %s: %s (rule %d, novox/hq ADR 0422 §8)", f.Module, f.ID, f.Why, f.Rule)
+		if f.Rule == 5 {
+			t.Errorf("%s: %s: %s (rule %d, issue 1841)", f.Module, f.ID, f.Why, f.Rule)
+		}
 	}
 }

checks/paths/realtree_test.go

@@ -8,18 +8,18 @@ import (
 )
 
 // The script's run of this check over the real tree (novox/hq ADR 0529 clause 2): no module names another's state on its machine,
-// the known list holds only what still occurs, and neither list gains a line the trunk's copies at the merge base do not hold.
-func TestNoModuleNamesAnothersStateAndTheListsOnlyShrink(t *testing.T) {
+// and the known list holds only what still occurs.
+func TestNoModuleNamesAnothersState(t *testing.T) {
 	root := filepath.Join("..", "..")
 	found, err := Check(root)
 	if err != nil {
 		t.Fatal(err)
 	}
-	shared, sharedWhy, err := list("shared")
+	shared, _, err := list("shared")
 	if err != nil {
 		t.Fatal(err)
 	}
-	known, knownWhy, err := list("known")
+	known, _, err := list("known")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -30,12 +30,25 @@ func TestNoModuleNamesAnothersStateAndTheListsOnlyShrink(t *testing.T) {
 	for _, k := range gone {
 		t.Errorf("%q is in checks/paths/known and no longer occurs: take it out, so it cannot come back", k)
 	}
+}
+
+// And neither list gains a line the trunk's copies at the merge base do not hold, unless the line says why after `# allow:`. Skipped,
+// and said, where no trunk is beside.
+func TestTheListsOfPathsHoldNoLineTheTrunksDoNot(t *testing.T) {
 	sharedPath, okShared := trunk.Copy(t, "checks/paths/shared")
 	knownPath, okKnown := trunk.Copy(t, "checks/paths/known")
 	if !okShared || !okKnown {
-		t.Log("NOT COMPARED: the trunk's lists were not both found, so additions to them were not judged")
+		t.Log("the trunk holds not both lists: their first copies, to which nothing was added")
 		return
 	}
+	shared, sharedWhy, err := list("shared")
+	if err != nil {
+		t.Fatal(err)
+	}
+	known, knownWhy, err := list("known")
+	if err != nil {
+		t.Fatal(err)
+	}
 	baseShared, _, err := list(sharedPath)
 	if err != nil {
 		t.Fatal(err)

checks/templates/realtree_test.go

@@ -6,9 +6,11 @@ import (
 	"checks/trunk"
 )
 
-// The script's run of this check over the real tree (novox/hq ADR 0529 clause 2): the list of modules keeping file text inline
-// gains none the trunk's copy at the merge base does not hold. Without a trunk copy the modules in seed are the trunk's.
+// The script's comparison of this list with the trunk's (novox/hq ADR 0529 clause 2): the list of modules keeping file text inline
+// gains none the trunk's copy at the merge base does not hold. The catalogue itself is judged against the list by
+// TestTheCatalogueKeepsFileTextInlineOnlyWhereListed. Skipped, and said, where no trunk is beside.
 func TestTheInlineListHoldsNoModuleTheTrunksDoesNot(t *testing.T) {
+	path, ok := trunk.Copy(t, "checks/templates/inline")
 	found, err := Check("../..")
 	if err != nil {
 		t.Fatal(err)
@@ -18,18 +20,12 @@ func TestTheInlineListHoldsNoModuleTheTrunksDoesNot(t *testing.T) {
 		t.Fatal(err)
 	}
 	base := seed
-	if path, ok := trunk.Copy(t, "checks/templates/inline"); ok {
+	if ok {
 		if base, err = Listed(path); err != nil {
 			t.Fatal(err)
 		}
 	}
-	refused, stale, added := Judge(found, listed, base)
-	for _, m := range refused {
-		t.Error(Refusal(m, found[m], TemplatesDir("../..", m)))
-	}
-	for _, m := range stale {
-		t.Errorf("checks/templates/inline names %s, which keeps no file text inline: take it out", m)
-	}
+	_, _, added := Judge(found, listed, base)
 	for _, m := range added {
 		t.Errorf("checks/templates/inline gains %s, which the trunk's copy does not hold: the list only shrinks", m)
 	}

checks/trunk/trunk.go

@@ -1,19 +1,17 @@
 // Package trunk is where a catalogue check finds the trunk's copy of a list that may only shrink (novox/hq ADR 0529 clause 2):
 // the file as the catalogue's base commit holds it, to judge a change's list against.
 //
-// A check in a merge check finds it in the clone of the change's own repository at its base commit, which the checker places
-// beside the change in MESH_CHECK_BESIDE as it places a sibling, under the repository's name. Run by hand, without that
-// directory, it reads the file from git at the merge base with origin/<MESH_CHECK_BASE, main when unset>, as the script this
-// replaces did. A trunk that holds no such file had none: the list's first copy, which nothing was added to. Where no base
-// can be found at all, the comparison is not made, and the test says so in its log: NOT COMPARED, never a pass that looks like one.
+// The checker fetches the change's own repository at its base commit beside the change, as it fetches a sibling, and says where
+// in MESH_CHECK_BESIDE; the copy is under the repository's name (the same convention the other repositories' tests use). With
+// MESH_CHECK_BESIDE unset the comparison is skipped, and the test says NOT COMPARED WITH THE TRUNK. Set, without the catalogue's
+// copy in it, the test fails loudly: a check that cannot compare must not look as if it had. A trunk that holds no such file had
+// none: the list's first copy, which nothing was added to.
 package trunk
 
 import (
 	"errors"
 	"os"
-	"os/exec"
 	"path/filepath"
-	"strings"
 	"testing"
 )
 
@@ -23,16 +21,19 @@ const Repository = "mesh-catalog"
 // BesideEnv is where a merge check says the clones beside the one checked are.
 const BesideEnv = "MESH_CHECK_BESIDE"
 
-// Copy is the trunk's copy of the file at rel (a path from the catalogue's root), written to a file of the test's own: its
-// path, and whether there is one. False with a log line when the trunk holds none or cannot be read; the caller then judges
-// the list as a first copy.
+// Copy is the trunk's copy of the file at rel (a path from the catalogue's root), written to a file of the test's own: its path,
+// and whether the trunk holds one (false: its first copy). It skips the test when no trunk is beside, and fails it when the
+// directory is named and holds no copy of the catalogue.
 func Copy(t testing.TB, rel string) (string, bool) {
 	t.Helper()
-	raw, found, how := Read(rel, "../..")
-	if how != "" {
-		t.Log(how)
-	}
-	if !found {
+	raw, found, skip, err := Read(rel)
+	switch {
+	case skip != "":
+		t.Skip(skip)
+	case err != nil:
+		t.Fatal(err)
+	case !found:
+		t.Logf("the trunk holds no %s: its first copy", rel)
 		return "", false
 	}
 	path := filepath.Join(t.TempDir(), filepath.Base(rel))
@@ -42,49 +43,25 @@ func Copy(t testing.TB, rel string) (string, bool) {
 	return path, true
 }
 
-// Read is the trunk's copy of rel, whether the trunk holds one, and a line to log when the comparison was not made or
-// where it was made. root is the catalogue's checkout (for git).
-func Read(rel, root string) (content []byte, found bool, say string) {
-	if beside := os.Getenv(BesideEnv); beside != "" {
-		dir := filepath.Join(beside, Repository)
-		if _, err := os.Stat(dir); err == nil {
-			raw, err := os.ReadFile(filepath.Join(dir, rel))
-			if errors.Is(err, os.ErrNotExist) {
-				return nil, false, "the trunk holds no " + rel + " (read from " + dir + "): its first copy"
-			}
-			if err != nil {
-				return nil, false, "NOT COMPARED WITH THE TRUNK: " + err.Error()
-			}
-			return raw, true, "judged against the trunk's " + rel + " in " + dir
-		}
+// Read is the trunk's copy of rel and whether the trunk holds one. skip is the words to skip the comparison with, when no trunk is
+// named; err is a trunk named and not there, or unreadable.
+func Read(rel string) (content []byte, found bool, skip string, err error) {
+	beside := os.Getenv(BesideEnv)
+	if beside == "" {
+		return nil, false, "NOT COMPARED WITH THE TRUNK: " + BesideEnv + " is not set, so the trunk's copy of " + rel +
+			" is not beside this run; the checker places it", nil
+	}
+	dir := filepath.Join(beside, Repository)
+	if _, err := os.Stat(dir); err != nil {
+		return nil, false, "", errors.New(Repository + " is not beside this check in " + BesideEnv + "=" + beside +
+			", so " + rel + " cannot be judged against the trunk's copy: " + err.Error())
+	}
+	raw, err := os.ReadFile(filepath.Join(dir, rel))
+	if errors.Is(err, os.ErrNotExist) {
+		return nil, false, "", nil
 	}
-	base := "origin/" + firstOf(os.Getenv("MESH_CHECK_BASE"), "main")
-	mergeBase, err := git(root, "merge-base", "HEAD", base)
 	if err != nil {
-		return nil, false, "NOT COMPARED WITH THE TRUNK: no merge base with " + base + ", and no clone of the trunk in " + BesideEnv +
-			"; " + rel + " is held to the list it was first written with"
+		return nil, false, "", err
 	}
-	raw, err := git(root, "show", strings.TrimSpace(string(mergeBase))+":"+rel)
-	if err != nil {
-		if _, exists := git(root, "cat-file", "-e", strings.TrimSpace(string(mergeBase))+":"+rel); exists != nil {
-			return nil, false, "the trunk holds no " + rel + " at the merge base: its first copy"
-		}
-		return nil, false, "NOT COMPARED WITH THE TRUNK: " + err.Error()
-	}
-	return raw, true, "judged against the trunk's " + rel + " at the merge base with " + base
-}
-
-func git(dir string, args ...string) ([]byte, error) {
-	cmd := exec.Command("git", args...)
-	cmd.Dir = dir
-	return cmd.Output()
-}
-
-func firstOf(s ...string) string {
-	for _, x := range s {
-		if x != "" {
-			return x
-		}
-	}
-	return ""
+	return raw, true, "", nil
 }

checks/trunk/trunk_test.go

@@ -16,27 +16,26 @@ func TestTheCloneBesideTheCheckIsTheTrunk(t *testing.T) {
 		t.Fatal(err)
 	}
 	t.Setenv(BesideEnv, beside)
-	raw, found, say := Read("checks/x/known", t.TempDir())
-	if !found || string(raw) != "a\nb\n" || say == "" {
-		t.Fatalf("read %q found %v, said %q", raw, found, say)
+	raw, found, skip, err := Read("checks/x/known")
+	if err != nil || skip != "" || !found || string(raw) != "a\nb\n" {
+		t.Fatalf("read %q found %v skip %q err %v", raw, found, skip, err)
 	}
-	if _, found, say := Read("checks/x/absent", t.TempDir()); found || say == "" {
-		t.Fatalf("a file the trunk does not hold was found (%v), said %q", found, say)
+	if _, found, skip, err := Read("checks/x/absent"); err != nil || skip != "" || found {
+		t.Fatalf("a file the trunk does not hold: found %v skip %q err %v; wanted its first copy", found, skip, err)
 	}
 }
 
-func TestWithNoCloneAndNoGitTheComparisonIsSaidNotMade(t *testing.T) {
+func TestWithNothingBesideTheComparisonIsSkippedAndSaidNotMade(t *testing.T) {
 	t.Setenv(BesideEnv, "")
-	_, found, say := Read("checks/x/known", t.TempDir()) // not a git checkout
-	if found || say == "" || say[:12] != "NOT COMPARED" {
-		t.Fatalf("found %v, said %q; wanted NOT COMPARED", found, say)
+	_, found, skip, err := Read("checks/x/known")
+	if found || err != nil || len(skip) < 12 || skip[:12] != "NOT COMPARED" {
+		t.Fatalf("found %v, skip %q, err %v; wanted NOT COMPARED", found, skip, err)
 	}
 }
 
-func TestACloneBesideWithoutTheCatalogueFallsBackToGit(t *testing.T) {
+func TestACloneBesideWithoutTheCatalogueFailsLoudly(t *testing.T) {
 	t.Setenv(BesideEnv, t.TempDir()) // the siblings are there, the catalogue's own base is not
-	_, found, say := Read("checks/x/known", t.TempDir())
-	if found || say == "" {
-		t.Fatalf("found %v, said %q", found, say)
+	if _, _, skip, err := Read("checks/x/known"); err == nil || skip != "" {
+		t.Fatalf("skip %q err %v; wanted an error", skip, err)
 	}
 }