From d524da03375380604c9ee29ff9c7a2b7a2cb97c2 Mon Sep 17 00:00:00 2001 From: Christopher Langton Date: Thu, 12 Nov 2026 21:27:06 +1100 Subject: [PATCH] test(imagesrc,apispecs,managedfile): cover the untested helper rules - imagesrc: metaFor for tag and digest refs, parsePlatform (default, arch, variant, malformed), explainRegistryError (auth, schema 1, plain), registryOf, and Image.Close on nil and without cleanup. - apispecs: Discover skips empty, oversized (sparse file) and unparsable spec files without stopping the walk and returns path order; Summary operation counts; plural and itoa. - managedfile: File.Mode, ownsContent, and RemoveFile dry runs in every mode report the outcome without touching the file or its backup. The plural case found that Summary printed "2 losss": plural appended a bare s to "loss". Words ending in s now take es. Closes #289 --- internal/apispecs/apispecs.go | 7 +- internal/apispecs/summary_test.go | 109 ++++++++++++++ internal/imagesrc/imagesrc_helpers_test.go | 99 +++++++++++++ internal/managedfile/mode_dryrun_test.go | 165 +++++++++++++++++++++ 4 files changed, 379 insertions(+), 1 deletion(-) create mode 100644 internal/apispecs/summary_test.go create mode 100644 internal/imagesrc/imagesrc_helpers_test.go create mode 100644 internal/managedfile/mode_dryrun_test.go diff --git a/internal/apispecs/apispecs.go b/internal/apispecs/apispecs.go index a705d538f5..df256da2fa 100644 --- a/internal/apispecs/apispecs.go +++ b/internal/apispecs/apispecs.go @@ -114,7 +114,12 @@ func Summary(f Found) string { func plural(n int, word string) string { s := itoa(n) + " " + word if n != 1 { - s += "s" + // Summary pluralises "loss" as well as "operation". + if strings.HasSuffix(word, "s") { + s += "es" + } else { + s += "s" + } } return s } diff --git a/internal/apispecs/summary_test.go b/internal/apispecs/summary_test.go new file mode 100644 index 0000000000..b0766b284d --- /dev/null +++ b/internal/apispecs/summary_test.go @@ -0,0 +1,109 @@ +package apispecs + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +const onePetSpec = ` +openapi: 3.0.3 +info: {title: Pets, version: "1"} +paths: + /pets: + get: {responses: {"200": {description: ok}}} +` + +func TestDiscoverSkipsEmptyOversizedAndBrokenFiles(t *testing.T) { + dir := t.TempDir() + write(t, dir, "a/openapi.yaml", onePetSpec) + // Zero bytes, under a spec name: skipped, not an error. + write(t, dir, "b/openapi.json", "") + // A spec name whose content does not parse: skipped, and the walk goes on. + write(t, dir, "c/openapi.yaml", "openapi: [this is: not: valid") + write(t, dir, "d/swagger.yaml", ` +swagger: "2.0" +info: {title: Legacy, version: "1"} +paths: + /a: {get: {responses: {"200": {description: ok}}}} + /b: {post: {responses: {"200": {description: ok}}}} +`) + // Over the size cap: never read. A sparse file costs no disk. + big := filepath.Join(dir, "e", "openapi.json") + if err := os.MkdirAll(filepath.Dir(big), 0o755); err != nil { + t.Fatal(err) + } + f, err := os.Create(big) + if err != nil { + t.Fatal(err) + } + if err := f.Truncate(maxSpecBytes + 1); err != nil { + t.Fatal(err) + } + _ = f.Close() + + found, err := Discover(dir) + if err != nil { + t.Fatal(err) + } + if got := paths(found); strings.Join(got, ",") != "a/openapi.yaml,d/swagger.yaml" { + t.Fatalf("found %v, want the two valid specs in path order", got) + } +} + +func TestSummary(t *testing.T) { + dir := t.TempDir() + write(t, dir, "one/openapi.yaml", onePetSpec) + write(t, dir, "two/openapi.yaml", ` +openapi: 3.0.3 +info: {title: Pets, version: "1"} +paths: + /pets: + get: {responses: {"200": {description: ok}}} + post: {responses: {"200": {description: ok}}} +`) + found, err := Discover(dir) + if err != nil { + t.Fatal(err) + } + if len(found) != 2 { + t.Fatalf("found %v", paths(found)) + } + + one := Summary(found[0]) + if !strings.HasPrefix(one, "one/openapi.yaml: ") || !strings.HasSuffix(one, ", 1 operation") { + t.Errorf("Summary = %q, want the path and a singular operation count", one) + } + two := Summary(found[1]) + if !strings.HasSuffix(two, ", 2 operations") { + t.Errorf("Summary = %q, want a plural operation count", two) + } + if strings.Contains(one, "loss") { + t.Errorf("no losses means no losses clause: %q", one) + } +} + +func TestPluralAndItoa(t *testing.T) { + for n, want := range map[int]string{0: "0", 7: "7", 10: "10", 12345: "12345"} { + if got := itoa(n); got != want { + t.Errorf("itoa(%d) = %q, want %q", n, got, want) + } + } + for _, tt := range []struct { + n int + word string + want string + }{ + {0, "operation", "0 operations"}, + {1, "operation", "1 operation"}, + {3, "operation", "3 operations"}, + // Summary counts conversion losses; "losss" is what a bare +s makes. + {1, "loss", "1 loss"}, + {2, "loss", "2 losses"}, + } { + if got := plural(tt.n, tt.word); got != tt.want { + t.Errorf("plural(%d, %q) = %q, want %q", tt.n, tt.word, got, tt.want) + } + } +} diff --git a/internal/imagesrc/imagesrc_helpers_test.go b/internal/imagesrc/imagesrc_helpers_test.go new file mode 100644 index 0000000000..27a03fe44f --- /dev/null +++ b/internal/imagesrc/imagesrc_helpers_test.go @@ -0,0 +1,99 @@ +package imagesrc + +import ( + "errors" + "fmt" + "strings" + "testing" + + "github.com/google/go-containerregistry/pkg/name" + "github.com/google/go-containerregistry/pkg/v1/remote" +) + +func TestMetaFor(t *testing.T) { + tag, err := name.ParseReference("ghcr.io/acme/app:1.2.3") + if err != nil { + t.Fatal(err) + } + m := metaFor("ghcr.io/acme/app:1.2.3", tag) + if m.Ref != "ghcr.io/acme/app:1.2.3" || m.Registry != "ghcr.io" || m.Repository != "acme/app" || m.Tag != "1.2.3" || m.ManifestListDigest != "" { + t.Errorf("tag ref meta = %+v", m) + } + + const digest = "sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + dig, err := name.ParseReference("alpine@" + digest) + if err != nil { + t.Fatal(err) + } + m = metaFor("alpine@"+digest, dig) + if m.Registry != "index.docker.io" || m.Repository != "library/alpine" || m.ManifestListDigest != digest || m.Tag != "" { + t.Errorf("digest ref meta = %+v", m) + } +} + +func TestParsePlatform(t *testing.T) { + p, err := parsePlatform("") + if err != nil || p.OS != "linux" || p.Architecture != hostArch() { + t.Errorf("empty platform = %+v, %v; want linux/%s", p, err, hostArch()) + } + + p, err = parsePlatform("linux/arm64") + if err != nil || p.OS != "linux" || p.Architecture != "arm64" { + t.Errorf("linux/arm64 = %+v, %v", p, err) + } + + p, err = parsePlatform("linux/arm/v7") + if err != nil || p.Variant != "v7" { + t.Errorf("linux/arm/v7 = %+v, %v", p, err) + } + + if _, err := parsePlatform("linux/arm/v7/extra"); err == nil || !strings.Contains(err.Error(), `platform "linux/arm/v7/extra"`) { + t.Errorf("a malformed platform must fail naming it, got %v", err) + } +} + +func TestExplainRegistryError(t *testing.T) { + const ref = "ghcr.io/acme/app:1" + for _, msg := range []string{"UNAUTHORIZED: authentication required", "DENIED: requested access to the resource is denied", "unexpected status code 401"} { + raw := errors.New(msg) + err := explainRegistryError(ref, raw) + if !strings.Contains(err.Error(), "docker login ghcr.io") || !errors.Is(err, raw) { + t.Errorf("%q: got %v", msg, err) + } + } + + schema1 := fmt.Errorf("wrapped: %w", remote.ErrSchema1) + if err := explainRegistryError(ref, schema1); !strings.Contains(err.Error(), "retired Docker schema 1") || !errors.Is(err, remote.ErrSchema1) { + t.Errorf("schema 1: got %v", err) + } + + plain := errors.New("connection reset") + if err := explainRegistryError(ref, plain); err.Error() != "fetch ghcr.io/acme/app:1: connection reset" { + t.Errorf("plain error: got %v", err) + } +} + +func TestRegistryOf(t *testing.T) { + for ref, want := range map[string]string{ + "ghcr.io/acme/app:1": "ghcr.io", + "alpine": "index.docker.io", + "localhost:5000/x:latest": "localhost:5000", + "Not A Valid Ref!!": "Not A Valid Ref!!", + } { + if got := registryOf(ref); got != want { + t.Errorf("registryOf(%q) = %q, want %q", ref, got, want) + } + } +} + +func TestImageCloseIsSafe(t *testing.T) { + var nilImage *Image + nilImage.Close() + (&Image{}).Close() + + called := 0 + (&Image{cleanup: func() { called++ }}).Close() + if called != 1 { + t.Errorf("cleanup ran %d times, want 1", called) + } +} diff --git a/internal/managedfile/mode_dryrun_test.go b/internal/managedfile/mode_dryrun_test.go new file mode 100644 index 0000000000..b8ce78ae74 --- /dev/null +++ b/internal/managedfile/mode_dryrun_test.go @@ -0,0 +1,165 @@ +package managedfile + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestFileMode(t *testing.T) { + merge := func(s string) (string, error) { return s, nil } + for _, tt := range []struct { + name string + f File + want Mode + }{ + {"merge wins over structured", File{Merge: merge, Structured: true}, ModeMerge}, + {"merge", File{Merge: merge}, ModeMerge}, + {"structured", File{Structured: true}, ModeStructured}, + {"neither", File{}, ModeBlock}, + } { + if got := tt.f.Mode(); got != tt.want { + t.Errorf("%s: Mode() = %v, want %v", tt.name, got, tt.want) + } + } +} + +func TestOwnsContent(t *testing.T) { + const host = "packages.vulnetix.com" + for _, tt := range []struct { + name string + marker string + existing string + host string + want bool + }{ + {"host alone", "", "url = https://packages.vulnetix.com/npm", host, true}, + {"marker alone", "# vulnetix-credentials", "# vulnetix-credentials\nsecret=x", host, true}, + {"marker without host argument", "# vulnetix-credentials", "# vulnetix-credentials", "", true}, + {"neither present", "# vulnetix-credentials", "registry=https://registry.npmjs.org", host, false}, + {"host and marker both empty", "", "anything at all", "", false}, + } { + if got := (File{Marker: tt.marker}).ownsContent(tt.existing, tt.host); got != tt.want { + t.Errorf("%s: ownsContent = %v, want %v", tt.name, got, tt.want) + } + } +} + +// dryRunRemove runs RemoveFile with dryRun and checks that neither the file +// nor its backup changed on disk. +func dryRunRemove(t *testing.T, f File, host string) RemoveOutcome { + t.Helper() + before, err := os.ReadFile(f.Path) + if err != nil { + t.Fatal(err) + } + bakBefore, bakErr := os.ReadFile(f.Path + BackupSuffix) + + out, err := RemoveFile(f, pkgMarkers, host, true) + if err != nil { + t.Fatal(err) + } + + after, err := os.ReadFile(f.Path) + if err != nil { + t.Fatalf("dry run removed the file: %v", err) + } + if string(after) != string(before) { + t.Fatalf("dry run rewrote the file:\n%q\n%q", before, after) + } + bakAfter, bakErrAfter := os.ReadFile(f.Path + BackupSuffix) + if (bakErr == nil) != (bakErrAfter == nil) || string(bakBefore) != string(bakAfter) { + t.Fatal("dry run changed the backup") + } + return out +} + +func writeFile(t *testing.T, path, body string) { + t.Helper() + if err := os.WriteFile(path, []byte(body), 0o600); err != nil { + t.Fatal(err) + } +} + +func TestRemoveFileDryRunReportsWithoutTouching(t *testing.T) { + const host = "packages.vulnetix.com" + dir := t.TempDir() + + t.Run("structured with backup is restored", func(t *testing.T) { + p := filepath.Join(dir, "settings.xml") + writeFile(t, p, "https://packages.vulnetix.com") + writeFile(t, p+BackupSuffix, "") + out := dryRunRemove(t, File{Path: p, Structured: true}, host) + if !out.Existed || !out.Configured || !out.Restored || out.Deleted || out.Stripped { + t.Errorf("outcome = %+v", out) + } + }) + + t.Run("structured of ours is deleted", func(t *testing.T) { + p := filepath.Join(dir, "pip.conf") + writeFile(t, p, "index-url = https://packages.vulnetix.com/pypi") + out := dryRunRemove(t, File{Path: p, Structured: true}, host) + if !out.Configured || !out.Deleted || out.Restored || out.Stripped { + t.Errorf("outcome = %+v", out) + } + }) + + t.Run("structured not ours is left", func(t *testing.T) { + p := filepath.Join(dir, "other.conf") + writeFile(t, p, "index-url = https://pypi.org/simple") + out := dryRunRemove(t, File{Path: p, Structured: true}, host) + if !out.Existed || out.Configured || out.Deleted { + t.Errorf("outcome = %+v", out) + } + }) + + t.Run("merge without backup is stripped", func(t *testing.T) { + p := filepath.Join(dir, "settings.json") + writeFile(t, p, `{"ours": true, "theirs": true}`) + f := File{ + Path: p, + Merge: func(s string) (string, error) { return s, nil }, + Strip: func(_, existing string) (string, bool) { + return strings.Replace(existing, `"ours": true, `, "", 1), true + }, + } + out := dryRunRemove(t, f, host) + if out.Mode != ModeMerge || !out.Configured || !out.Stripped || out.Deleted || out.Restored { + t.Errorf("outcome = %+v", out) + } + }) + + t.Run("merge with nothing to strip", func(t *testing.T) { + p := filepath.Join(dir, "plain.json") + writeFile(t, p, `{"theirs": true}`) + f := File{ + Path: p, + Merge: func(s string) (string, error) { return s, nil }, + Strip: func(_, existing string) (string, bool) { return existing, false }, + } + out := dryRunRemove(t, f, host) + if out.Configured || out.Stripped { + t.Errorf("outcome = %+v", out) + } + }) + + t.Run("block only ours is deleted", func(t *testing.T) { + p := filepath.Join(dir, "only.rc") + writeFile(t, p, Block(pkgMarkers, "export GOPROXY=x")+"\n") + out := dryRunRemove(t, File{Path: p}, host) + if !out.Configured || !out.Deleted || out.Stripped { + t.Errorf("outcome = %+v", out) + } + }) + + t.Run("block among user content is stripped", func(t *testing.T) { + p := filepath.Join(dir, "mixed.rc") + writeFile(t, p, "export PATH=/usr/bin\n"+Block(pkgMarkers, "export GOPROXY=x")+"\n") + writeFile(t, p+BackupSuffix, "never restored for a block") + out := dryRunRemove(t, File{Path: p}, host) + if !out.Configured || !out.Stripped || out.Deleted || out.Restored { + t.Errorf("outcome = %+v", out) + } + }) +}