diff --git a/checks/config/realtree_test.go b/checks/config/realtree_test.go index b69602ce..32907d83 100644 --- a/checks/config/realtree_test.go +++ b/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) } diff --git a/checks/owned/realtree_test.go b/checks/owned/realtree_test.go index 1a21d906..80de61e4 100644 --- a/checks/owned/realtree_test.go +++ b/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) + } } } diff --git a/checks/paths/realtree_test.go b/checks/paths/realtree_test.go index 93167779..608afd52 100644 --- a/checks/paths/realtree_test.go +++ b/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) diff --git a/checks/templates/realtree_test.go b/checks/templates/realtree_test.go index 9aaebecc..223df33b 100644 --- a/checks/templates/realtree_test.go +++ b/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) } diff --git a/checks/trunk/trunk.go b/checks/trunk/trunk.go index 2b16893f..3c57cf8d 100644 --- a/checks/trunk/trunk.go +++ b/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/, 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 } diff --git a/checks/trunk/trunk_test.go b/checks/trunk/trunk_test.go index 6d6ab29f..ce4f994b 100644 --- a/checks/trunk/trunk_test.go +++ b/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) } }