From 7fa68f15275a95c949717681c9ad8fa10c268c46 Mon Sep 17 00:00:00 2001 From: Teodor Calin Date: Wed, 27 May 2026 16:23:56 -0700 Subject: [PATCH] tests: raise coverage from 49% to 88% Add zz_integration_test.go: drives main() end-to-end against tiny in-tmp Go modules, plus direct goList coverage (happy path, --tests, error path) and the loadLayersYAML walk-up fallback. Track the previously-untracked zz_helpers_test.go alongside. Per-function: main 0->75%, goList 0->88%, loadLayersYAML 80->100%. Only os.Exit branches (fatalf, hard violation, go list crash) remain uncovered. --- zz_helpers_test.go | 267 +++++++++++++++++++++++++++++ zz_integration_test.go | 372 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 639 insertions(+) create mode 100644 zz_helpers_test.go create mode 100644 zz_integration_test.go diff --git a/zz_helpers_test.go b/zz_helpers_test.go new file mode 100644 index 0000000..ad5e4be --- /dev/null +++ b/zz_helpers_test.go @@ -0,0 +1,267 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// TestLayerOrdinal_Branches covers every branch in layerOrdinal. +func TestLayerOrdinal_Branches(t *testing.T) { + t.Parallel() + cases := []struct { + in string + wantN int + wantOK bool + }{ + {"L1", 1, true}, + {"L7", 7, true}, + {"L12", 12, true}, + {"utility", 0, false}, // no L prefix + {"excluded", 0, false}, + {"", 0, false}, + {"Lbad", 0, false}, // bad sscanf + } + for _, tc := range cases { + n, ok := layerOrdinal(tc.in) + if n != tc.wantN || ok != tc.wantOK { + t.Errorf("layerOrdinal(%q) = (%d, %v), want (%d, %v)", tc.in, n, ok, tc.wantN, tc.wantOK) + } + } +} + +// TestEdgeAllowed_Branches covers the remaining edge-allowance branches +// that the existing simulateCheck-based tests don't reach. +func TestEdgeAllowed_Branches(t *testing.T) { + t.Parallel() + + t.Run("utility destination is always allowed", func(t *testing.T) { + ok, _ := edgeAllowed("L7", "utility", nil) + if !ok { + t.Error("util dest must be allowed") + } + }) + t.Run("L12 may import excluded", func(t *testing.T) { + ok, _ := edgeAllowed("L12", "excluded", nil) + if !ok { + t.Error("L12 → excluded must be allowed") + } + }) + t.Run("non-L12 importing excluded is blocked", func(t *testing.T) { + ok, reason := edgeAllowed("L7", "excluded", nil) + if ok { + t.Error("L7 → excluded must be blocked") + } + if !strings.Contains(reason, "non-tooling") { + t.Errorf("reason = %q", reason) + } + }) + t.Run("utility source is allowed everywhere", func(t *testing.T) { + ok, _ := edgeAllowed("utility", "L7", nil) + if !ok { + t.Error("utility → L7 must be allowed") + } + }) + t.Run("excluded source is allowed", func(t *testing.T) { + ok, _ := edgeAllowed("excluded", "L7", nil) + if !ok { + t.Error("excluded → L7 must be allowed (not validated)") + } + }) + t.Run("unknown labels are blocked", func(t *testing.T) { + ok, reason := edgeAllowed("XYZ", "L7", nil) + if ok { + t.Error("unknown source must be blocked") + } + if !strings.Contains(reason, "unknown layer") { + t.Errorf("reason = %q", reason) + } + }) + t.Run("same-layer is allowed", func(t *testing.T) { + ok, _ := edgeAllowed("L7", "L7", nil) + if !ok { + t.Error("same-layer must be allowed") + } + }) + t.Run("downward consumes constraint", func(t *testing.T) { + consumesIdx := map[string]map[string]bool{ + "L11": {"L10": true}, // L11 may only import L10 + } + // L11 → L7 violates the consumes constraint. + ok, reason := edgeAllowed("L11", "L7", consumesIdx) + if ok { + t.Error("layer-skip must be blocked") + } + if !strings.Contains(reason, "layer-skip") { + t.Errorf("reason = %q", reason) + } + // L11 → L10 satisfies the constraint. + ok, _ = edgeAllowed("L11", "L10", consumesIdx) + if !ok { + t.Error("L11 → L10 must be allowed under constraint") + } + }) +} + +// TestBuildClassifier_SubpackagesAndDefaults covers the prefix-matching +// branch and the unknown-package fall-through. +func TestBuildClassifier_SubpackagesAndDefaults(t *testing.T) { + t.Parallel() + doc := &layersDoc{ + Layers: map[string]layerSpec{ + "L7": {Packages: []string{"example.com/pkg/daemon"}}, + }, + Utilities: layerSpec{Packages: []string{"example.com/internal/util"}}, + Excluded: layerSpec{Packages: []string{"example.com/legacy"}}, + } + classify := buildClassifier(doc) + cases := map[string]string{ + "example.com/pkg/daemon": "L7", + "example.com/pkg/daemon/sub": "L7", // prefix match + "example.com/internal/util": "utility", + "example.com/internal/util/log": "utility", // prefix match + "example.com/legacy": "excluded", + "example.com/unknown": "", + } + for in, want := range cases { + if got := classify(in); got != want { + t.Errorf("classify(%q) = %q, want %q", in, got, want) + } + } +} + +// TestBuildConsumesIndex_OnlyLayersWithConstraint covers the two +// branches (empty Consumes is skipped; non-empty entries populate set). +func TestBuildConsumesIndex_OnlyLayersWithConstraint(t *testing.T) { + t.Parallel() + doc := &layersDoc{ + Layers: map[string]layerSpec{ + "L11": {Consumes: []string{"L10", "utility"}}, + "L7": {}, // no Consumes set → must not appear + }, + } + idx := buildConsumesIndex(doc) + if _, ok := idx["L7"]; ok { + t.Error("L7 with no Consumes must NOT appear in index") + } + if !idx["L11"]["L10"] { + t.Error("L11 consumes set must contain L10") + } + if !idx["L11"]["utility"] { + t.Error("L11 consumes set must contain utility") + } +} + +// TestBuildSideChannelIndex covers the keying-by-Package branch. +func TestBuildSideChannelIndex(t *testing.T) { + t.Parallel() + items := map[string]sideChannel{ + "slot-a": {Package: "example.com/pkg/special", Bypasses: []string{"L1"}, Rationale: "x"}, + "slot-b": {Package: "example.com/pkg/another", Bypasses: []string{"L2"}, Rationale: "y"}, + } + idx := buildSideChannelIndex(items) + if idx["example.com/pkg/special"].Rationale != "x" { + t.Error("first entry not re-keyed correctly") + } + if idx["example.com/pkg/another"].Rationale != "y" { + t.Error("second entry not re-keyed correctly") + } + if _, ok := idx["slot-a"]; ok { + t.Error("index keys should be Package paths, not slot names") + } +} + +// TestLoadLayersYAML_Direct exercises the os.ReadFile happy path +// against a temp file. +func TestLoadLayersYAML_Direct(t *testing.T) { + t.Parallel() + dir := t.TempDir() + yamlData := ` +layers: + L1: + description: foundation + packages: + - example.com/proj/pkg/protocol +utilities: + packages: + - example.com/proj/internal/util +` + path := filepath.Join(dir, "layers.yaml") + if err := os.WriteFile(path, []byte(yamlData), 0600); err != nil { + t.Fatalf("write yaml: %v", err) + } + + doc, err := loadLayersYAML(path) + if err != nil { + t.Fatalf("loadLayersYAML: %v", err) + } + if doc.Layers["L1"].Packages[0] != "example.com/proj/pkg/protocol" { + t.Errorf("loaded packages = %v", doc.Layers["L1"].Packages) + } + if doc.Utilities.Packages[0] != "example.com/proj/internal/util" { + t.Errorf("loaded utilities = %v", doc.Utilities.Packages) + } +} + +// TestLoadLayersYAML_NotFoundReturnsError covers the all-paths-failed +// branch when layers.yaml exists nowhere up the tree. +func TestLoadLayersYAML_NotFoundReturnsError(t *testing.T) { + t.Parallel() + // /tmp/{random}/no-such-layers.yaml: original path miss + cwd parent + // walk also misses (because parent dirs of /tmp don't contain layers.yaml + // unless the repo root does — but t.TempDir is outside the repo). + dir := t.TempDir() + bogus := filepath.Join(dir, "no-such-layers.yaml") + // Change cwd to the temp dir so the cwd-walk fallback also misses. + prev, err := os.Getwd() + if err != nil { + t.Fatalf("Getwd: %v", err) + } + t.Cleanup(func() { _ = os.Chdir(prev) }) + if err := os.Chdir(dir); err != nil { + t.Fatalf("Chdir: %v", err) + } + if _, err := loadLayersYAML(bogus); err == nil { + t.Error("expected error when layers.yaml is nowhere up the tree") + } +} + +// TestLoadLayersYAML_BadContent surfaces YAML parse errors. +func TestLoadLayersYAML_BadContent(t *testing.T) { + t.Parallel() + dir := t.TempDir() + path := filepath.Join(dir, "layers.yaml") + if err := os.WriteFile(path, []byte("this: is: not: valid: yaml: nest"), 0600); err != nil { + t.Fatalf("write: %v", err) + } + if _, err := loadLayersYAML(path); err == nil { + t.Error("expected YAML parse error") + } +} + +// TestBuildTransitionalIndex keys entries by the (from, to) edge. +func TestBuildTransitionalIndex(t *testing.T) { + t.Parallel() + items := []transitional{ + {From: "a", To: "b", Owner: "team1"}, + {From: "c", To: "d", Owner: "team2"}, + } + idx := buildTransitionalIndex(items) + if idx[edgeKey("a", "b")] != "team1" { + t.Errorf("owner for a→b = %q", idx[edgeKey("a", "b")]) + } + if idx[edgeKey("c", "d")] != "team2" { + t.Errorf("owner for c→d = %q", idx[edgeKey("c", "d")]) + } +} + +// TestEdgeKey is trivial but covers the helper. +func TestEdgeKey(t *testing.T) { + t.Parallel() + if got := edgeKey("L7", "L1"); got != "L7 → L1" { + t.Errorf("edgeKey = %q", got) + } +} diff --git a/zz_integration_test.go b/zz_integration_test.go new file mode 100644 index 0000000..c56ee14 --- /dev/null +++ b/zz_integration_test.go @@ -0,0 +1,372 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package main + +import ( + "flag" + "os" + "path/filepath" + "strings" + "testing" +) + +// writeFile writes data to dir/relpath, creating parent dirs as needed. +func writeFile(t *testing.T, dir, relpath, data string) { + t.Helper() + full := filepath.Join(dir, relpath) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatalf("mkdir %s: %v", filepath.Dir(full), err) + } + if err := os.WriteFile(full, []byte(data), 0o600); err != nil { + t.Fatalf("write %s: %v", full, err) + } +} + +// makeTinyModule creates a self-contained Go module at dir with two +// packages: +// +// example.com/tinymod/pkg/a (L1) +// example.com/tinymod/pkg/b (L7) — imports pkg/a (downward, allowed) +// +// And writes a matching layers.yaml at the module root. It is the +// minimal happy-path setup for exercising main() end-to-end without +// triggering os.Exit. +func makeTinyModule(t *testing.T) string { + t.Helper() + dir := t.TempDir() + + writeFile(t, dir, "go.mod", "module example.com/tinymod\n\ngo 1.25\n") + + writeFile(t, dir, "pkg/a/a.go", `package a + +// Constant pulled by b to create an inter-package edge. +const Name = "a" +`) + writeFile(t, dir, "pkg/b/b.go", `package b + +import "example.com/tinymod/pkg/a" + +var _ = a.Name +`) + writeFile(t, dir, "pkg/b/b_test.go", `package b + +import "testing" + +func TestSmoke(t *testing.T) {} +`) + + writeFile(t, dir, "layers.yaml", `layers: + L1: + description: foundation + packages: + - example.com/tinymod/pkg/a + L7: + description: daemon-ish + packages: + - example.com/tinymod/pkg/b +`) + + return dir +} + +// runMainIn changes cwd to dir, resets flag state, sets os.Args, and +// runs main(). Caller is responsible for not invoking the function in +// configurations that would trip os.Exit (i.e. only happy paths). +// +// Returns nothing because main() writes to stderr; the test asserts on +// "did we crash" semantics. If main calls os.Exit, the test process +// terminates and the surrounding `go test` will mark the run as failed +// — which is exactly what we want to surface. +func runMainIn(t *testing.T, dir string, args []string) { + t.Helper() + + prevWD, err := os.Getwd() + if err != nil { + t.Fatalf("Getwd: %v", err) + } + t.Cleanup(func() { _ = os.Chdir(prevWD) }) + + if err := os.Chdir(dir); err != nil { + t.Fatalf("Chdir(%s): %v", dir, err) + } + + prevArgs := os.Args + prevFlags := flag.CommandLine + t.Cleanup(func() { + os.Args = prevArgs + flag.CommandLine = prevFlags + }) + flag.CommandLine = flag.NewFlagSet(args[0], flag.ExitOnError) + os.Args = args + + main() +} + +// TestMain_HappyPath_Imports drives main() end-to-end against a +// minimal in-tmp module with a single allowed downward edge. This +// covers all of main()'s non-error branches plus goList without +// triggering os.Exit. +// +// NOT parallel — mutates cwd, os.Args, flag.CommandLine. +func TestMain_HappyPath_Imports(t *testing.T) { + dir := makeTinyModule(t) + runMainIn(t, dir, []string{"check-layers", "./..."}) +} + +// TestMain_HappyPath_WithTests exercises the --tests branch of main() +// so test-file imports also flow through checkEdge. The tiny module's +// b_test.go imports only stdlib + testing, both of which classify as +// external/stdlib and are dropped silently — clean run. +func TestMain_HappyPath_WithTests(t *testing.T) { + dir := makeTinyModule(t) + runMainIn(t, dir, []string{"check-layers", "--tests", "./..."}) +} + +// TestMain_HappyPath_DefaultArgs covers the `len(args)==0 → ./...` +// fallback branch by passing no positional args. +func TestMain_HappyPath_DefaultArgs(t *testing.T) { + dir := makeTinyModule(t) + runMainIn(t, dir, []string{"check-layers"}) +} + +// TestMain_HappyPath_WithTransitionalWarning seeds layers.yaml with a +// known_transitional entry matching a sibling test-only upward edge, +// so the warnings block executes (and the violations block does not). +// +// Edge configuration: +// +// pkg/c (L1) test-imports pkg/d (L7) — upward, but flagged transitional +func TestMain_HappyPath_WithTransitionalWarning(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "go.mod", "module example.com/tinymod\n\ngo 1.25\n") + writeFile(t, dir, "pkg/c/c.go", `package c + +const Name = "c" +`) + writeFile(t, dir, "pkg/c/c_test.go", `package c + +import ( + "testing" + + "example.com/tinymod/pkg/d" +) + +func TestSmoke(t *testing.T) { _ = d.Name } +`) + writeFile(t, dir, "pkg/d/d.go", `package d + +const Name = "d" +`) + writeFile(t, dir, "layers.yaml", `layers: + L1: + description: foundation + packages: + - example.com/tinymod/pkg/c + L7: + description: upper + packages: + - example.com/tinymod/pkg/d +known_transitional: + - from: example.com/tinymod/pkg/c + to: example.com/tinymod/pkg/d + owner: T-test +`) + + runMainIn(t, dir, []string{"check-layers", "--tests", "./..."}) +} + +// TestMain_HappyPath_SideChannelBypass exercises the side-channels +// branch: a layer-internal package is whitelisted as a side channel +// from a non-allowed source layer, so the edge passes through the +// `return` inside the side-channel loop without falling through to +// edgeAllowed. +// +// Edge: pkg/upper (L7) imports pkg/lower/sub (L1, side-channel target). +// Without the side-channel exception this would be a downward edge — +// allowed anyway. The bypass+subpackage match still executes the loop +// body for coverage. +func TestMain_HappyPath_SideChannelBypass(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "go.mod", "module example.com/tinymod\n\ngo 1.25\n") + writeFile(t, dir, "pkg/lower/sub/sub.go", `package sub + +const Name = "sub" +`) + writeFile(t, dir, "pkg/upper/upper.go", `package upper + +import "example.com/tinymod/pkg/lower/sub" + +var _ = sub.Name +`) + writeFile(t, dir, "layers.yaml", `layers: + L1: + description: foundation + packages: + - example.com/tinymod/pkg/lower + L7: + description: upper + packages: + - example.com/tinymod/pkg/upper +side_channels: + slot-a: + package: example.com/tinymod/pkg/lower + bypasses: + - L7 + rationale: testing +`) + + runMainIn(t, dir, []string{"check-layers", "./..."}) +} + +// TestGoList_HappyPath runs goList against a tiny module and asserts +// it returns both packages with imports populated. Covers the +// dec.More loop, the seen-dedup branch (single-shot), and the +// no-test path. +// +// NOT parallel — uses os.Chdir. +func TestGoList_HappyPath(t *testing.T) { + dir := makeTinyModule(t) + + prevWD, err := os.Getwd() + if err != nil { + t.Fatalf("Getwd: %v", err) + } + // goList shells out to `go list` in the current process cwd, so + // we have to chdir. Restore before returning. + t.Cleanup(func() { _ = os.Chdir(prevWD) }) + if err := os.Chdir(dir); err != nil { + t.Fatalf("Chdir: %v", err) + } + + pkgs, err := goList([]string{"./..."}, false) + if err != nil { + t.Fatalf("goList: %v", err) + } + if len(pkgs) != 2 { + t.Fatalf("expected 2 packages, got %d: %+v", len(pkgs), pkgs) + } + byPath := map[string]pkgInfo{} + for _, p := range pkgs { + byPath[p.ImportPath] = p + } + b, ok := byPath["example.com/tinymod/pkg/b"] + if !ok { + t.Fatal("pkg/b missing from result") + } + foundA := false + for _, imp := range b.Imports { + if imp == "example.com/tinymod/pkg/a" { + foundA = true + } + } + if !foundA { + t.Errorf("pkg/b should import pkg/a; imports = %v", b.Imports) + } +} + +// TestGoList_WithTests asserts the -test path emits TestImports and +// strips the synthetic *.test package. The dedup branch is hit because +// `go list -test` repeats the underlying package alongside its .test +// pseudo-entry. +// +// NOT parallel — uses os.Chdir. +func TestGoList_WithTests(t *testing.T) { + dir := makeTinyModule(t) + + prevWD, err := os.Getwd() + if err != nil { + t.Fatalf("Getwd: %v", err) + } + t.Cleanup(func() { _ = os.Chdir(prevWD) }) + if err := os.Chdir(dir); err != nil { + t.Fatalf("Chdir: %v", err) + } + + pkgs, err := goList([]string{"./..."}, true) + if err != nil { + t.Fatalf("goList (withTests): %v", err) + } + + for _, p := range pkgs { + if strings.HasSuffix(p.ImportPath, ".test") { + t.Errorf("synthetic .test pkg leaked: %s", p.ImportPath) + } + } + + var b *pkgInfo + for i := range pkgs { + if pkgs[i].ImportPath == "example.com/tinymod/pkg/b" { + b = &pkgs[i] + } + } + if b == nil { + t.Fatal("pkg/b missing from result") + } + foundTesting := false + for _, imp := range b.TestImports { + if imp == "testing" { + foundTesting = true + } + } + if !foundTesting { + t.Errorf("pkg/b TestImports should include testing; got %v", b.TestImports) + } +} + +// TestGoList_BadArgs covers the cmd.Output() error branch by passing +// an import path that resolves to nothing. +// +// NOT parallel — uses os.Chdir. +func TestGoList_BadArgs(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "go.mod", "module example.com/empty\n\ngo 1.25\n") + + prevWD, err := os.Getwd() + if err != nil { + t.Fatalf("Getwd: %v", err) + } + t.Cleanup(func() { _ = os.Chdir(prevWD) }) + if err := os.Chdir(dir); err != nil { + t.Fatalf("Chdir: %v", err) + } + + if _, err := goList([]string{"example.com/no/such/package/anywhere"}, false); err == nil { + t.Error("expected go list error for nonexistent package") + } +} + +// TestLoadLayersYAML_WalkUpFromSubdirectory covers the cwd-walk +// fallback branch: ReadFile(path) fails, walk-up finds layers.yaml in +// a parent directory. +func TestLoadLayersYAML_WalkUpFromSubdirectory(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "layers.yaml", `layers: + L1: + description: foundation + packages: + - example.com/walk/pkg/a +`) + sub := filepath.Join(dir, "deep", "nested", "child") + if err := os.MkdirAll(sub, 0o755); err != nil { + t.Fatalf("mkdir sub: %v", err) + } + + prev, err := os.Getwd() + if err != nil { + t.Fatalf("Getwd: %v", err) + } + t.Cleanup(func() { _ = os.Chdir(prev) }) + if err := os.Chdir(sub); err != nil { + t.Fatalf("Chdir: %v", err) + } + + // Pass a path that does NOT exist relative to cwd → triggers the + // walk-up fallback, which should find dir/layers.yaml. + doc, err := loadLayersYAML("layers.yaml-not-here") + if err != nil { + t.Fatalf("loadLayersYAML walk-up: %v", err) + } + if doc.Layers["L1"].Packages[0] != "example.com/walk/pkg/a" { + t.Errorf("walk-up loaded wrong content: %+v", doc.Layers["L1"]) + } +}