From edb8d0abb33fa7a9c1f8cddaa23c3f0092de1288 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Sun, 25 Oct 2026 18:17:25 -0300 Subject: [PATCH 1/7] refactor(cli): drop checks no outcome depends on Mutation testing showed these edits change nothing a caller sees: skipJSONValue only meets opening delimiters at a value's position and can consume an object's keys like its values; orderedObjectKeys' empty input already fails the first token; ociAnnotationID's missing or empty annotation already fails to unmarshal; filepath.Localize only returns local paths, so the IsLocal check after it refuses nothing; every caller of downloadFeatureTarball passes a context; and the replacer's capacity hint is unobservable. The 512 MiB download cap is exempt, as only a 512 MiB response could show it move. Co-Authored-By: Claude Opus 5.5 --- internal/cli/collection_commands.go | 33 +++++++++++------------------ internal/cli/feature_deps.go | 12 +++++------ internal/cli/feature_install.go | 10 ++++----- 3 files changed, 22 insertions(+), 33 deletions(-) diff --git a/internal/cli/collection_commands.go b/internal/cli/collection_commands.go index 65c4fa8..0eda8aa 100644 --- a/internal/cli/collection_commands.go +++ b/internal/cli/collection_commands.go @@ -599,7 +599,7 @@ func generateDocs(projectFolder, registry, namespace, githubOwner, githubRepo, c } func renderDocsTemplate(template string, values map[string]string) string { - pairs := make([]string, 0, len(values)*2) + var pairs []string for name, value := range values { pairs = append(pairs, "#{"+name+"}", value) } @@ -707,15 +707,9 @@ func defaultCell(ov map[string]any) string { // orderedObjectKeys returns the keys of a JSON object in source order. func orderedObjectKeys(raw json.RawMessage) []string { - if len(raw) == 0 { - return nil - } dec := json.NewDecoder(bytes.NewReader(raw)) t, err := dec.Token() - if err != nil { - return nil - } - if d, ok := t.(json.Delim); !ok || d != '{' { + if err != nil || t != json.Delim('{') { return nil } var keys []string @@ -734,27 +728,24 @@ func orderedObjectKeys(raw json.RawMessage) []string { return keys } +// skipJSONValue consumes the next value from dec. At a value's position the +// decoder only ever yields an opening delimiter, so any Delim starts a +// container; an object's keys are string tokens, consumed like its values. func skipJSONValue(dec *json.Decoder) error { t, err := dec.Token() if err != nil { return err } - if d, ok := t.(json.Delim); ok && (d == '{' || d == '[') { - for dec.More() { - if d == '{' { - if _, err := dec.Token(); err != nil { // key - return err - } - } - if err := skipJSONValue(dec); err != nil { - return err - } - } - if _, err := dec.Token(); err != nil { // closing delim + if _, ok := t.(json.Delim); !ok { + return nil + } + for dec.More() { + if err := skipJSONValue(dec); err != nil { return err } } - return nil + _, err = dec.Token() // closing delim + return err } // createTarArchive creates a plain (uncompressed) tar archive from a directory, diff --git a/internal/cli/feature_deps.go b/internal/cli/feature_deps.go index 9f2a55b..7cbd289 100644 --- a/internal/cli/feature_deps.go +++ b/internal/cli/feature_deps.go @@ -163,14 +163,14 @@ func featureMetadataFromTarball(blob []byte) (features.Feature, error) { return meta, nil } +// ociAnnotationID is the id in the manifest's metadata annotation, or "" when +// the annotation is missing, empty or not JSON (none of which unmarshal). func ociAnnotationID(manifest *oci.ManifestContainer) string { - if mj, ok := manifest.Manifest.Annotations["dev.containers.metadata"]; ok && mj != "" { - var meta features.Feature - if json.Unmarshal([]byte(mj), &meta) == nil { - return meta.ID - } + var meta features.Feature + if json.Unmarshal([]byte(manifest.Manifest.Annotations["dev.containers.metadata"]), &meta) != nil { + return "" } - return "" + return meta.ID } func readLocalFeatureMetadata(dir string) (features.Feature, error) { diff --git a/internal/cli/feature_install.go b/internal/cli/feature_install.go index f58c6c7..e8a7cfb 100644 --- a/internal/cli/feature_install.go +++ b/internal/cli/feature_install.go @@ -912,11 +912,12 @@ func extractTarGz(archivePath, destDir string) error { // Tar paths always use forward slashes. Localize rejects absolute paths, // parent traversal, and names that cannot be represented safely on the - // current platform. Root also prevents escapes through symlinks already - // present below destDir. + // current platform; what it returns is always local, as filepath.IsLocal + // reports. Root also prevents escapes through symlinks already present + // below destDir. cleanName := path.Clean(header.Name) localName, err := filepath.Localize(cleanName) - if err != nil || !filepath.IsLocal(localName) { + if err != nil { return fmt.Errorf("tar entry %q escapes the destination directory", header.Name) } @@ -992,9 +993,6 @@ func (e *digestMismatchError) Error() string { // It returns the tarball bytes and their computed "sha256:" digest so the // caller can pin/verify integrity (TS records and checks this digest). func downloadFeatureTarball(ctx context.Context, logger log.Logger, tarballURL string, env map[string]string) ([]byte, string, error) { - if ctx == nil { - ctx = context.Background() - } req, err := http.NewRequestWithContext(ctx, http.MethodGet, tarballURL, nil) if err != nil { return nil, "", err From 27ec2b34bd27c2a4c4f45f5a45ecb0a94a1e4143 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Sun, 25 Oct 2026 18:17:26 -0300 Subject: [PATCH 2/7] test(cli): hold the collection, metadata and tarball commands' edits A mutation run over these files left edits no test refused: the argument wiring of templates metadata, features info, generate-docs and features test, features test's flag conflicts, the template metadata failures, createTarArchive's entry names, empty user names and footer, extracted file modes, the 2xx-only tarball download and the per-user Linux cache folder. Each test asserts what a caller sees, without Docker, reusing readRegistry and httptest servers. Co-Authored-By: Claude Opus 5.5 --- .../cli/collection_commands_repo_held_test.go | 182 ++++++++++++++++++ internal/cli/disallowed_repo_held_test.go | 24 +++ .../cli/feature_install_repo_held_test.go | 65 +++++++ internal/cli/features_info_repo_held_test.go | 28 +++ .../cli/templates_metadata_repo_held_test.go | 69 +++++++ 5 files changed, 368 insertions(+) create mode 100644 internal/cli/collection_commands_repo_held_test.go create mode 100644 internal/cli/disallowed_repo_held_test.go create mode 100644 internal/cli/feature_install_repo_held_test.go create mode 100644 internal/cli/features_info_repo_held_test.go create mode 100644 internal/cli/templates_metadata_repo_held_test.go diff --git a/internal/cli/collection_commands_repo_held_test.go b/internal/cli/collection_commands_repo_held_test.go new file mode 100644 index 0000000..7183d1e --- /dev/null +++ b/internal/cli/collection_commands_repo_held_test.go @@ -0,0 +1,182 @@ +package cli + +import ( + "archive/tar" + "bytes" + "encoding/json" + "errors" + "io" + "os" + "path/filepath" + "slices" + "strings" + "testing" + + "github.com/spf13/cobra" +) + +// generate-docs documents the folder its positional argument names, else the +// one --project-folder names; the working directory (the default ".") is left +// alone either way. +func TestGenerateDocsCommandsProjectFolder(t *testing.T) { + t.Chdir(t.TempDir()) + writeFeature(t, ".", "decoy", `{"id":"decoy","version":"1.0.0","name":"Decoy"}`) + commands := map[string]struct { + cmd func() *cobra.Command + metadata string + flags []string + }{ + "features": {realFeaturesGenerateDocsCmd, "devcontainer-feature.json", []string{"--namespace", "o/r"}}, + "templates": {realTemplatesGenerateDocsCmd, "devcontainer-template.json", nil}, + } + for name, c := range commands { + for _, positional := range []bool{true, false} { + project := t.TempDir() + if err := os.WriteFile(filepath.Join(project, c.metadata), []byte(`{"id":"solo","version":"1.0.0","name":"Solo"}`), 0o644); err != nil { + t.Fatal(err) + } + args := []string{"--project-folder", project} + if positional { + args = []string{project} + } + cmd := c.cmd() + cmd.SilenceUsage, cmd.SilenceErrors = true, true + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + cmd.SetArgs(append(args, c.flags...)) + if err := cmd.Execute(); err != nil { + t.Fatalf("%s %v: %v", name, args, err) + } + if _, err := os.Stat(filepath.Join(project, "README.md")); err != nil { + t.Errorf("%s %v: README not generated in the project folder: %v", name, args, err) + } + } + } + if _, err := os.Stat(filepath.Join("decoy", "README.md")); !errors.Is(err, os.ErrNotExist) { + t.Errorf("the working directory was documented: %v", err) + } +} + +func runFeaturesTestCmd(t *testing.T, args ...string) (string, error) { + t.Helper() + cmd := realFeaturesTestCmd() + cmd.SilenceUsage, cmd.SilenceErrors = true, true + var out bytes.Buffer + cmd.SetOut(&out) + cmd.SetErr(&bytes.Buffer{}) + cmd.SetArgs(args) + err := cmd.Execute() + return out.String(), err +} + +// features test refuses the flag combinations TS refuses, before it looks at +// the collection; each flag alone is accepted and reaches the collection check. +func TestFeaturesTestCommandFlagConflicts(t *testing.T) { + empty := t.TempDir() + conflicts := map[string][]string{ + "Cannot combine --global-scenarios-only and --features": {"--global-scenarios-only", "--features", "a"}, + "Cannot combine --skip-scenarios and --global-scenarios-only": {"--global-scenarios-only", "--skip-scenarios"}, + } + for want, args := range conflicts { + if _, err := runFeaturesTestCmd(t, append(args, "-p", empty)...); err == nil || err.Error() != want { + t.Errorf("%v: err = %v, want %q", args, err, want) + } + } + for _, args := range [][]string{{"--features", "a"}, {"--global-scenarios-only"}, {"--skip-scenarios"}} { + if _, err := runFeaturesTestCmd(t, append(args, "-p", empty)...); exitCode(err) != 1 { + t.Errorf("%v: err = %v, want the collection check's exit 1", args, err) + } + } +} + +// features test checks the collection its positional target names, or +// --project-folder's when the target is missing or "."; a collection without +// src/ and test/ exits 1 naming the folder on stdout. +func TestFeaturesTestCommandTarget(t *testing.T) { + project, target := t.TempDir(), t.TempDir() + cases := []struct { + args []string + want string + }{ + {[]string{target, "-p", project}, target}, + {[]string{".", "-p", project}, project}, + {[]string{"-p", project}, project}, + } + for _, c := range cases { + out, err := runFeaturesTestCmd(t, c.args...) + if exitCode(err) != 1 { + t.Errorf("%v: err = %v, want exit 1", c.args, err) + } + if want := "[-] Folder '" + c.want + "' does not contain the required 'src' and 'test' folders.\n"; !strings.HasSuffix(out, want) { + t.Errorf("%v: stdout = %q, want it to end with %q", c.args, out, want) + } + } +} + +// orderedObjectKeys lists an object's own keys in source order, whatever its +// values nest, and nothing for anything but an object. +func TestOrderedObjectKeys(t *testing.T) { + raw := `{"z":{"x":"y","k":{"deep":[1,{"q":"r"}]}},"a":["s",{"t":"u"}],"m":"v","b":null}` + if got, want := orderedObjectKeys(json.RawMessage(raw)), []string{"z", "a", "m", "b"}; !slices.Equal(got, want) { + t.Errorf("keys = %q, want %q", got, want) + } + for _, raw := range []string{``, `["a","b"]`, `"a"`, `{`} { + if got := orderedObjectKeys(json.RawMessage(raw)); len(got) != 0 { + t.Errorf("%s: keys = %q, want none", raw, got) + } + } +} + +// createTarArchive roots the entries at "./", with forward slashes and a +// trailing slash on directories, and records no user or group names, as +// node-tar does. +func TestCreateTarArchiveEntries(t *testing.T) { + src := t.TempDir() + if err := os.MkdirAll(filepath.Join(src, "sub"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(src, "sub", "install.sh"), []byte("echo hi\n"), 0o644); err != nil { + t.Fatal(err) + } + archive := filepath.Join(t.TempDir(), "out.tgz") + if err := createTarArchive(archive, src); err != nil { + t.Fatal(err) + } + f, err := os.Open(archive) + if err != nil { + t.Fatal(err) + } + defer f.Close() + tr := tar.NewReader(f) + var names []string + for { + h, err := tr.Next() + if err == io.EOF { + break + } + if err != nil { + t.Fatal(err) + } + names = append(names, h.Name) + if h.Uname != "" || h.Gname != "" { + t.Errorf("%s: uname %q, gname %q, want both empty", h.Name, h.Uname, h.Gname) + } + if h.Name == "./sub/install.sh" { + if b, _ := io.ReadAll(tr); string(b) != "echo hi\n" { + t.Errorf("install.sh = %q", b) + } + } + } + if want := []string{"./", "./sub/", "./sub/install.sh"}; !slices.Equal(names, want) { + t.Errorf("entries = %q, want %q", names, want) + } + // The archive is finished: whole 512-byte blocks ending in the two zero + // blocks that mark the end of a tar. + data, err := os.ReadFile(archive) + if err != nil { + t.Fatal(err) + } + if len(data)%512 != 0 || len(data) < 1024 || !bytes.Equal(data[len(data)-1024:], make([]byte, 1024)) { + t.Errorf("archive of %d bytes does not end in the tar footer", len(data)) + } +} diff --git a/internal/cli/disallowed_repo_held_test.go b/internal/cli/disallowed_repo_held_test.go new file mode 100644 index 0000000..b4fe652 --- /dev/null +++ b/internal/cli/disallowed_repo_held_test.go @@ -0,0 +1,24 @@ +package cli + +import ( + "os" + "os/user" + "path/filepath" + "runtime" + "testing" +) + +// On Linux the cache folder carries the user's name, as TS getCacheFolder's +// does, so two users of one machine do not share it. +func TestCacheFolderNamesTheLinuxUser(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("the per-user folder is Linux's") + } + u, err := user.Current() + if err != nil || u.Username == "" { + t.Skipf("no current user name: %v", err) + } + if got, want := cacheFolder(), filepath.Join(os.TempDir(), "devcontainercli-"+u.Username); got != want { + t.Errorf("cacheFolder() = %q, want %q", got, want) + } +} diff --git a/internal/cli/feature_install_repo_held_test.go b/internal/cli/feature_install_repo_held_test.go new file mode 100644 index 0000000..3bb6e64 --- /dev/null +++ b/internal/cli/feature_install_repo_held_test.go @@ -0,0 +1,65 @@ +package cli + +import ( + "archive/tar" + "bytes" + "context" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" +) + +// A regular file keeps the mode its tar entry records, so a Feature's +// install.sh stays executable; an entry recording no mode keeps the mode the +// file was created with instead of becoming unreadable. +func TestExtractTarGzFileModes(t *testing.T) { + var buf bytes.Buffer + tw := tar.NewWriter(&buf) + for name, mode := range map[string]int64{"install.sh": 0o750, "nomode.txt": 0} { + if err := tw.WriteHeader(&tar.Header{Name: name, Mode: mode, Size: 2, Typeflag: tar.TypeReg}); err != nil { + t.Fatal(err) + } + if _, err := tw.Write([]byte("hi")); err != nil { + t.Fatal(err) + } + } + if err := tw.Close(); err != nil { + t.Fatal(err) + } + archive := filepath.Join(t.TempDir(), "feature.tar") + if err := os.WriteFile(archive, buf.Bytes(), 0o644); err != nil { + t.Fatal(err) + } + dest := t.TempDir() + if err := extractTarGz(archive, dest); err != nil { + t.Fatal(err) + } + if fi, err := os.Stat(filepath.Join(dest, "install.sh")); err != nil || fi.Mode().Perm() != 0o750 { + t.Errorf("install.sh: %v, %v, want mode 0750", fi.Mode(), err) + } + if b, err := os.ReadFile(filepath.Join(dest, "nomode.txt")); err != nil || string(b) != "hi" { + t.Errorf("nomode.txt: %q, %v, want it readable", b, err) + } +} + +// A tarball download succeeds only on a 2xx answer, as TS treats only +// 200-299 as success: a 300 fails it. +func TestDownloadFeatureTarballStatus(t *testing.T) { + for status, ok := range map[int]bool{200: true, 299: true, 300: false} { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(status) + _, _ = w.Write([]byte("tgz")) + })) + data, _, err := downloadFeatureTarball(context.Background(), nil, srv.URL+"/f.tgz", nil) + srv.Close() + if ok && (err != nil || string(data) != "tgz") { + t.Errorf("HTTP %d: %q, %v, want the body", status, data, err) + } + if !ok && (err == nil || !strings.Contains(err.Error(), "HTTP 300")) { + t.Errorf("HTTP %d: err = %v, want the status", status, err) + } + } +} diff --git a/internal/cli/features_info_repo_held_test.go b/internal/cli/features_info_repo_held_test.go new file mode 100644 index 0000000..08af65e --- /dev/null +++ b/internal/cli/features_info_repo_held_test.go @@ -0,0 +1,28 @@ +package cli + +import ( + "bytes" + "strings" + "testing" +) + +// info takes the mode from its first argument and the +// Feature from its second. +func TestFeaturesInfoCommandArguments(t *testing.T) { + reg := newReadRegistry(t) + reg.manifests["ns/tool"] = featureManifest("tool") + reg.tags["ns/tool"] = []string{"1.0.0"} + + cmd := realFeaturesInfoCmd() + cmd.SilenceUsage, cmd.SilenceErrors = true, true + var out bytes.Buffer + cmd.SetOut(&out) + cmd.SetErr(&bytes.Buffer{}) + cmd.SetArgs([]string{"tags", reg.host + "/ns/tool:1", "--output-format", "json"}) + if err := cmd.ExecuteContext(t.Context()); err != nil { + t.Fatal(err) + } + if got := strings.Join(strings.Fields(out.String()), ""); got != `{"publishedTags":["1.0.0"]}` { + t.Errorf("stdout = %s, want the Feature's tags", out.String()) + } +} diff --git a/internal/cli/templates_metadata_repo_held_test.go b/internal/cli/templates_metadata_repo_held_test.go new file mode 100644 index 0000000..9d32c70 --- /dev/null +++ b/internal/cli/templates_metadata_repo_held_test.go @@ -0,0 +1,69 @@ +package cli + +import ( + "bytes" + "strconv" + "strings" + "testing" +) + +// templateManifest is a Template manifest whose annotations are the given +// JSON object body. +func templateManifest(annotations string) []byte { + return []byte(`{"schemaVersion":2,"mediaType":"application/vnd.oci.image.manifest.v1+json",` + + `"config":{"mediaType":"application/vnd.devcontainers","digest":"sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a","size":2},` + + `"layers":[],"annotations":{` + annotations + `}}`) +} + +func runTemplatesMetadataCmd(t *testing.T, templateID string) (string, error) { + t.Helper() + cmd := realTemplatesMetadataCmd() + cmd.SilenceUsage, cmd.SilenceErrors = true, true + var out bytes.Buffer + cmd.SetOut(&out) + cmd.SetErr(&bytes.Buffer{}) + cmd.SetArgs([]string{templateID}) + err := cmd.ExecuteContext(t.Context()) + return out.String(), err +} + +// templates metadata prints the manifest's metadata annotation as +// JSON.stringify(JSON.parse(annotation)) does: compacted, keys in source order. +func TestTemplatesMetadataPrintsTheAnnotation(t *testing.T) { + reg := newReadRegistry(t) + metadata := `{ "id": "tpl", "version": "1.0.0", "a": [1, 2] }` + reg.manifests["ns/tpl"] = templateManifest(`"dev.containers.metadata":` + strconv.Quote(metadata)) + + got, err := runTemplatesMetadataCmd(t, reg.host+"/ns/tpl:latest") + if err != nil { + t.Fatal(err) + } + if want := `{"id":"tpl","version":"1.0.0","a":[1,2]}` + "\n"; got != want { + t.Errorf("stdout = %q, want %q", got, want) + } +} + +// Every failure prints an empty object on stdout, as TS does before exiting 1, +// and returns the reason. +func TestTemplatesMetadataFailures(t *testing.T) { + reg := newReadRegistry(t) + reg.manifests["ns/none"] = templateManifest(`"other":"x"`) + reg.manifests["ns/empty"] = templateManifest(`"dev.containers.metadata":""`) + reg.manifests["ns/broken"] = templateManifest(`"dev.containers.metadata":"{\"id\":"`) + cases := []struct{ name, id, want string }{ + {"unparsable id", "UPPER/Not A Ref", "parse template identifier"}, + {"no manifest", reg.host + "/ns/missing:latest", "fetch manifest for template"}, + {"no annotation", reg.host + "/ns/none:latest", "does not contain metadata"}, + {"empty annotation", reg.host + "/ns/empty:latest", "does not contain metadata"}, + {"invalid annotation", reg.host + "/ns/broken:latest", "has invalid metadata"}, + } + for _, c := range cases { + got, err := runTemplatesMetadataCmd(t, c.id) + if err == nil || !strings.Contains(err.Error(), c.want) { + t.Errorf("%s: err = %v, want %q", c.name, err, c.want) + } + if got != "{}\n" { + t.Errorf("%s: stdout = %q, want an empty object", c.name, got) + } + } +} From 551bec96646e6b7bc760d468683650ae70a7b3de Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Sun, 25 Oct 2026 18:04:49 -0300 Subject: [PATCH 3/7] fix(cli): fold the features lockfile into the cache key read-configuration --cache-key is documented to hash the committed features lockfile, but computeCacheKey only used the lockfile when ReadLockfile's bool was true, and that bool reports the empty marker file, which comes back with a nil lock. A lockfile with content was therefore never hashed, so changing the pinned digests kept the key. Use the lockfile whenever ReadLockfile returns one. Keys for configurations without a lockfile, or with an empty one, are unchanged. Co-Authored-By: Claude Opus 5.5 --- internal/cli/cache_key.go | 4 +- internal/cli/cache_key_repo_held_test.go | 62 ++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 internal/cli/cache_key_repo_held_test.go diff --git a/internal/cli/cache_key.go b/internal/cli/cache_key.go index c8e02a8..cae8bec 100644 --- a/internal/cli/cache_key.go +++ b/internal/cli/cache_key.go @@ -54,7 +54,9 @@ func computeCacheKey(result *config.LoadResult, env map[string]string) (string, } } - if lock, ok, _ := features.ReadLockfile(cfg.ConfigFilePath); ok && lock != nil { + // ReadLockfile's bool reports an empty marker file, which pins nothing; a + // lockfile with content comes back as a non-nil lock. + if lock, _, _ := features.ReadLockfile(cfg.ConfigFilePath); lock != nil { if lockJSON, lerr := json.Marshal(lock); lerr == nil { in.Lockfile = lockJSON } diff --git a/internal/cli/cache_key_repo_held_test.go b/internal/cli/cache_key_repo_held_test.go new file mode 100644 index 0000000..b85b43e --- /dev/null +++ b/internal/cli/cache_key_repo_held_test.go @@ -0,0 +1,62 @@ +package cli + +import ( + "os" + "path/filepath" + "testing" +) + +// A committed features lockfile pins resolved digests, so changing it changes +// the cache key (docs/go-only-features.md, read-configuration --cache-key); an +// empty lockfile, which pins nothing, keys like no lockfile at all. +func TestComputeCacheKeyFoldsInTheLockfile(t *testing.T) { + dir := t.TempDir() + cfgPath := filepath.Join(dir, "devcontainer.json") + lockPath := filepath.Join(dir, "devcontainer-lock.json") + key := func() string { + t.Helper() + k, err := computeCacheKey(imageResult("ubuntu:22.04", cfgPath), nil) + if err != nil { + t.Fatal(err) + } + return k + } + none := key() + + if err := os.WriteFile(lockPath, []byte(""), 0o644); err != nil { + t.Fatal(err) + } + if k := key(); k != none { + t.Error("an empty lockfile changed the key") + } + + lock := func(digest string) string { + return `{"features":{"ghcr.io/o/f:1":{"version":"1.0.0","resolved":"ghcr.io/o/f@` + digest + `","integrity":"` + digest + `"}}}` + } + if err := os.WriteFile(lockPath, []byte(lock("sha256:aaa")), 0o644); err != nil { + t.Fatal(err) + } + pinnedA := key() + if pinnedA == none { + t.Error("a lockfile did not change the key") + } + if err := os.WriteFile(lockPath, []byte(lock("sha256:bbb")), 0o644); err != nil { + t.Fatal(err) + } + if key() == pinnedA { + t.Error("a lockfile pinning another digest kept the key") + } +} + +// A proxy variable that is set but empty configures no proxy, so it keys like +// an unset one. +func TestComputeCacheKeyIgnoresEmptyProxy(t *testing.T) { + cfgPath := filepath.Join(t.TempDir(), "devcontainer.json") + base, err := computeCacheKey(imageResult("ubuntu:22.04", cfgPath), nil) + if err != nil { + t.Fatal(err) + } + if k, _ := computeCacheKey(imageResult("ubuntu:22.04", cfgPath), map[string]string{"HTTPS_PROXY": ""}); k != base { + t.Error("an empty proxy variable changed the key") + } +} From e0a7a708fa949068f577a4211cd6d7926709f61a Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Sun, 25 Oct 2026 18:04:53 -0300 Subject: [PATCH 4/7] test(cli): hold the outdated, upgrade and features test commands A mutation run over outdated.go, features_test_runner.go, secrets.go and validate.go left edits no test refused. Add tests for what callers see: upgrade's flag checks, lockfile versions and write location against the httptest registry; features test's Feature discovery, global scenarios, exit codes and container cleanup through a docker on PATH; the id-label, remote-env, mount and terminal validators; build secrets. Two small source changes make the rest testable or remove dead code: the ` exec` command is built by testScriptCommand, leaving execTestInContainer to the parity lane that runs it (features.test-single-scenario-success), and upgrade drops a lockfile path fallback for an empty ConfigFilePath, which a loaded configuration never has. Co-Authored-By: Claude Opus 5.5 --- internal/cli/features_test_runner.go | 13 +- .../features_test_runner_repo_held_test.go | 219 ++++++++++++++++++ internal/cli/outdated.go | 7 +- internal/cli/outdated_repo_held_test.go | 216 +++++++++++++++++ internal/cli/secrets_repo_held_test.go | 39 ++++ internal/cli/validate_repo_held_test.go | 87 +++++++ 6 files changed, 575 insertions(+), 6 deletions(-) create mode 100644 internal/cli/features_test_runner_repo_held_test.go create mode 100644 internal/cli/outdated_repo_held_test.go create mode 100644 internal/cli/secrets_repo_held_test.go create mode 100644 internal/cli/validate_repo_held_test.go diff --git a/internal/cli/features_test_runner.go b/internal/cli/features_test_runner.go index 9e10521..933d092 100644 --- a/internal/cli/features_test_runner.go +++ b/internal/cli/features_test_runner.go @@ -483,8 +483,19 @@ func makeWorkspaceAccessible(dir string) { }) } +// execTestInContainer runs a test script in the test container with this +// binary's `exec`, and reports whether it passed. +// +// mutate-lane: TestParityMatrix (features.test-single-scenario-success passes +// only when the scenario script's success reaches the exit code through it). func execTestInContainer(containerId, testScript, workspaceFolder string) bool { self, _ := os.Executable() + return testScriptCommand(self, containerId, testScript, workspaceFolder).Run() == nil +} + +// testScriptCommand is the ` exec` that sources the test library and a +// test script in the container, with the script's output on this process's. +func testScriptCommand(self, containerId, testScript, workspaceFolder string) *exec.Cmd { // The workspace is mounted at /workspaces/; the test lib and scripts live // there, so cd into it (not its parent) before sourcing them. containerWs := "/workspaces/" + filepath.Base(workspaceFolder) @@ -492,7 +503,7 @@ func execTestInContainer(containerId, testScript, workspaceFolder string) bool { fmt.Sprintf("cd %s && source dev-container-features-test-lib && source %s && reportResults", containerWs, testScript)) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr - return cmd.Run() == nil + return cmd } func cleanupTestContainers(logger log.Logger) { diff --git a/internal/cli/features_test_runner_repo_held_test.go b/internal/cli/features_test_runner_repo_held_test.go new file mode 100644 index 0000000..c8e87a5 --- /dev/null +++ b/internal/cli/features_test_runner_repo_held_test.go @@ -0,0 +1,219 @@ +package cli + +import ( + "bytes" + "os" + "path/filepath" + "slices" + "strings" + "testing" + + "github.com/devcontainers/cli/internal/log" +) + +// fakeDockerOnPath puts a `docker` first on PATH whose `ps` prints ids and +// whose `rm -f ` appends the id to the returned file, the way the runner's +// cleanup calls the real one. +func fakeDockerOnPath(t *testing.T, ids string) (psArgs, removed string) { + t.Helper() + dir := t.TempDir() + psArgs, removed = filepath.Join(dir, "ps.args"), filepath.Join(dir, "removed") + script := "#!/bin/sh\ncase \"$1\" in\n" + + "ps) echo \"$@\" > '" + psArgs + "'; printf '" + ids + "' ;;\n" + + "rm) echo \"$3\" >> '" + removed + "' ;;\n" + + "esac\n" + writeFile(t, filepath.Join(dir, "docker"), script, 0o755) + t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) + return psArgs, removed +} + +func readOrEmpty(t *testing.T, path string) string { + t.Helper() + data, err := os.ReadFile(path) + if err != nil && !os.IsNotExist(err) { + t.Fatal(err) + } + return string(data) +} + +func bufferLogger(buf *bytes.Buffer) log.Logger { + return log.New(log.Options{Level: log.LevelInfo, Format: "text", Writer: buf}) +} + +// Cleanup removes every container the test run labelled, and says how many. +func TestCleanupTestContainersRemovesLabelledContainers(t *testing.T) { + psArgs, removed := fakeDockerOnPath(t, `c1\n`) + var logs bytes.Buffer + cleanupTestContainers(bufferLogger(&logs)) + if got := readOrEmpty(t, psArgs); got != "ps -a --filter label=devcontainer.is_test_run=true --format {{.ID}}\n" { + t.Errorf("docker ps args = %q", got) + } + if got := readOrEmpty(t, removed); got != "c1\n" { + t.Errorf("removed = %q, want c1", got) + } + if !strings.Contains(logs.String(), "Cleaning up 1 test containers...") { + t.Errorf("logs = %q", logs.String()) + } +} + +// With no labelled containers there is nothing to remove or announce. +func TestCleanupTestContainersWithNone(t *testing.T) { + _, removed := fakeDockerOnPath(t, ``) + var logs bytes.Buffer + cleanupTestContainers(bufferLogger(&logs)) + if got := readOrEmpty(t, removed); got != "" { + t.Errorf("removed = %q, want nothing", got) + } + if strings.Contains(logs.String(), "Cleaning up") { + t.Errorf("logs = %q, want no cleanup line", logs.String()) + } +} + +// The workspace a test script enters is opened to every user, files and +// folders alike, so a non-root remote user can read it. +func TestMakeWorkspaceAccessible(t *testing.T) { + dir := t.TempDir() + sub := filepath.Join(dir, "feature") + writeFile(t, filepath.Join(sub, "test.sh"), "true\n", 0o600) + if err := os.Chmod(sub, 0o700); err != nil { + t.Fatal(err) + } + makeWorkspaceAccessible(dir) + for _, p := range []string{dir, sub, filepath.Join(sub, "test.sh")} { + info, err := os.Stat(p) + if err != nil { + t.Fatal(err) + } + if info.Mode().Perm() != 0o777 { + t.Errorf("%s mode = %v, want 0777", p, info.Mode().Perm()) + } + } +} + +// A test script runs through ` exec` in the test container, from the +// mounted workspace, with its output on this process's stdout and stderr. +func TestTestScriptCommand(t *testing.T) { + cmd := testScriptCommand("/bin/devcontainer", "cid", "color/test.sh", "/tmp/devcontainer-test-1") + want := []string{"/bin/devcontainer", "exec", "--container-id", "cid", "--", "bash", "-c", + "cd /workspaces/devcontainer-test-1 && source dev-container-features-test-lib && source color/test.sh && reportResults"} + if !slices.Equal(cmd.Args, want) { + t.Errorf("args = %q, want %q", cmd.Args, want) + } + if cmd.Stdout != os.Stdout || cmd.Stderr != os.Stderr { + t.Error("the script's output does not reach this process's stdout and stderr") + } +} + +// testCollection writes a collection with Features a and b (each with a +// test.sh), a _global scenario g, and a stray file in test/. +func testCollection(t *testing.T) string { + t.Helper() + base := t.TempDir() + for _, f := range []string{"a", "b"} { + writeFile(t, filepath.Join(base, "src", f, "install.sh"), "#!/bin/sh\n", 0o755) + writeFile(t, filepath.Join(base, "test", f, "test.sh"), "true\n", 0o755) + } + writeFile(t, filepath.Join(base, "test", "README.md"), "notes\n", 0o644) + writeFile(t, filepath.Join(base, "test", "_global", "scenarios.json"), `{"g":{"image":"alpine"}}`, 0o644) + writeFile(t, filepath.Join(base, "test", "_global", "g.sh"), "true\n", 0o755) + return base +} + +func runTestCommand(t *testing.T, collection string, featuresList []string) (code int, stdout, logs string) { + t.Helper() + out := &captureOutput{} + var buf bytes.Buffer + code = runFeaturesTestCommand(out, bufferLogger(&buf), collection, featuresList, "", + false, false, false, false, "alpine", "", false, true, false, nil) + return code, out.out.String(), buf.String() +} + +// reportedNames lists the quoted names of the report's result lines, in order. +func reportedNames(stdout string) []string { + var names []string + for _, line := range strings.Split(stdout, "\n") { + if _, rest, ok := strings.Cut(line, "'"); ok && strings.Contains(line, ":") && !strings.Contains(line, "Folder") { + name, _, _ := strings.Cut(rest, "'") + names = append(names, name) + } + } + return names +} + +// Without --features every Feature folder under test/ is tested (not _global, +// not a stray file), then the global scenarios; containers are cleaned up. +func TestRunFeaturesTestCommandDiscoversFeatures(t *testing.T) { + _, removed := fakeDockerOnPath(t, `leftover\n`) + fakeTestContainers(t, nil, nil, func(string, string) bool { return true }) + code, stdout, _ := runTestCommand(t, testCollection(t), nil) + if code != 0 { + t.Errorf("exit = %d, want 0\n%s", code, stdout) + } + if got, want := reportedNames(stdout), []string{"a", "b", "g"}; !slices.Equal(got, want) { + t.Errorf("reported %q, want %q\n%s", got, want, stdout) + } + if got := readOrEmpty(t, removed); got != "leftover\n" { + t.Errorf("removed = %q, want the labelled container", got) + } +} + +// --features tests exactly the named Features, and skips the global scenarios. +func TestRunFeaturesTestCommandNamedFeatures(t *testing.T) { + fakeDockerOnPath(t, ``) + fakeTestContainers(t, nil, nil, func(string, string) bool { return true }) + code, stdout, _ := runTestCommand(t, testCollection(t), []string{"a"}) + if code != 0 { + t.Errorf("exit = %d, want 0\n%s", code, stdout) + } + if got, want := reportedNames(stdout), []string{"a"}; !slices.Equal(got, want) { + t.Errorf("reported %q, want %q\n%s", got, want, stdout) + } +} + +// A collection without both src/ and test/ fails with exit 1 and TS's message +// on stdout, whichever of the two is missing. +func TestRunFeaturesTestCommandRequiresSrcAndTest(t *testing.T) { + for _, present := range []string{"src", "test"} { + base := t.TempDir() + if err := os.MkdirAll(filepath.Join(base, present), 0o755); err != nil { + t.Fatal(err) + } + code, stdout, _ := runTestCommand(t, base, nil) + want := "[-] Folder '" + base + "' does not contain the required 'src' and 'test' folders.\n" + if code != 1 || !strings.HasSuffix(stdout, want) { + t.Errorf("only %s/: exit = %d, stdout = %q", present, code, stdout) + } + } +} + +// With no Feature folder to test, the command fails with exit 1. +func TestRunFeaturesTestCommandWithoutFeatures(t *testing.T) { + base := t.TempDir() + for _, d := range []string{"src", filepath.Join("test", "_global")} { + if err := os.MkdirAll(filepath.Join(base, d), 0o755); err != nil { + t.Fatal(err) + } + } + fakeTestContainers(t, nil, nil, func(string, string) bool { return true }) + code, stdout, logs := runTestCommand(t, base, nil) + if code != 1 || !strings.Contains(logs, "No features to test.") || strings.Contains(stdout, "TEST REPORT") { + t.Errorf("exit = %d, logs = %q, stdout = %q", code, logs, stdout) + } +} + +// A test/ folder that cannot be listed fails the command with exit 1. +func TestRunFeaturesTestCommandUnreadableTestFolder(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root lists any folder") + } + base := testCollection(t) + testDir := filepath.Join(base, "test") + if err := os.Chmod(testDir, 0o000); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(testDir, 0o755) }) + code, _, logs := runTestCommand(t, base, nil) + if code != 1 || !strings.Contains(logs, "Failed to discover feature tests") { + t.Errorf("exit = %d, logs = %q", code, logs) + } +} diff --git a/internal/cli/outdated.go b/internal/cli/outdated.go index 94ca4b9..903965c 100644 --- a/internal/cli/outdated.go +++ b/internal/cli/outdated.go @@ -6,7 +6,6 @@ import ( "encoding/json" "fmt" "os" - "path/filepath" "regexp" "slices" "sort" @@ -437,11 +436,9 @@ func newUpgradeCmd() *cobra.Command { cfg := loadResult.Config - // Read existing lockfile + // The lockfile sits next to the configuration, whose path a loaded + // configuration always carries. lockfilePath := cfg.ConfigFilePath - if lockfilePath == "" { - lockfilePath = filepath.Join(ws, "devcontainer.json") - } // Generate new lockfile from current features config // This is a simplified version — the full implementation diff --git a/internal/cli/outdated_repo_held_test.go b/internal/cli/outdated_repo_held_test.go new file mode 100644 index 0000000..66355de --- /dev/null +++ b/internal/cli/outdated_repo_held_test.go @@ -0,0 +1,216 @@ +package cli + +import ( + "bytes" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "io" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/devcontainers/cli/internal/features" + "github.com/spf13/cobra" +) + +func executeCmd(t *testing.T, cmd *cobra.Command, args ...string) (string, error) { + t.Helper() + // The root command silences usage and errors; the bare subcommand does not. + cmd.SilenceUsage, cmd.SilenceErrors = true, true + var out bytes.Buffer + cmd.SetOut(&out) + cmd.SetErr(io.Discard) + cmd.SetArgs(args) + err := cmd.ExecuteContext(t.Context()) + return out.String(), err +} + +// outdated reads the workspace given by --workspace-folder, and the current +// directory without it. +func TestOutdatedCmdWorkspaceFolderDefaultsToCwd(t *testing.T) { + ws := writeOutdatedWorkspace(t, `{}`, "") + t.Chdir(t.TempDir()) + if got, err := executeCmd(t, newOutdatedCmd(), "--workspace-folder", ws); err != nil || got != "No features configured.\n" { + t.Errorf("--workspace-folder: err = %v, stdout = %q", err, got) + } + t.Chdir(ws) + if got, err := executeCmd(t, newOutdatedCmd()); err != nil || got != "No features configured.\n" { + t.Errorf("cwd: err = %v, stdout = %q", err, got) + } +} + +// manifestWithAnnotations is a Feature manifest carrying the given annotations +// object (JSON), or none when it is empty. +func manifestWithAnnotations(annotations string) []byte { + m := `{"schemaVersion":2,"mediaType":"application/vnd.oci.image.manifest.v1+json",` + + `"config":{"mediaType":"application/vnd.devcontainers","digest":"sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a","size":2},` + + `"layers":[{"mediaType":"application/vnd.devcontainers.layer.v1+tar","digest":"sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a","size":2}]` + if annotations != "" { + m += `,"annotations":` + annotations + } + return []byte(m + `}`) +} + +// upgradeRegistry serves Features whose lockfile version comes from each of +// the places upgrade looks: the manifest's metadata, else the newest published +// version the tag wants, else the tag itself. +func upgradeRegistry(t *testing.T) (*readRegistry, string) { + t.Helper() + reg := newReadRegistry(t) + reg.manifests["ns/annotated"] = featureManifest("annotated") + reg.tags["ns/annotated"] = []string{"1.0.0", "1.5.0"} + reg.manifests["ns/plain"] = manifestWithAnnotations("") + reg.tags["ns/plain"] = []string{"1.0.0", "1.5.0", "2.0.0"} + reg.manifests["ns/noversion"] = manifestWithAnnotations(`{"dev.containers.metadata":"{\"id\":\"noversion\"}"}`) + reg.tags["ns/noversion"] = []string{"1.2.0"} + reg.manifests["ns/untagged"] = manifestWithAnnotations("") + h := reg.host + features := `{"` + h + `/ns/annotated:1":{}, "` + h + `/ns/plain:1":{}, "` + h + `/ns/noversion:1":{}, "` + + h + `/ns/untagged:1":{}, "` + h + `/ns/missing:1":{}, "./local":{}}` + return reg, features +} + +func parseLockfile(t *testing.T, data string) features.Lockfile { + t.Helper() + var lf features.Lockfile + if err := json.Unmarshal([]byte(data), &lf); err != nil { + t.Fatalf("not a lockfile: %v\n%s", err, data) + } + return lf +} + +// upgrade --dry-run prints the lockfile for every OCI Feature whose manifest +// resolves, pinned to its manifest digest, at the version TS records; local +// Features and ones without a manifest are left out. +func TestUpgradeDryRunPrintsLockfile(t *testing.T) { + reg, feats := upgradeRegistry(t) + ws := writeOutdatedWorkspace(t, feats, "") + got, err := executeCmd(t, newUpgradeCmd(), "--workspace-folder", ws, "--dry-run") + if err != nil { + t.Fatal(err) + } + lf := parseLockfile(t, got) + h := reg.host + want := map[string]string{ + h + "/ns/annotated:1": "1.0.0", + h + "/ns/plain:1": "1.5.0", + h + "/ns/noversion:1": "1.2.0", + h + "/ns/untagged:1": "1", + } + if len(lf.Features) != len(want) { + t.Errorf("lockfile features = %v, want %v", lf.Features, want) + } + for id, version := range want { + if e := lf.Features[id]; e.Version != version { + t.Errorf("%s version = %q, want %q", id, e.Version, version) + } + } + sum := sha256.Sum256(reg.manifests["ns/annotated"]) + digest := "sha256:" + hex.EncodeToString(sum[:]) + if e := lf.Features[h+"/ns/annotated:1"]; e.Integrity != digest || e.Resolved != h+"/ns/annotated@"+digest { + t.Errorf("annotated entry = %+v, want it pinned to %s", e, digest) + } + if _, err := os.Stat(filepath.Join(ws, ".devcontainer", "devcontainer-lock.json")); !os.IsNotExist(err) { + t.Errorf("--dry-run wrote a lockfile: %v", err) + } +} + +// Without --dry-run the lockfile is written next to the configuration +// --config names; one that cannot be written fails the command. +func TestUpgradeWritesLockfileNextToConfig(t *testing.T) { + reg := newReadRegistry(t) + reg.manifests["ns/tool"] = featureManifest("tool") + cfg := filepath.Join(t.TempDir(), "custom.json") + writeFile(t, cfg, `{"image":"alpine","features":{"`+reg.host+`/ns/tool:1":{}}}`, 0o644) + ws := t.TempDir() + if _, err := executeCmd(t, newUpgradeCmd(), "--workspace-folder", ws, "--config", cfg); err != nil { + t.Fatal(err) + } + data, err := os.ReadFile(features.LockfilePath(cfg)) + if err != nil { + t.Fatal(err) + } + if e := parseLockfile(t, string(data)).Features[reg.host+"/ns/tool:1"]; e.Version != "1.0.0" { + t.Errorf("written entry = %+v, want version 1.0.0", e) + } + + blocked := filepath.Join(t.TempDir(), "devcontainer.json") + writeFile(t, blocked, `{"image":"alpine","features":{"`+reg.host+`/ns/tool:1":{}}}`, 0o644) + if err := os.Mkdir(features.LockfilePath(blocked), 0o755); err != nil { + t.Fatal(err) + } + if _, err := executeCmd(t, newUpgradeCmd(), "--workspace-folder", ws, "--config", blocked); err == nil { + t.Error("a lockfile path that is a directory: want an error") + } +} + +// upgrade reads the workspace given by --workspace-folder, and the current +// directory without it. +func TestUpgradeWorkspaceFolderDefaultsToCwd(t *testing.T) { + ws := writeOutdatedWorkspace(t, `{}`, "") + t.Chdir(t.TempDir()) + if got, err := executeCmd(t, newUpgradeCmd(), "--workspace-folder", ws, "--dry-run"); err != nil || parseLockfile(t, got).Features == nil { + t.Errorf("--workspace-folder: err = %v, stdout = %q", err, got) + } + t.Chdir(ws) + if got, err := executeCmd(t, newUpgradeCmd(), "--dry-run"); err != nil || parseLockfile(t, got).Features == nil { + t.Errorf("cwd: err = %v, stdout = %q", err, got) + } +} + +// --feature and --target-version go together, and the target is a bare +// x / x.y / x.y.z version (TS upgradeCommand.ts .check()); a configuration that +// cannot be loaded, or a log file that cannot be opened, fails the command. +func TestUpgradeRejectsBadInvocations(t *testing.T) { + ws := writeOutdatedWorkspace(t, `{}`, "") + together := "The '--target-version' and '--feature' flag must be used together." + cases := []struct { + args []string + want string + }{ + {[]string{"--feature", "ghcr.io/o/f"}, together}, + {[]string{"--target-version", "2"}, together}, + {[]string{"--feature", "ghcr.io/o/f", "--target-version", "2.x"}, "Invalid version '2.x'. Must be in the form of 'x', 'x.y', or 'x.y.z'"}, + {[]string{"--log-file", t.TempDir()}, "open log file"}, + {[]string{"--workspace-folder", t.TempDir()}, "devcontainer.json"}, + } + for _, c := range cases { + args := append([]string{"--workspace-folder", ws, "--dry-run"}, c.args...) + if _, err := executeCmd(t, newUpgradeCmd(), args...); err == nil || !strings.Contains(err.Error(), c.want) { + t.Errorf("%q: err = %v, want %q", c.args, err, c.want) + } + } + if _, err := executeCmd(t, newUpgradeCmd(), "--workspace-folder", ws, "--dry-run", "--feature", "ghcr.io/o/f", "--target-version", "2.1"); err != nil { + t.Errorf("--feature with a bare --target-version: %v", err) + } +} + +// The keys of a "features" value that is not an object are not Feature ids. +func TestFeatureKeyOrderFromFileNonObjectFeatures(t *testing.T) { + cfg := filepath.Join(t.TempDir(), "devcontainer.json") + writeFile(t, cfg, `{"features":["a","b"]}`, 0o644) + if keys, err := featureKeyOrderFromFile(cfg); err != nil || keys != nil { + t.Errorf("keys = %q, err = %v, want none", keys, err) + } +} + +// A column is as wide as its widest cell, an empty column has no width, and a +// cell one shorter than its column is padded by one. +func TestTextTableWidths(t *testing.T) { + got := textTable([][]string{{"", "ab", "x"}, {"", "a", "y"}}) + if want := " ab x\n a y"; got != want { + t.Errorf("table = %q, want %q", got, want) + } +} + +// The tag or digest is cut at the first ':' or '@' after the last '/', even +// right after it (TS /[:@][^/]*$/). +func TestGetFeatureIdWithoutVersionRightAfterSlash(t *testing.T) { + for in, want := range map[string]string{"o/@sha256:x": "o/", "@sha256:x": "", "o/:1": "o/"} { + if got := getFeatureIdWithoutVersion(in); got != want { + t.Errorf("getFeatureIdWithoutVersion(%q) = %q, want %q", in, got, want) + } + } +} diff --git a/internal/cli/secrets_repo_held_test.go b/internal/cli/secrets_repo_held_test.go new file mode 100644 index 0000000..5c70955 --- /dev/null +++ b/internal/cli/secrets_repo_held_test.go @@ -0,0 +1,39 @@ +package cli + +import ( + "os" + "path/filepath" + "slices" + "testing" +) + +// --secrets-file feeds the build its secrets as KEY=VALUE; no file, an +// unreadable one or a malformed one gives the build none. +func TestBuildSecretsFromFile(t *testing.T) { + dir := t.TempDir() + good := filepath.Join(dir, "secrets.json") + if err := os.WriteFile(good, []byte(`{"NPM_TOKEN":"abc","GH_TOKEN":"x=y"}`), 0o600); err != nil { + t.Fatal(err) + } + got := buildSecretsFromFile(good) + slices.Sort(got) + if want := []string{"GH_TOKEN=x=y", "NPM_TOKEN=abc"}; !slices.Equal(got, want) { + t.Errorf("secrets = %q, want %q", got, want) + } + bad := filepath.Join(dir, "bad.json") + if err := os.WriteFile(bad, []byte(`not json`), 0o600); err != nil { + t.Fatal(err) + } + for _, p := range []string{"", filepath.Join(dir, "missing.json"), bad} { + if got := buildSecretsFromFile(p); got != nil { + t.Errorf("buildSecretsFromFile(%q) = %q, want nil", p, got) + } + } +} + +// A one-letter secret name is still a name: only an empty one is dropped. +func TestBuildSecretIDsOneLetterName(t *testing.T) { + if got, want := buildSecretIDs([]string{"A=1", "=2"}), []string{"A"}; !slices.Equal(got, want) { + t.Errorf("ids = %q, want %q", got, want) + } +} diff --git a/internal/cli/validate_repo_held_test.go b/internal/cli/validate_repo_held_test.go new file mode 100644 index 0000000..132cb18 --- /dev/null +++ b/internal/cli/validate_repo_held_test.go @@ -0,0 +1,87 @@ +package cli + +import "testing" + +// An --id-label is =: a name and a value on both sides of an '='. +// TS rejects a label without '=', with an empty name or with an empty value. +func TestValidateIDLabels(t *testing.T) { + for _, ok := range []string{"a=b", "devcontainer.local_folder=/x", "a=b=c"} { + if err := validateIDLabels([]string{ok}); err != nil { + t.Errorf("validateIDLabels(%q) = %v, want nil", ok, err) + } + } + for _, bad := range []string{"ab", "=b", "a=", "="} { + err := validateIDLabels([]string{"ok=1", bad}) + if err == nil || err.Error() != "Unmatched argument format: id-label must match =" { + t.Errorf("validateIDLabels(%q) = %v, want the format error", bad, err) + } + } + if err := validateIDLabels(nil); err != nil { + t.Errorf("no labels: %v", err) + } +} + +// A --remote-env needs a name before its '='; an empty value is allowed. +func TestValidateRemoteEnvs(t *testing.T) { + for _, ok := range []string{"A=b", "A=", "A=b=c"} { + if err := validateRemoteEnvs([]string{ok}); err != nil { + t.Errorf("validateRemoteEnvs(%q) = %v, want nil", ok, err) + } + } + for _, bad := range []string{"A", "=b"} { + err := validateRemoteEnvs([]string{"OK=1", bad}) + if err == nil || err.Error() != "Unmatched argument format: remote-env must match =" { + t.Errorf("validateRemoteEnvs(%q) = %v, want the format error", bad, err) + } + } +} + +func TestValidateMounts(t *testing.T) { + for _, ok := range []string{ + "type=bind,source=/a,target=/b", + "type=volume,source=v,target=/b,external=true", + } { + if err := validateMounts([]string{ok}); err != nil { + t.Errorf("validateMounts(%q) = %v, want nil", ok, err) + } + } + for _, bad := range []string{"type=tmpfs,source=a,target=/b", "source=/a,target=/b", "type=bind,source=/a,target=/b,external=yes"} { + if err := validateMounts([]string{bad}); err == nil { + t.Errorf("validateMounts(%q) = nil, want an error", bad) + } + } +} + +// --terminal-columns and --terminal-rows imply each other, as yargs .implies(). +func TestValidateTerminalImplications(t *testing.T) { + cases := []struct { + cols, rows int + want string + }{ + {0, 0, ""}, + {80, 24, ""}, + {80, 0, "Implications failed:\n terminal-columns -> terminal-rows"}, + {0, 24, "Implications failed:\n terminal-rows -> terminal-columns"}, + } + for _, c := range cases { + err := validateTerminalImplications(c.cols, c.rows) + got := "" + if err != nil { + got = err.Error() + } + if got != c.want { + t.Errorf("validateTerminalImplications(%d, %d) = %q, want %q", c.cols, c.rows, got, c.want) + } + } +} + +// The first flag whose value is not one of its choices is the one reported. +func TestValidateEnums(t *testing.T) { + if err := validateEnums(enumFlag{"log-level", "debug", logLevels}, enumFlag{"log-format", "json", logFormats}); err != nil { + t.Errorf("valid choices: %v", err) + } + err := validateEnums(enumFlag{"log-level", "loud", logLevels}, enumFlag{"log-format", "xml", logFormats}) + if want := `Invalid value "loud" for --log-level. Choose from: info, debug, trace`; err == nil || err.Error() != want { + t.Errorf("err = %v, want %s", err, want) + } +} From 954fd8a6882b55d1e55b782265a65b8bf598e9a0 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Sun, 25 Oct 2026 18:27:41 -0300 Subject: [PATCH 5/7] test(cli): hold every edit mutate makes in the command files A whole-repository mutate run left 66 edits in open, stop, check, exec, up, build and gpu that no test refused. The new tests check what a caller depends on, without Docker: the positional [path] of open, stop, down and build, open's editor launch and config, check's exit code, saved state and JSON, the up/build host hint on a real pseudo-terminal, the lifecycle marker scripts run by the host's sh, exec's Compose container lookup against the fake engine, and the GPU runtime probe. Two edits nobody can observe carry a mutate-exempt instead: the tabwriter's minwidth, tabwidth and flags in the check table (moved to a const so the padding stays mutated and tested), and the signal branch of execExitCode, where ExitCode -1 always comes with a signaled WaitStatus. Co-Authored-By: Claude Opus 5.5 --- internal/cli/build_repo_held_test.go | 96 +++++++++++++++ internal/cli/check.go | 4 +- internal/cli/check_held_test.go | 81 +++++++++++++ internal/cli/check_repo_held_test.go | 106 ++++++++++++++++ internal/cli/exec.go | 1 + internal/cli/exec_repo_held_test.go | 109 +++++++++++++++++ internal/cli/gpu_repo_held_test.go | 39 ++++++ internal/cli/open_repo_held_test.go | 174 +++++++++++++++++++++++++++ internal/cli/stop_repo_held_test.go | 71 +++++++++++ internal/cli/up_repo_held_test.go | 111 +++++++++++++++++ 10 files changed, 791 insertions(+), 1 deletion(-) create mode 100644 internal/cli/build_repo_held_test.go create mode 100644 internal/cli/check_repo_held_test.go create mode 100644 internal/cli/exec_repo_held_test.go create mode 100644 internal/cli/gpu_repo_held_test.go create mode 100644 internal/cli/open_repo_held_test.go create mode 100644 internal/cli/stop_repo_held_test.go create mode 100644 internal/cli/up_repo_held_test.go diff --git a/internal/cli/build_repo_held_test.go b/internal/cli/build_repo_held_test.go new file mode 100644 index 0000000..ce55dfa --- /dev/null +++ b/internal/cli/build_repo_held_test.go @@ -0,0 +1,96 @@ +package cli + +import ( + "bytes" + "encoding/json" + "os" + "path/filepath" + "testing" + + "github.com/devcontainers/cli/internal/config" +) + +// buildConfigNotFound runs build with args and returns the path its "config +// not found" error names. +func buildConfigNotFound(t *testing.T, args ...string) string { + t.Helper() + cmd := newBuildCmd() + cmd.SilenceUsage, cmd.SilenceErrors = true, true + var out bytes.Buffer + cmd.SetOut(&out) + cmd.SetErr(&bytes.Buffer{}) + cmd.SetArgs(args) + _ = cmd.ExecuteContext(t.Context()) + var got struct{ Outcome, Message string } + if err := json.Unmarshal(out.Bytes(), &got); err != nil { + t.Fatalf("stdout is not one JSON result: %v\n%s", err, out.String()) + } + if got.Outcome != "error" { + t.Fatalf("outcome = %q, want error", got.Outcome) + } + return got.Message +} + +// build [path] builds the workspace the argument names, not the current +// directory; --workspace-folder wins over the argument. +func TestBuildPositionalPathIsTheWorkspace(t *testing.T) { + ws := t.TempDir() + t.Chdir(t.TempDir()) + want := "Dev container config (" + filepath.Join(ws, ".devcontainer", "devcontainer.json") + ") not found." + if got := buildConfigNotFound(t, ws); got != want { + t.Errorf("message = %q, want %q", got, want) + } + if got := buildConfigNotFound(t, t.TempDir(), "--workspace-folder", ws); got != want { + t.Errorf("message with --workspace-folder = %q, want %q", got, want) + } +} + +// A build target names the stage to build, and the Dockerfile is used as it +// is, with no stage name added. +func TestPrepareDockerfileBuildUsesTheConfiguredTarget(t *testing.T) { + dir := t.TempDir() + dockerfile := filepath.Join(dir, "Dockerfile") + if err := os.WriteFile(dockerfile, []byte("FROM alpine AS base\nFROM base\n"), 0o644); err != nil { + t.Fatal(err) + } + cfg := &config.DevContainer{ + ConfigFilePath: filepath.Join(dir, "devcontainer.json"), + Build: &config.Build{Dockerfile: "Dockerfile", Target: "base"}, + } + prep, err := prepareDockerfileBuild(cfg) + if err != nil { + t.Fatal(err) + } + defer prep.Cleanup() + if prep.StageName != "base" || prep.Path != dockerfile { + t.Errorf("stage %q at %q, want base at %q", prep.StageName, prep.Path, dockerfile) + } +} + +// Without a target, the final stage gets a name to build, in a Dockerfile +// written next to the user's, whether the Dockerfile is named under build or +// at the top level. +func TestPrepareDockerfileBuildNamesTheFinalStageWithoutATarget(t *testing.T) { + for name, cfg := range map[string]config.DevContainer{ + "build": {Build: &config.Build{Dockerfile: "Dockerfile"}}, + "top level": {DockerFile: "Dockerfile"}, + } { + t.Run(name, func(t *testing.T) { + dir := t.TempDir() + dockerfile := filepath.Join(dir, "Dockerfile") + if err := os.WriteFile(dockerfile, []byte("FROM alpine\n"), 0o644); err != nil { + t.Fatal(err) + } + cfg.ConfigFilePath = filepath.Join(dir, "devcontainer.json") + prep, err := prepareDockerfileBuild(&cfg) + if err != nil { + t.Fatal(err) + } + defer prep.Cleanup() + const stage = "dev_container_auto_added_stage_label" + if prep.StageName != stage || prep.Path != dockerfile+".devcontainer.build" { + t.Errorf("stage %q at %q, want %s at the written Dockerfile", prep.StageName, prep.Path, stage) + } + }) + } +} diff --git a/internal/cli/check.go b/internal/cli/check.go index accfd7b..6f23b89 100644 --- a/internal/cli/check.go +++ b/internal/cli/check.go @@ -147,7 +147,9 @@ func writeSetupResult(out Output, opts *setupOpts, actions []doctor.Action, fina // writeReportTable renders the ✔/⚠/✖ table plus per-line remediation hints. func writeReportTable(w io.Writer, report doctor.Report) { - tw := tabwriter.NewWriter(w, 0, 0, 2, ' ', 0) + // mutate-exempt: a minwidth of 1 is below every cell's width plus the padding of 2, tabwidth counts only with '\t' padding, and flag 1 (FilterHTML) re-measures only HTML, which no status symbol or check name holds + const minwidth, tabwidth, flags = 0, 0, 0 + tw := tabwriter.NewWriter(w, minwidth, tabwidth, 2, ' ', flags) for _, r := range report.Results { fmt.Fprintf(tw, "%s\t%s\t%s\n", r.Status.Symbol(), r.Name, r.Summary) } diff --git a/internal/cli/check_held_test.go b/internal/cli/check_held_test.go index 00ecbfb..87a027c 100644 --- a/internal/cli/check_held_test.go +++ b/internal/cli/check_held_test.go @@ -4,6 +4,8 @@ import ( "encoding/json" "errors" "io" + "os" + "path/filepath" "strings" "testing" @@ -98,3 +100,82 @@ func TestWriteSetupResultText(t *testing.T) { t.Errorf("stderr = %q, want no note when the state was saved", saved.err.String()) } } + +// The table pads its columns two spaces past their widest cell. +func TestWriteReportTableAlignsTheColumns(t *testing.T) { + var buf strings.Builder + writeReportTable(&buf, sampleReport()) + want := "✔ docker-daemon Docker daemon reachable (server 27.0)\n" + + "⚠ build-cache-export cannot export cache\n" + + " → build-cache-export: run `devcontainer setup`\n" + + "\noverall: ⚠ warn\n" + if buf.String() != want { + t.Errorf("table =\n%s\nwant\n%s", buf.String(), want) + } +} + +// runCheckWithoutDocker runs check against a docker CLI that does not exist, +// so the daemon check fails, with the check state under stateHome. +func runCheckWithoutDocker(t *testing.T, out Output, json bool, stateHome string) error { + t.Helper() + t.Setenv("XDG_STATE_HOME", stateHome) + return runCheck(t.Context(), out, &checkOpts{json: json, dockerPath: filepath.Join(t.TempDir(), "docker")}) +} + +// A failing host prints the table, saves the state and exits 1. +func TestRunCheckFailingHostExitsOne(t *testing.T) { + state := t.TempDir() + out := &captureOutput{} + err := runCheckWithoutDocker(t, out, false, state) + if exitCode(err) != 1 { + t.Fatalf("err = %v, want exit code 1 for a failing host", err) + } + if s := out.out.String(); !strings.HasSuffix(s, "\noverall: ✖ fail\n") { + t.Errorf("stdout = %q, want the table of a failing host", s) + } + if out.err.Len() != 0 { + t.Errorf("stderr = %q, want nothing when the state was saved", out.err.String()) + } + saved, ok, err := doctor.LoadFrom(filepath.Join(state, "devcontainer", "check.json")) + if err != nil || !ok || saved.Overall != doctor.StatusFail { + t.Errorf("saved state = %+v, %v, %v; want the failing report", saved, ok, err) + } +} + +// A state that cannot be saved is a note on stderr, not a failure of its own. +func TestRunCheckNotesAStateItCouldNotSave(t *testing.T) { + blocker := filepath.Join(t.TempDir(), "file") + if err := os.WriteFile(blocker, nil, 0o644); err != nil { + t.Fatal(err) + } + out := &captureOutput{} + err := runCheckWithoutDocker(t, out, false, blocker) + if exitCode(err) != 1 { + t.Fatalf("err = %v, want exit code 1 for a failing host", err) + } + if s := out.err.String(); !strings.HasPrefix(s, "note: could not save check state: ") { + t.Errorf("stderr = %q, want the note", s) + } +} + +// --json prints the report as JSON and still exits 1 for a failing host; a +// stdout that cannot be written fails with that error. +func TestRunCheckJSON(t *testing.T) { + out := &captureOutput{} + err := runCheckWithoutDocker(t, out, true, t.TempDir()) + if exitCode(err) != 1 { + t.Fatalf("err = %v, want exit code 1 for a failing host", err) + } + var got doctor.Report + if err := json.Unmarshal(out.out.Bytes(), &got); err != nil { + t.Fatalf("stdout is not JSON: %v\n%s", err, out.out.String()) + } + if got.Overall != doctor.StatusFail || len(got.Results) == 0 { + t.Errorf("report = %+v, want a failing report", got) + } + + err = runCheckWithoutDocker(t, &brokenStdout{}, true, t.TempDir()) + if err == nil || !strings.Contains(err.Error(), "stdout closed") { + t.Fatalf("err = %v, want the write error", err) + } +} diff --git a/internal/cli/check_repo_held_test.go b/internal/cli/check_repo_held_test.go new file mode 100644 index 0000000..827cff1 --- /dev/null +++ b/internal/cli/check_repo_held_test.go @@ -0,0 +1,106 @@ +//go:build linux + +package cli + +import ( + "bufio" + "fmt" + "io" + "os" + "path/filepath" + "strings" + "syscall" + "testing" + "unsafe" + + "github.com/devcontainers/cli/internal/doctor" +) + +// openPTY opens a pseudo-terminal: what is written to the returned terminal +// can be read back from the returned master. +func openPTY(t *testing.T) (master, terminal *os.File) { + t.Helper() + master, err := os.OpenFile("/dev/ptmx", os.O_RDWR, 0) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { master.Close() }) + var unlock int32 + if _, _, errno := syscall.Syscall(syscall.SYS_IOCTL, master.Fd(), syscall.TIOCSPTLCK, uintptr(unsafe.Pointer(&unlock))); errno != 0 { + t.Fatalf("unlock pty: %v", errno) + } + var n uint32 + if _, _, errno := syscall.Syscall(syscall.SYS_IOCTL, master.Fd(), syscall.TIOCGPTN, uintptr(unsafe.Pointer(&n))); errno != 0 { + t.Fatalf("pty number: %v", errno) + } + terminal, err = os.OpenFile(fmt.Sprintf("/dev/pts/%d", n), os.O_RDWR|syscall.O_NOCTTY, 0) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { terminal.Close() }) + return master, terminal +} + +type terminalStderr struct{ terminal *os.File } + +func (terminalStderr) Stdout() io.Writer { return io.Discard } +func (o terminalStderr) Stderr() io.Writer { return o.terminal } + +// warnOnTerminal runs warnUncheckedHost with a terminal for stderr and the +// check state under stateHome, and returns what it wrote there. +func warnOnTerminal(t *testing.T, stateHome string) string { + t.Helper() + t.Setenv("XDG_STATE_HOME", stateHome) + master, terminal := openPTY(t) + warnUncheckedHost(terminalStderr{terminal}) + const end = "END-OF-OUTPUT" + fmt.Fprintln(terminal, end) + var got strings.Builder + r := bufio.NewReader(master) + for { + line, err := r.ReadString('\n') + if err != nil { + t.Fatalf("read terminal: %v (read %q)", err, got.String()) + } + line = strings.TrimRight(line, "\r\n") + if line == end { + return got.String() + } + got.WriteString(line + "\n") + } +} + +func saveCheckState(t *testing.T, overall doctor.Status) string { + t.Helper() + home := t.TempDir() + if err := doctor.SaveTo(filepath.Join(home, "devcontainer", "check.json"), doctor.Report{Overall: overall}); err != nil { + t.Fatal(err) + } + return home +} + +// On a terminal, up and build hint at `check` when the host was never checked, +// warn when the last check failed, and say nothing after a passing check or +// when the state cannot be read. +func TestWarnUncheckedHostOnATerminal(t *testing.T) { + unreadable := t.TempDir() + if err := os.MkdirAll(filepath.Join(unreadable, "devcontainer", "check.json"), 0o755); err != nil { + t.Fatal(err) + } + for _, tc := range []struct { + name string + state string + want string + }{ + {"never checked", t.TempDir(), "hint: host not yet checked — run `devcontainer check` to verify your Docker setup.\n"}, + {"failing host", saveCheckState(t, doctor.StatusFail), "warning: `devcontainer check` reported a failing host configuration — run `devcontainer check` for details.\n"}, + {"passing host", saveCheckState(t, doctor.StatusOK), ""}, + {"unreadable state", unreadable, ""}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := warnOnTerminal(t, tc.state); got != tc.want { + t.Errorf("stderr = %q, want %q", got, tc.want) + } + }) + } +} diff --git a/internal/cli/exec.go b/internal/cli/exec.go index 3595e5d..62918c9 100644 --- a/internal/cli/exec.go +++ b/internal/cli/exec.go @@ -366,6 +366,7 @@ func execExitCode(exitErr *exec.ExitError) int { if code != -1 { return code } + // mutate-exempt: ExitCode is -1 only for a child a signal ended, whose WaitStatus is always there and Signaled, so || takes the same branch if ws, ok := exitErr.Sys().(syscall.WaitStatus); ok && ws.Signaled() { return 128 + int(ws.Signal()) } diff --git a/internal/cli/exec_repo_held_test.go b/internal/cli/exec_repo_held_test.go new file mode 100644 index 0000000..111be88 --- /dev/null +++ b/internal/cli/exec_repo_held_test.go @@ -0,0 +1,109 @@ +package cli + +import ( + "os" + "path/filepath" + "testing" + + "github.com/devcontainers/cli/internal/docker" +) + +// composeWorkspace writes a workspace named proj whose devcontainer.json runs +// the app service of a Compose project. +func composeWorkspace(t *testing.T) string { + t.Helper() + ws := filepath.Join(t.TempDir(), "proj") + dir := filepath.Join(ws, ".devcontainer") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + files := map[string]string{ + "devcontainer.json": `{"dockerComposeFile":"docker-compose.yml","service":"app"}`, + "docker-compose.yml": "services:\n app:\n image: alpine\n", + } + for name, content := range files { + if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } + return ws +} + +func fakeEngineClient(t *testing.T) *docker.EngineClient { + t.Helper() + engine, err := docker.NewEngineClient(quietLogger()) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { engine.Close() }) + return engine +} + +// exec finds the workspace's container by its local_folder label first, and +// only then looks for the service of its Compose project. +func TestFindContainerByOptsPrefersTheLabeledContainer(t *testing.T) { + ws := composeWorkspace(t) + startFakeEngine(t, map[string]map[string]string{ + "labeled": {"devcontainer.local_folder": ws}, + }) + opts := &execOpts{workspaceFolder: ws} + if got := findContainerByOpts(t.Context(), fakeEngineClient(t), opts, "", quietLogger()); got != "labeled" { + t.Errorf("container = %q, want labeled", got) + } +} + +// A Compose workspace's container without devcontainer labels is found by the +// project (the workspace folder's name, or COMPOSE_PROJECT_NAME) and service. +func TestFindContainerByOptsFindsTheComposeService(t *testing.T) { + ws := composeWorkspace(t) + startFakeEngine(t, map[string]map[string]string{ + "app": {"com.docker.compose.project": "proj_devcontainer", "com.docker.compose.service": "app"}, + "custom": {"com.docker.compose.project": "custom", "com.docker.compose.service": "app"}, + "db": {"com.docker.compose.project": "proj_devcontainer", "com.docker.compose.service": "db"}, + }) + engine := fakeEngineClient(t) + opts := &execOpts{workspaceFolder: ws} + if got := findContainerByOpts(t.Context(), engine, opts, "", quietLogger()); got != "app" { + t.Errorf("container = %q, want app", got) + } + t.Setenv("COMPOSE_PROJECT_NAME", "Custom") + if got := findContainerByOpts(t.Context(), engine, opts, "", quietLogger()); got != "custom" { + t.Errorf("container with COMPOSE_PROJECT_NAME = %q, want custom", got) + } +} + +// No Compose container is found without a workspace, without a config, for a +// config that is not Compose, or when the project has no such service. +func TestFindComposeContainerFindsNothing(t *testing.T) { + startFakeEngine(t, map[string]map[string]string{ + "app": {"com.docker.compose.project": "proj_devcontainer", "com.docker.compose.service": "app"}, + "img": {"com.docker.compose.project": "img_devcontainer", "com.docker.compose.service": ""}, + }) + engine := fakeEngineClient(t) + + compose := composeWorkspace(t) + t.Chdir(compose) + if got := findComposeContainer(t.Context(), engine, &execOpts{}, quietLogger()); got != "" { + t.Errorf("container without a workspace = %q, want none", got) + } + + if got := findComposeContainer(t.Context(), engine, &execOpts{workspaceFolder: t.TempDir()}, quietLogger()); got != "" { + t.Errorf("container without a config = %q, want none", got) + } + + img := filepath.Join(t.TempDir(), "img") + if err := os.MkdirAll(filepath.Join(img, ".devcontainer"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(img, ".devcontainer", "devcontainer.json"), []byte(`{"image":"alpine"}`), 0o644); err != nil { + t.Fatal(err) + } + if got := findComposeContainer(t.Context(), engine, &execOpts{workspaceFolder: img}, quietLogger()); got != "" { + t.Errorf("container of an image config = %q, want none", got) + } + + t.Setenv("COMPOSE_PROJECT_NAME", "empty") + if got := findComposeContainer(t.Context(), engine, &execOpts{workspaceFolder: compose}, quietLogger()); got != "" { + t.Errorf("container of a project without the service = %q, want none", got) + } +} diff --git a/internal/cli/gpu_repo_held_test.go b/internal/cli/gpu_repo_held_test.go new file mode 100644 index 0000000..f41731e --- /dev/null +++ b/internal/cli/gpu_repo_held_test.go @@ -0,0 +1,39 @@ +package cli + +import ( + "context" + "errors" + "slices" + "testing" +) + +// In detect mode the GPU is enabled only when `docker info` ran and reported +// the nvidia runtime; a failed docker info or one without the runtime ("") leaves it off. +func TestCheckGPUAvailabilityDetectsTheNvidiaRuntime(t *testing.T) { + orig := nvidiaContainerCLIInfo + t.Cleanup(func() { nvidiaContainerCLIInfo = orig }) + nvidiaContainerCLIInfo = func(context.Context) (bool, bool) { return false, false } + + for _, tc := range []struct { + name string + stdout string + err error + want bool + }{ + {"runtime reported", "nvidia-container-runtime\n", nil, true}, + {"no runtime", "\n", nil, false}, + {"docker info failed", "", errors.New("no docker"), false}, + } { + t.Run(tc.name, func(t *testing.T) { + r := &fakeDockerRunner{reply: func([]string) (string, int, error) { return tc.stdout, 0, tc.err }} + if got := checkGPUAvailability(t.Context(), "detect", fakeDockerClient(r)); got != tc.want { + t.Errorf("checkGPUAvailability = %v, want %v", got, tc.want) + } + want := dockerCall{"info", "-f", "{{.Runtimes.nvidia}}"} + if len(r.calls) != 1 || !slices.Equal(r.calls[0], want) { + t.Errorf("docker calls = %v, want [%v]", r.calls, want) + } + }) + } +} diff --git a/internal/cli/open_repo_held_test.go b/internal/cli/open_repo_held_test.go new file mode 100644 index 0000000..bc9ca20 --- /dev/null +++ b/internal/cli/open_repo_held_test.go @@ -0,0 +1,174 @@ +package cli + +import ( + "bytes" + "os" + "path/filepath" + "strconv" + "strings" + "testing" +) + +// openWorkspace writes a workspace whose devcontainer.json names an image, and +// returns its folder and config path. +func openWorkspace(t *testing.T) (string, string) { + t.Helper() + ws := t.TempDir() + dir := filepath.Join(ws, ".devcontainer") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + cfg := filepath.Join(dir, "devcontainer.json") + if err := os.WriteFile(cfg, []byte(`{"image":"alpine"}`), 0o644); err != nil { + t.Fatal(err) + } + return ws, cfg +} + +// wantOpenURI is the folder URI open builds for ws and its config. +func wantOpenURI(t *testing.T, ws, cfg string) string { + t.Helper() + uri, err := devcontainerFolderURI(ws, cfg, "/workspaces/"+filepath.Base(ws)) + if err != nil { + t.Fatal(err) + } + return uri +} + +func runOpenCmd(t *testing.T, args ...string) (string, string, error) { + t.Helper() + cmd := newOpenCmd() + cmd.SilenceUsage, cmd.SilenceErrors = true, true + var out, errOut bytes.Buffer + cmd.SetOut(&out) + cmd.SetErr(&errOut) + cmd.SetArgs(args) + err := cmd.ExecuteContext(t.Context()) + return out.String(), errOut.String(), err +} + +// open [path] --dry-run prints the folder URI of the workspace the argument +// names, and on stderr the editor command that would open it. +func TestOpenDryRunOpensThePositionalWorkspace(t *testing.T) { + ws, cfg := openWorkspace(t) + t.Chdir(t.TempDir()) + stdout, stderr, err := runOpenCmd(t, ws, "--dry-run") + if err != nil { + t.Fatal(err) + } + uri := wantOpenURI(t, ws, cfg) + if stdout != uri+"\n" { + t.Errorf("stdout = %q, want %q", stdout, uri+"\n") + } + if want := "code --folder-uri " + uri + "\n"; stderr != want { + t.Errorf("stderr = %q, want %q", stderr, want) + } +} + +// --workspace-folder wins over the positional path. +func TestOpenWorkspaceFlagWinsOverThePositionalPath(t *testing.T) { + ws, cfg := openWorkspace(t) + stdout, _, err := runOpenCmd(t, t.TempDir(), "--workspace-folder", ws, "--dry-run") + if err != nil { + t.Fatal(err) + } + if uri := wantOpenURI(t, ws, cfg); stdout != uri+"\n" { + t.Errorf("stdout = %q, want %q", stdout, uri+"\n") + } +} + +// Without a path open opens the current directory, through the editor +// --editor names. +func TestOpenDefaultsToTheCurrentDirectory(t *testing.T) { + ws, cfg := openWorkspace(t) + t.Chdir(ws) + stdout, stderr, err := runOpenCmd(t, "--dry-run", "--editor", "cursor") + if err != nil { + t.Fatal(err) + } + uri := wantOpenURI(t, ws, cfg) + if stdout != uri+"\n" { + t.Errorf("stdout = %q, want %q", stdout, uri+"\n") + } + if want := "cursor --folder-uri " + uri + "\n"; stderr != want { + t.Errorf("stderr = %q, want %q", stderr, want) + } +} + +// An empty editor falls back to code. +func TestOpenEmptyEditorIsCode(t *testing.T) { + ws, cfg := openWorkspace(t) + var out captureOutput + if err := runOpen(t.Context(), &out, openOpts{workspaceFolder: ws, dryRun: true}); err != nil { + t.Fatal(err) + } + if want := "code --folder-uri " + wantOpenURI(t, ws, cfg) + "\n"; out.err.String() != want { + t.Errorf("stderr = %q, want %q", out.err.String(), want) + } +} + +// --config names the devcontainer.json the URI carries. +func TestOpenConfigFlagNamesTheConfigFile(t *testing.T) { + ws, _ := openWorkspace(t) + other := filepath.Join(ws, "other.json") + if err := os.WriteFile(other, []byte(`{"image":"debian"}`), 0o644); err != nil { + t.Fatal(err) + } + stdout, _, err := runOpenCmd(t, ws, "--config", other, "--dry-run") + if err != nil { + t.Fatal(err) + } + if uri := wantOpenURI(t, ws, other); stdout != uri+"\n" { + t.Errorf("stdout = %q, want %q", stdout, uri+"\n") + } +} + +// A workspace without a devcontainer.json fails before any editor runs. +func TestOpenWithoutAConfigFails(t *testing.T) { + stdout, _, err := runOpenCmd(t, t.TempDir(), "--dry-run") + if err == nil { + t.Fatal("open succeeded without a devcontainer.json") + } + if stdout != "" { + t.Errorf("stdout = %q, want nothing", stdout) + } +} + +// writeEditor writes an editor launcher that prints its arguments on stdout, a +// line on stderr, and exits with code. +func writeEditor(t *testing.T, code int) string { + t.Helper() + path := filepath.Join(t.TempDir(), "editor") + script := "#!/bin/sh\necho \"$*\"\necho launched >&2\nexit " + strconv.Itoa(code) + "\n" + if err := os.WriteFile(path, []byte(script), 0o755); err != nil { + t.Fatal(err) + } + return path +} + +// Without --dry-run open runs the editor with the folder URI, passing its +// output through. +func TestOpenRunsTheEditor(t *testing.T) { + ws, cfg := openWorkspace(t) + editor := writeEditor(t, 0) + stdout, stderr, err := runOpenCmd(t, ws, "--editor", editor) + if err != nil { + t.Fatal(err) + } + if want := "--folder-uri " + wantOpenURI(t, ws, cfg) + "\n"; stdout != want { + t.Errorf("stdout = %q, want the editor's %q", stdout, want) + } + if want := "Opening " + ws + " in VS Code (dev container)...\nlaunched\n"; stderr != want { + t.Errorf("stderr = %q, want %q", stderr, want) + } +} + +// An editor that fails fails open. +func TestOpenFailsWhenTheEditorFails(t *testing.T) { + ws, _ := openWorkspace(t) + editor := writeEditor(t, 3) + _, _, err := runOpenCmd(t, ws, "--editor", editor) + if err == nil || !strings.Contains(err.Error(), "launch \""+editor+"\"") { + t.Fatalf("err = %v, want the launch failure", err) + } +} diff --git a/internal/cli/stop_repo_held_test.go b/internal/cli/stop_repo_held_test.go new file mode 100644 index 0000000..954ddd8 --- /dev/null +++ b/internal/cli/stop_repo_held_test.go @@ -0,0 +1,71 @@ +package cli + +import ( + "bytes" + "encoding/json" + "path/filepath" + "slices" + "testing" + + "github.com/spf13/cobra" +) + +func runStopCommand(t *testing.T, cmd *cobra.Command, args ...string) map[string]any { + t.Helper() + cmd.SilenceUsage, cmd.SilenceErrors = true, true + var out bytes.Buffer + cmd.SetOut(&out) + cmd.SetErr(&bytes.Buffer{}) + cmd.SetArgs(args) + _ = cmd.ExecuteContext(t.Context()) + var got map[string]any + if err := json.Unmarshal(out.Bytes(), &got); err != nil { + t.Fatalf("stdout is not one JSON result: %v\n%s", err, out.String()) + } + return got +} + +// stop [path] and down [path] act on the container of the workspace the +// argument names, not the current directory's; --workspace-folder wins over +// the argument. +func TestStopAndDownActOnThePositionalWorkspace(t *testing.T) { + for _, tc := range []struct { + name string + cmd func() *cobra.Command + action string + }{{"stop", newStopCmd, "stopped"}, {"down", newDownCmd, "removed"}} { + t.Run(tc.name, func(t *testing.T) { + ws, cwd := t.TempDir(), t.TempDir() + t.Chdir(cwd) + startFakeEngine(t, map[string]map[string]string{ + "ws": {"devcontainer.local_folder": ws}, + "cwd": {"devcontainer.local_folder": cwd}, + }) + cli, _ := fakeDockerCLI(t, false, 0) + + got := runStopCommand(t, tc.cmd(), ws, "--docker-path", cli) + assertResult(t, got, map[string]any{"outcome": "success", "containerId": "ws", "result": tc.action}) + + got = runStopCommand(t, tc.cmd(), t.TempDir(), "--workspace-folder", ws, "--docker-path", cli) + assertResult(t, got, map[string]any{"outcome": "success", "containerId": "ws", "result": tc.action}) + }) + } +} + +// --config looks first for the workspace's container of that config file. +func TestStopConfigFlagLooksForThatConfigsContainer(t *testing.T) { + ws := t.TempDir() + cfg := filepath.Join(ws, ".devcontainer", "devcontainer.json") + engine := startFakeEngine(t, map[string]map[string]string{ + "c1": {"devcontainer.local_folder": ws, "devcontainer.config_file": cfg}, + }) + cli, _ := fakeDockerCLI(t, false, 0) + + got := runStopOrDownForTest(t, stopOpts{workspaceFolder: ws, configPath: cfg, dockerPath: cli}, false) + + assertResult(t, got, map[string]any{"outcome": "success", "containerId": "c1", "result": "stopped"}) + want := "list devcontainer.config_file=" + cfg + ",devcontainer.local_folder=" + ws + if calls := engine.recorded(); !slices.Contains(calls, want) { + t.Errorf("engine calls = %v, want %q", calls, want) + } +} diff --git a/internal/cli/up_repo_held_test.go b/internal/cli/up_repo_held_test.go new file mode 100644 index 0000000..4e572a8 --- /dev/null +++ b/internal/cli/up_repo_held_test.go @@ -0,0 +1,111 @@ +package cli + +import ( + "bytes" + "context" + "errors" + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/devcontainers/cli/internal/lifecycle" +) + +// hostShell is a lifecycle.ContainerExecutor that runs each command with the +// host's sh, HOME set to home, so the marker scripts up runs in a container run +// for real. err, when set, is returned instead of running anything. +type hostShell struct { + home string + err error + commands []string +} + +func (h *hostShell) ExecInContainer(ctx context.Context, _, _ string, _ []string, command string) (string, string, int, error) { + h.commands = append(h.commands, command) + if h.err != nil { + return "", "", 0, h.err + } + cmd := exec.CommandContext(ctx, "sh", "-c", command) + cmd.Env = append(os.Environ(), "HOME="+h.home) + var stdout, stderr bytes.Buffer + cmd.Stdout, cmd.Stderr = &stdout, &stderr + err := cmd.Run() + if exitErr, ok := errors.AsType[*exec.ExitError](err); ok { + return stdout.String(), stderr.String(), exitErr.ExitCode(), nil + } + return stdout.String(), stderr.String(), 0, err +} + +func hostShellServer(t *testing.T, h *hostShell) *lifecycle.ShellServer { + t.Helper() + return lifecycle.NewShellServer(t.Context(), h, "c1", "") +} + +// The markers live in the container user's $HOME/.devcontainer; a HOME that +// cannot be read, or is empty, leaves no folder. +func TestResolveContainerUserDataFolder(t *testing.T) { + home := t.TempDir() + if got := resolveContainerUserDataFolder(hostShellServer(t, &hostShell{home: home})); got != home+"/.devcontainer" { + t.Errorf("folder = %q, want %q", got, home+"/.devcontainer") + } + if got := resolveContainerUserDataFolder(hostShellServer(t, &hostShell{})); got != "" { + t.Errorf("folder = %q for an empty HOME, want none", got) + } + if got := resolveContainerUserDataFolder(hostShellServer(t, &hostShell{err: errors.New("exec failed")})); got != "" { + t.Errorf("folder = %q when the exec failed, want none", got) + } +} + +// failingShell answers every command with exit code 1 and HOME's value on +// stdout, as a shell whose printf failed after writing would. +type failingShell struct{} + +func (failingShell) ExecInContainer(context.Context, string, string, []string, string) (string, string, int, error) { + return "/home/me", "", 1, nil +} + +func TestResolveContainerUserDataFolderOfAFailedProbe(t *testing.T) { + if got := resolveContainerUserDataFolder(lifecycle.NewShellServer(t.Context(), failingShell{}, "c1", "")); got != "" { + t.Errorf("folder = %q when the probe exited 1, want none", got) + } +} + +// A hook runs once per timestamp: the first ask writes the marker and runs it, +// the same timestamp again does not, and a new one (a new container or start) +// runs it again. An unknown timestamp runs it without touching the marker. +func TestMarkerShouldRunOncePerTimestamp(t *testing.T) { + folder := filepath.Join(t.TempDir(), ".devcontainer") + h := &hostShell{home: t.TempDir()} + server := hostShellServer(t, h) + + if !markerShouldRun(server, folder, "postCreateCommand", "2026-01-01T00:00:00Z") { + t.Error("first run: want the hook to run") + } + marker, err := os.ReadFile(filepath.Join(folder, ".postCreateCommandMarker")) + if err != nil || string(marker) != "2026-01-01T00:00:00Z\n" { + t.Errorf("marker = %q, %v; want the timestamp", marker, err) + } + if markerShouldRun(server, folder, "postCreateCommand", "2026-01-01T00:00:00Z") { + t.Error("same timestamp: want the hook skipped") + } + if !markerShouldRun(server, folder, "postCreateCommand", "2026-02-02T00:00:00Z") { + t.Error("new timestamp: want the hook to run") + } + + ran := len(h.commands) + if !markerShouldRun(server, folder, "postStartCommand", "") { + t.Error("unknown timestamp: want the hook to run") + } + if len(h.commands) != ran { + t.Errorf("unknown timestamp ran %q in the container", h.commands[ran:]) + } +} + +// A marker that cannot be checked does not run the hook. +func TestMarkerShouldRunWhenTheExecFails(t *testing.T) { + server := hostShellServer(t, &hostShell{err: errors.New("exec failed")}) + if markerShouldRun(server, "/nowhere", "postCreateCommand", "ts") { + t.Error("want the hook skipped when the marker exec failed") + } +} From c1918b67510562424fbcde025e6ca4fc0010e32f Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Sun, 25 Oct 2026 17:59:16 -0300 Subject: [PATCH 6/7] refactor(devtool): move the commands into internal/devtools cmd/devtool had no test, so go tool mutate reported every edit in it as untested. Its commands now live in devtools.Main, which takes the arguments and standard streams and returns the exit code; main only hands it the process's. Tests drive each command through Main with fake sudo, docker, go, sh and node first on PATH and throwaway git repositories, so nothing touches the host, and assert the output, exit codes, calls and file modes the Taskfile and workflows depend on. ci-prepare-runner's decision is split into prepareRunner(free) so both disk branches are tested. Command lines, output and exit codes are unchanged: an old and a new build gave identical results for every command run locally. readLines drops its 4 MiB scanner buffer: git prints paths, which PATH_MAX keeps far below the default 64 KiB token limit. Co-Authored-By: Claude Opus 5.5 --- cmd/devtool/main.go | 530 +------------------------- cmd/devtool/main_test.go | 47 +++ internal/devtools/command.go | 549 +++++++++++++++++++++++++++ internal/devtools/command_test.go | 608 ++++++++++++++++++++++++++++++ 4 files changed, 1209 insertions(+), 525 deletions(-) create mode 100644 cmd/devtool/main_test.go create mode 100644 internal/devtools/command.go create mode 100644 internal/devtools/command_test.go diff --git a/cmd/devtool/main.go b/cmd/devtool/main.go index 0af67ba..a81ef62 100644 --- a/cmd/devtool/main.go +++ b/cmd/devtool/main.go @@ -1,536 +1,16 @@ // Command devtool is the repo's CI/dev helper, replacing the former scripts/*.sh. -// Each subcommand wires stdin/stdout, git, `go tool`, and docker around the pure, -// unit-tested logic in internal/devtools. Invoked from the Taskfile and the CI -// workflow (e.g. `go run ./cmd/devtool coverage-gate coverage.out TOTAL=48`). +// Its commands live in internal/devtools (devtools.Main), which wires stdin/stdout, +// git, `go tool`, and docker around the pure, unit-tested logic there. Invoked +// from the Taskfile and the CI workflow (e.g. `go run ./cmd/devtool coverage-gate +// coverage.out TOTAL=48`). package main import ( - "bufio" - "encoding/json" - "fmt" - "io" "os" - "os/exec" - "path/filepath" - "strconv" - "strings" - "time" "github.com/devcontainers/cli/internal/devtools" ) func main() { - if len(os.Args) < 2 { - fmt.Fprintln(os.Stderr, "usage: devtool [args...]") - os.Exit(2) - } - cmd, args := os.Args[1], os.Args[2:] - var err error - switch cmd { - case "parity-affected": - err = parityAffected() - case "parity-affected-commands": - err = parityAffectedCommands(args) - case "daily-changed": - err = dailyChanged(args) - case "coverage-report": - err = coverageReport(args) - case "coverage-gate": - err = coverageGate(args) - case "coverage-merge": - err = coverageMerge(args) - case "ci-prepare-runner": - err = ciPrepareRunner() - case "docker-cache-export": - err = dockerCacheExport() - case "release-resolve": - err = releaseResolve() - case "metrics": - err = metrics(args) - default: - fmt.Fprintf(os.Stderr, "devtool: unknown command %q\n", cmd) - os.Exit(2) - } - if err != nil { - // exitError carries a specific code (gate failures, probe failures); other - // errors are usage/infra problems. - if ee, ok := err.(exitError); ok { - if ee.msg != "" { - fmt.Fprintln(os.Stderr, ee.msg) - } - os.Exit(ee.code) - } - fmt.Fprintf(os.Stderr, "devtool %s: %v\n", cmd, err) - os.Exit(1) - } -} - -type exitError struct { - code int - msg string -} - -func (e exitError) Error() string { return e.msg } - -// --- parity selection ------------------------------------------------------ - -func parityAffected() error { - files, err := readLines(os.Stdin) - if err != nil { - return err - } - fmt.Println(devtools.ParityAffected(files)) - return nil -} - -func parityAffectedCommands(args []string) error { - base := "" - if len(args) > 0 { - base = args[0] - } - head := "HEAD" - if len(args) > 1 { - head = args[1] - } - - if base == "" || !gitCommitExists(base) { - fmt.Fprintln(os.Stderr, "unknown base → running the full matrix") - fmt.Println("commands=all") - fmt.Println("run=true") - return nil - } - - out, err := runOut("git", "diff", "--name-only", base, head) - if err != nil { - return fmt.Errorf("git diff: %w", err) - } - fmt.Fprintln(os.Stderr, "changed files:") - fmt.Fprintln(os.Stderr, out) - - cmds := devtools.ParityAffected(strings.Split(out, "\n")) - fmt.Fprintf(os.Stderr, "affected parity commands: %s\n", cmds) - - fmt.Printf("commands=%s\n", cmds) - if cmds == "none" { - fmt.Println("run=false") - } else { - fmt.Println("run=true") - } - return nil -} - -func dailyChanged(args []string) error { - window := 25 - if len(args) > 0 { - w, err := strconv.Atoi(args[0]) - if err != nil { - return fmt.Errorf("window-hours %q: %w", args[0], err) - } - window = w - } - - now, err := nowEpoch() - if err != nil { - return err - } - lastStr, err := runOut("git", "log", "-1", "--format=%ct") - if err != nil { - return fmt.Errorf("git log: %w", err) - } - last, err := strconv.Atoi(strings.TrimSpace(lastStr)) - if err != nil { - return fmt.Errorf("commit timestamp %q: %w", lastStr, err) - } - - ageHours := (now - last) / 3600 - if devtools.DailyChanged(last, now, window) { - fmt.Fprintf(os.Stderr, "HEAD commit is %dh old (window %dh)\n", ageHours, window) - fmt.Println("run=true") - } else { - fmt.Fprintf(os.Stderr, "HEAD commit is %dh old (window %dh)\n", ageHours, window) - fmt.Println("run=false") - fmt.Fprintf(os.Stderr, "no commits within the last %dh — skipping the full runtime matrix\n", window) - } - return nil -} - -// nowEpoch honors DAILY_NOW_EPOCH (tests / reproducibility), else wall clock. -func nowEpoch() (int, error) { - if v := os.Getenv("DAILY_NOW_EPOCH"); v != "" { - n, err := strconv.Atoi(v) - if err != nil { - return 0, fmt.Errorf("DAILY_NOW_EPOCH %q: %w", v, err) - } - return n, nil - } - return int(time.Now().Unix()), nil -} - -// --- coverage -------------------------------------------------------------- - -func coverageReport(args []string) error { - if len(args) < 1 { - return fmt.Errorf("usage: coverage-report [title]") - } - src := args[0] - title := "coverage" - if len(args) > 1 { - title = args[1] - } - report, err := renderCoverage(src, title) - if err != nil { - return err - } - fmt.Print(report) - return nil -} - -// renderCoverage renders a profile file or a covdata directory (converted with -// `go tool covdata textfmt`) to a Markdown table. -func renderCoverage(src, title string) (string, error) { - profile := src - if info, err := os.Stat(src); err == nil && info.IsDir() { - if empty, _ := dirEmpty(src); empty { - return fmt.Sprintf("### Coverage — %s\n\n_(no coverage data)_\n", title), nil - } - profile = strings.TrimRight(src, "/") + ".out" - if err := run("go", "tool", "covdata", "textfmt", "-i="+src, "-o="+profile); err != nil { - return "", fmt.Errorf("covdata textfmt: %w", err) - } - } - f, err := os.Open(profile) - if err != nil { - return fmt.Sprintf("### Coverage — %s\n\n_(no coverage data)_\n", title), nil - } - defer f.Close() - pkgs, total, err := devtools.ParseProfile(f) - if err != nil { - return "", err - } - return devtools.RenderReport(pkgs, total, title), nil -} - -func coverageGate(args []string) error { - if len(args) < 2 { - return fmt.Errorf("usage: coverage-gate TOTAL= [pkg=min ...]") - } - f, err := os.Open(args[0]) - if err != nil { - return fmt.Errorf("open profile: %w", err) - } - defer f.Close() - pkgs, total, err := devtools.ParseProfile(f) - if err != nil { - return err - } - floors, err := devtools.ParseFloors(args[1:]) - if err != nil { - return err - } - results, ok := devtools.CoverageGate(pkgs, total, floors) - for _, r := range results { - fmt.Println(r.Line) - } - if !ok { - return exitError{code: 1} - } - return nil -} - -func coverageMerge(args []string) error { - if len(args) < 1 { - return fmt.Errorf("usage: coverage-merge [out-dir] [title]") - } - root := args[0] - out := strings.TrimRight(root, "/") + "/merged" - if len(args) > 1 { - out = args[1] - } - title := "merged (all lanes)" - if len(args) > 2 { - title = args[2] - } - - inputs, err := laneDirs(root, out) - if err != nil { - return err - } - if len(inputs) == 0 { - fmt.Printf("### Coverage — %s\n\n_(no lane data to merge)_\n", title) - return nil - } - fmt.Fprintf(os.Stderr, "merging coverage lanes: %s\n", strings.Join(inputs, ",")) - - if err := os.RemoveAll(out); err != nil { - return err - } - if err := os.MkdirAll(out, 0o755); err != nil { - return err - } - if err := run("go", "tool", "covdata", "merge", "-i="+strings.Join(inputs, ","), "-o="+out); err != nil { - return fmt.Errorf("covdata merge: %w", err) - } - report, err := renderCoverage(out, title) - if err != nil { - return err - } - fmt.Print(report) - return nil -} - -// laneDirs returns the non-empty per-lane covdata subdirs under root (skipping the -// merge output dir itself). -func laneDirs(root, out string) ([]string, error) { - entries, err := os.ReadDir(root) - if err != nil { - return nil, fmt.Errorf("read %s: %w", root, err) - } - var dirs []string - for _, e := range entries { - if !e.IsDir() { - continue - } - d := filepath.Join(root, e.Name()) - if d == filepath.Clean(out) { - continue - } - if empty, _ := dirEmpty(d); empty { - continue - } - dirs = append(dirs, d) - } - return dirs, nil -} - -// --- CI runner side effects ------------------------------------------------ - -func ciPrepareRunner() error { - printDisk("disk before:") - free, err := devtools.FreeDisk("/") - if err != nil { - return fmt.Errorf("free disk: %w", err) - } - if devtools.NeedsDiskFreeing(free) { - // Free ~25-30GB of preinstalled SDKs; ignore failures (|| true in the script). - bloat := []string{ - "/usr/share/dotnet", "/opt/ghc", "/usr/local/lib/android", - "/opt/hostedtoolcache/CodeQL", "/usr/share/swift", - "/usr/local/share/boost", "/usr/lib/jvm", - } - _ = run("sudo", append([]string{"rm", "-rf"}, bloat...)...) - _ = run("sudo", "docker", "image", "prune", "-af") - printDisk("disk after:") - } else { - fmt.Printf("%d GB free: the runner has room, nothing deleted\n", free>>30) - } - - // Enable the containerd image store (build-cache export + --platform). - tee := exec.Command("sudo", "tee", "/etc/docker/daemon.json") - tee.Stdin = strings.NewReader(`{"features":{"containerd-snapshotter":true}}`) - tee.Stdout, tee.Stderr = os.Stdout, os.Stderr - if err := tee.Run(); err != nil { - return fmt.Errorf("write daemon.json: %w", err) - } - if err := run("sudo", "systemctl", "restart", "docker"); err != nil { - return fmt.Errorf("restart docker: %w", err) - } - return run("docker", "info", "-f", "driver={{.DriverStatus}}") -} - -func dockerCacheExport() error { - d, err := os.MkdirTemp("", "devcontainer-cache-export") - if err != nil { - return err - } - defer os.RemoveAll(d) - if err := os.WriteFile(filepath.Join(d, "Dockerfile"), []byte("FROM busybox\n"), 0o644); err != nil { - return err - } - // Exit non-zero (silently) iff the builder can't export cache — the caller uses - // this as a fast precondition probe. - cmd := exec.Command("docker", "buildx", "build", - "--cache-to=type=local,dest="+filepath.Join(d, "c"), - "-t", "devcontainer-cache-export-probe", d) - if err := cmd.Run(); err != nil { - return exitError{code: 1} - } - return nil -} - -// --- helpers --------------------------------------------------------------- - -func readLines(r io.Reader) ([]string, error) { - var lines []string - sc := bufio.NewScanner(r) - sc.Buffer(make([]byte, 0, 64*1024), 4*1024*1024) - for sc.Scan() { - lines = append(lines, sc.Text()) - } - return lines, sc.Err() -} - -func run(name string, args ...string) error { - cmd := exec.Command(name, args...) - cmd.Stdout, cmd.Stderr = os.Stdout, os.Stderr - return cmd.Run() -} - -func runOut(name string, args ...string) (string, error) { - out, err := exec.Command(name, args...).Output() - return strings.TrimRight(string(out), "\n"), err -} - -func gitCommitExists(ref string) bool { - return exec.Command("git", "cat-file", "-e", ref+"^{commit}").Run() == nil -} - -func printDisk(label string) { - fmt.Println(label) - _ = run("sh", "-c", "df -h / | tail -1") -} - -func dirEmpty(dir string) (bool, error) { - entries, err := os.ReadDir(dir) - if err != nil { - return false, err - } - return len(entries) == 0, nil -} - -// --- release --------------------------------------------------------------- - -// releaseResolve writes the version, tag and commit to release to $GITHUB_OUTPUT -// (or stdout), and a line to the job summary. What only the workflow knows comes -// in the environment: GITHUB_EVENT_NAME, VERSION_INPUT, GITHUB_REF_NAME, GITHUB_SHA. -func releaseResolve() error { - in := devtools.ReleaseInput{ - Event: os.Getenv("GITHUB_EVENT_NAME"), - VersionInput: os.Getenv("VERSION_INPUT"), - RefName: os.Getenv("GITHUB_REF_NAME"), - PushedSHA: os.Getenv("GITHUB_SHA"), - Today: time.Now().UTC().Format("20060102"), - TagExists: func(tag string) bool { - return exec.Command("git", "rev-parse", "-q", "--verify", "refs/tags/"+tag).Run() == nil - }, - } - if in.Event == "" || in.Event == "workflow_dispatch" { - tags, err := runOut("git", "tag", "-l", "v"+in.Today+".*") - if err != nil { - return fmt.Errorf("list tags: %w", err) - } - in.TagsToday = len(strings.Fields(tags)) - if in.HeadSHA, err = runOut("git", "rev-parse", "HEAD"); err != nil { - return fmt.Errorf("resolve HEAD: %w", err) - } - } - rel, err := devtools.ResolveRelease(in) - if err != nil { - return exitError{code: 1, msg: "::error::" + err.Error()} - } - if err := appendTo("GITHUB_OUTPUT", fmt.Sprintf("version=%s\ntag=%s\nsha=%s\n", rel.Version, rel.Tag, rel.SHA)); err != nil { - return err - } - return appendTo("GITHUB_STEP_SUMMARY", fmt.Sprintf("Releasing %s (version %s) from %s\n", rel.Tag, rel.Version, rel.SHA)) -} - -// appendTo appends s to the file the environment variable names, or writes it to -// stdout outside a workflow. -func appendTo(env, s string) error { - path := os.Getenv(env) - if path == "" { - _, err := fmt.Print(s) - return err - } - f, err := os.OpenFile(path, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o644) - if err != nil { - return fmt.Errorf("open %s: %w", env, err) - } - if _, err := f.WriteString(s); err != nil { - f.Close() - return fmt.Errorf("write %s: %w", env, err) - } - return f.Close() -} - -// --- metrics --------------------------------------------------------------- - -// metrics writes artifacts/metrics.json: startup time of the binary against the -// Node oracle (METRICS_RUNS runs, 30 by default) and the size of the binaries. -// Every measurement is best effort: one that cannot be taken is null. -// -// devtool metrics -func metrics(args []string) error { - if len(args) != 4 { - return fmt.Errorf("usage: devtool metrics ") - } - version, bin, oracle, out := args[0], args[1], args[2], args[3] - runs := 30 - if v := os.Getenv("METRICS_RUNS"); v != "" { - n, err := strconv.Atoi(v) - if err != nil { - return fmt.Errorf("METRICS_RUNS %q: %w", v, err) - } - runs = n - } - quiet := func(name string, args ...string) func() { - return func() { _ = exec.Command(name, args...).Run() } - } - fmt.Printf("metrics: timing tool = go, runs=%d\n", runs) - m := devtools.Metrics{ - Version: version, - GeneratedAt: time.Now().UTC().Format(time.RFC3339), - TimingTool: "go", - Runs: runs, - } - m.StartupMs.GoVersion = new(devtools.MeanMillis(runs, quiet(bin, "--version"))) - - // read-configuration against a minimal image-based workspace: variable - // substitution only, no Docker involved. - ws, err := os.MkdirTemp("", "devcontainer-metrics-") - if err != nil { - return err - } - defer os.RemoveAll(ws) - cfg := filepath.Join(ws, ".devcontainer", "devcontainer.json") - if err := os.MkdirAll(filepath.Dir(cfg), 0o755); err != nil { - return err - } - if err := os.WriteFile(cfg, []byte(`{"image":"mcr.microsoft.com/devcontainers/base:ubuntu"}`+"\n"), 0o644); err != nil { - return err - } - m.StartupMs.GoReadConfiguration = new(devtools.MeanMillis(runs, quiet(bin, "read-configuration", "--workspace-folder", ws))) - - if _, err := os.Stat(oracle); err == nil { - if _, err := exec.LookPath("node"); err == nil { - m.StartupMs.NodeVersion = new(devtools.MeanMillis(runs, quiet("node", oracle, "--version"))) - } - } - if m.StartupMs.NodeVersion == nil { - fmt.Printf("metrics: node oracle not found at %s (run 'task reference') — skipping Node A/B\n", oracle) - } - - dist := filepath.Join(filepath.Dir(bin), "dist", filepath.Base(bin)) - m.SizesBytes = devtools.SizesBytes{ - LocalBinary: devtools.FileSize(bin), - LinuxAmd64Binary: devtools.FileSize(dist + "-linux-amd64"), - LinuxAmd64Gzip: devtools.GzipSize(dist + "-linux-amd64"), - LinuxArm64Binary: devtools.FileSize(dist + "-linux-arm64"), - LinuxArm64Gzip: devtools.GzipSize(dist + "-linux-arm64"), - } - if m.SizesBytes.LinuxAmd64Binary == nil { - fmt.Println("metrics: dist binaries absent — run 'task build:cross' for release sizes") - } - - data, err := json.MarshalIndent(m, "", " ") - if err != nil { - return err - } - if err := os.MkdirAll(filepath.Dir(out), 0o755); err != nil { - return err - } - if err := os.WriteFile(out, append(data, '\n'), 0o644); err != nil { - return err - } - fmt.Printf("metrics: wrote %s\n%s\n", out, data) - return nil + os.Exit(devtools.Main(os.Args, os.Stdin, os.Stdout, os.Stderr)) } diff --git a/cmd/devtool/main_test.go b/cmd/devtool/main_test.go new file mode 100644 index 0000000..21d25f3 --- /dev/null +++ b/cmd/devtool/main_test.go @@ -0,0 +1,47 @@ +package main + +import ( + "errors" + "os" + "os/exec" + "strings" + "testing" +) + +// TestMain runs the command itself, instead of the tests, in the child process +// TestTheCommandIsWired starts. +func TestMain(m *testing.M) { + if os.Getenv("DEVTOOL_RUN_MAIN") == "1" { + main() + } + os.Exit(m.Run()) +} + +// devtool runs this test binary as the devtool command, with args and stdin. +func devtool(t *testing.T, stdin string, args ...string) (code int, stdout, stderr string) { + t.Helper() + cmd := exec.Command(os.Args[0], args...) + // GOCOVERDIR keeps a binary built with coverage from warning on stderr that it + // has nowhere to write its counters. + cmd.Env = append(os.Environ(), "DEVTOOL_RUN_MAIN=1", "GOCOVERDIR="+t.TempDir()) + cmd.Stdin = strings.NewReader(stdin) + var o, e strings.Builder + cmd.Stdout, cmd.Stderr = &o, &e + err := cmd.Run() + var exitErr *exec.ExitError + if err != nil && !errors.As(err, &exitErr) { + t.Fatal(err) + } + return cmd.ProcessState.ExitCode(), o.String(), e.String() +} + +// TestTheCommandIsWired holds what main adds to devtools.Main: the process's +// arguments, standard streams and exit code. +func TestTheCommandIsWired(t *testing.T) { + if code, out, errOut := devtool(t, "internal/cli/exec.go\n", "parity-affected"); code != 0 || out != "exec\n" || errOut != "" { + t.Errorf("parity-affected: code=%d stdout=%q stderr=%q, want 0 \"exec\\n\" \"\"", code, out, errOut) + } + if code, out, errOut := devtool(t, "", "nope"); code != 2 || out != "" || errOut != "devtool: unknown command \"nope\"\n" { + t.Errorf("unknown command: code=%d stdout=%q stderr=%q, want 2", code, out, errOut) + } +} diff --git a/internal/devtools/command.go b/internal/devtools/command.go new file mode 100644 index 0000000..05b3cd2 --- /dev/null +++ b/internal/devtools/command.go @@ -0,0 +1,549 @@ +package devtools + +import ( + "bufio" + "encoding/json" + "fmt" + "io" + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "time" +) + +// Main runs the devtool command args names (args[0] is the program, args[1] the +// command) and returns the process's exit code: 2 for a usage error, the code a +// gate or a probe chose, 1 for any other failure. Each command wires stdin, +// stdout, git, `go tool` and docker around the pure logic of this package; the +// children a command starts write to stdout and stderr too. +func Main(args []string, stdin io.Reader, stdout, stderr io.Writer) int { + if len(args) < 2 { + fmt.Fprintln(stderr, "usage: devtool [args...]") + return 2 + } + t := tool{in: stdin, out: stdout, errw: stderr} + cmd, rest := args[1], args[2:] + var err error + switch cmd { + case "parity-affected": + err = t.parityAffected() + case "parity-affected-commands": + err = t.parityAffectedCommands(rest) + case "daily-changed": + err = t.dailyChanged(rest) + case "coverage-report": + err = t.coverageReport(rest) + case "coverage-gate": + err = t.coverageGate(rest) + case "coverage-merge": + err = t.coverageMerge(rest) + case "ci-prepare-runner": + err = t.ciPrepareRunner() + case "docker-cache-export": + err = t.dockerCacheExport() + case "release-resolve": + err = t.releaseResolve() + case "metrics": + err = t.metrics(rest) + default: + fmt.Fprintf(stderr, "devtool: unknown command %q\n", cmd) + return 2 + } + if err == nil { + return 0 + } + // exitError carries a specific code (gate failures, probe failures); other + // errors are usage/infra problems. + if ee, ok := err.(exitError); ok { + if ee.msg != "" { + fmt.Fprintln(stderr, ee.msg) + } + return ee.code + } + fmt.Fprintf(stderr, "devtool %s: %v\n", cmd, err) + return 1 +} + +type exitError struct { + code int + msg string +} + +func (e exitError) Error() string { return e.msg } + +// tool is where a command reads its input and writes its output. +type tool struct { + in io.Reader + out, errw io.Writer +} + +// --- parity selection ------------------------------------------------------ + +func (t tool) parityAffected() error { + files, err := readLines(t.in) + if err != nil { + return err + } + fmt.Fprintln(t.out, ParityAffected(files)) + return nil +} + +func (t tool) parityAffectedCommands(args []string) error { + base := "" + if len(args) > 0 { + base = args[0] + } + head := "HEAD" + if len(args) > 1 { + head = args[1] + } + + if base == "" || !gitCommitExists(base) { + fmt.Fprintln(t.errw, "unknown base → running the full matrix") + fmt.Fprintln(t.out, "commands=all") + fmt.Fprintln(t.out, "run=true") + return nil + } + + out, err := runOut("git", "diff", "--name-only", base, head) + if err != nil { + return fmt.Errorf("git diff: %w", err) + } + fmt.Fprintln(t.errw, "changed files:") + fmt.Fprintln(t.errw, out) + + cmds := ParityAffected(strings.Split(out, "\n")) + fmt.Fprintf(t.errw, "affected parity commands: %s\n", cmds) + + fmt.Fprintf(t.out, "commands=%s\n", cmds) + if cmds == "none" { + fmt.Fprintln(t.out, "run=false") + } else { + fmt.Fprintln(t.out, "run=true") + } + return nil +} + +func (t tool) dailyChanged(args []string) error { + window := 25 + if len(args) > 0 { + w, err := strconv.Atoi(args[0]) + if err != nil { + return fmt.Errorf("window-hours %q: %w", args[0], err) + } + window = w + } + + now, err := nowEpoch() + if err != nil { + return err + } + lastStr, err := runOut("git", "log", "-1", "--format=%ct") + if err != nil { + return fmt.Errorf("git log: %w", err) + } + last, err := strconv.Atoi(strings.TrimSpace(lastStr)) + if err != nil { + return fmt.Errorf("commit timestamp %q: %w", lastStr, err) + } + + fmt.Fprintf(t.errw, "HEAD commit is %dh old (window %dh)\n", (now-last)/3600, window) + if DailyChanged(last, now, window) { + fmt.Fprintln(t.out, "run=true") + } else { + fmt.Fprintln(t.out, "run=false") + fmt.Fprintf(t.errw, "no commits within the last %dh — skipping the full runtime matrix\n", window) + } + return nil +} + +// nowEpoch honors DAILY_NOW_EPOCH (tests / reproducibility), else wall clock. +func nowEpoch() (int, error) { + if v := os.Getenv("DAILY_NOW_EPOCH"); v != "" { + n, err := strconv.Atoi(v) + if err != nil { + return 0, fmt.Errorf("DAILY_NOW_EPOCH %q: %w", v, err) + } + return n, nil + } + return int(time.Now().Unix()), nil +} + +// --- coverage -------------------------------------------------------------- + +func (t tool) coverageReport(args []string) error { + if len(args) < 1 { + return fmt.Errorf("usage: coverage-report [title]") + } + src := args[0] + title := "coverage" + if len(args) > 1 { + title = args[1] + } + report, err := t.renderCoverage(src, title) + if err != nil { + return err + } + fmt.Fprint(t.out, report) + return nil +} + +// renderCoverage renders a profile file or a covdata directory (converted with +// `go tool covdata textfmt`) to a Markdown table. +func (t tool) renderCoverage(src, title string) (string, error) { + profile := src + if info, err := os.Stat(src); err == nil && info.IsDir() { + if empty, _ := dirEmpty(src); empty { + return fmt.Sprintf("### Coverage — %s\n\n_(no coverage data)_\n", title), nil + } + profile = strings.TrimRight(src, "/") + ".out" + if err := t.run("go", "tool", "covdata", "textfmt", "-i="+src, "-o="+profile); err != nil { + return "", fmt.Errorf("covdata textfmt: %w", err) + } + } + f, err := os.Open(profile) + if err != nil { + return fmt.Sprintf("### Coverage — %s\n\n_(no coverage data)_\n", title), nil + } + defer f.Close() + pkgs, total, err := ParseProfile(f) + if err != nil { + return "", err + } + return RenderReport(pkgs, total, title), nil +} + +func (t tool) coverageGate(args []string) error { + if len(args) < 2 { + return fmt.Errorf("usage: coverage-gate TOTAL= [pkg=min ...]") + } + f, err := os.Open(args[0]) + if err != nil { + return fmt.Errorf("open profile: %w", err) + } + defer f.Close() + pkgs, total, err := ParseProfile(f) + if err != nil { + return err + } + floors, err := ParseFloors(args[1:]) + if err != nil { + return err + } + results, ok := CoverageGate(pkgs, total, floors) + for _, r := range results { + fmt.Fprintln(t.out, r.Line) + } + if !ok { + return exitError{code: 1} + } + return nil +} + +func (t tool) coverageMerge(args []string) error { + if len(args) < 1 { + return fmt.Errorf("usage: coverage-merge [out-dir] [title]") + } + root := args[0] + out := strings.TrimRight(root, "/") + "/merged" + if len(args) > 1 { + out = args[1] + } + title := "merged (all lanes)" + if len(args) > 2 { + title = args[2] + } + + inputs, err := laneDirs(root, out) + if err != nil { + return err + } + if len(inputs) == 0 { + fmt.Fprintf(t.out, "### Coverage — %s\n\n_(no lane data to merge)_\n", title) + return nil + } + fmt.Fprintf(t.errw, "merging coverage lanes: %s\n", strings.Join(inputs, ",")) + + if err := os.RemoveAll(out); err != nil { + return err + } + if err := os.MkdirAll(out, 0o755); err != nil { + return err + } + if err := t.run("go", "tool", "covdata", "merge", "-i="+strings.Join(inputs, ","), "-o="+out); err != nil { + return fmt.Errorf("covdata merge: %w", err) + } + report, err := t.renderCoverage(out, title) + if err != nil { + return err + } + fmt.Fprint(t.out, report) + return nil +} + +// laneDirs returns the non-empty per-lane covdata subdirs under root (skipping the +// merge output dir itself). +func laneDirs(root, out string) ([]string, error) { + entries, err := os.ReadDir(root) + if err != nil { + return nil, fmt.Errorf("read %s: %w", root, err) + } + var dirs []string + for _, e := range entries { + if !e.IsDir() { + continue + } + d := filepath.Join(root, e.Name()) + if d == filepath.Clean(out) { + continue + } + if empty, _ := dirEmpty(d); empty { + continue + } + dirs = append(dirs, d) + } + return dirs, nil +} + +// --- CI runner side effects ------------------------------------------------ + +func (t tool) ciPrepareRunner() error { + t.printDisk("disk before:") + free, err := FreeDisk("/") + if err != nil { + return fmt.Errorf("free disk: %w", err) + } + return t.prepareRunner(free) +} + +// prepareRunner makes a runner with free bytes on / ready for the runtime lanes: +// it deletes the preinstalled SDKs when the runner lacks RunnerDiskRoom, and +// switches docker to the containerd image store. +func (t tool) prepareRunner(free uint64) error { + if NeedsDiskFreeing(free) { + // Free ~25-30GB of preinstalled SDKs; ignore failures (|| true in the script). + bloat := []string{ + "/usr/share/dotnet", "/opt/ghc", "/usr/local/lib/android", + "/opt/hostedtoolcache/CodeQL", "/usr/share/swift", + "/usr/local/share/boost", "/usr/lib/jvm", + } + _ = t.run("sudo", append([]string{"rm", "-rf"}, bloat...)...) + _ = t.run("sudo", "docker", "image", "prune", "-af") + t.printDisk("disk after:") + } else { + fmt.Fprintf(t.out, "%d GB free: the runner has room, nothing deleted\n", free>>30) + } + + // Enable the containerd image store (build-cache export + --platform). + tee := exec.Command("sudo", "tee", "/etc/docker/daemon.json") + tee.Stdin = strings.NewReader(`{"features":{"containerd-snapshotter":true}}`) + tee.Stdout, tee.Stderr = t.out, t.errw + if err := tee.Run(); err != nil { + return fmt.Errorf("write daemon.json: %w", err) + } + if err := t.run("sudo", "systemctl", "restart", "docker"); err != nil { + return fmt.Errorf("restart docker: %w", err) + } + return t.run("docker", "info", "-f", "driver={{.DriverStatus}}") +} + +func (t tool) dockerCacheExport() error { + d, err := os.MkdirTemp("", "devcontainer-cache-export") + if err != nil { + return err + } + defer os.RemoveAll(d) + if err := os.WriteFile(filepath.Join(d, "Dockerfile"), []byte("FROM busybox\n"), 0o644); err != nil { // mutate-exempt: only the build below reads the file, as its owner, and it is removed on return + return err + } + // Exit non-zero (silently) iff the builder can't export cache — the caller uses + // this as a fast precondition probe. + cmd := exec.Command("docker", "buildx", "build", + "--cache-to=type=local,dest="+filepath.Join(d, "c"), + "-t", "devcontainer-cache-export-probe", d) + if err := cmd.Run(); err != nil { + return exitError{code: 1} + } + return nil +} + +// --- helpers --------------------------------------------------------------- + +// readLines splits r into lines. bufio.Scanner's default 64 KiB token limit is +// far beyond any path git prints (PATH_MAX is 4096). +func readLines(r io.Reader) ([]string, error) { + var lines []string + sc := bufio.NewScanner(r) + for sc.Scan() { + lines = append(lines, sc.Text()) + } + return lines, sc.Err() +} + +func (t tool) run(name string, args ...string) error { + cmd := exec.Command(name, args...) + cmd.Stdout, cmd.Stderr = t.out, t.errw + return cmd.Run() +} + +func runOut(name string, args ...string) (string, error) { + out, err := exec.Command(name, args...).Output() + return strings.TrimRight(string(out), "\n"), err +} + +func gitCommitExists(ref string) bool { + return exec.Command("git", "cat-file", "-e", ref+"^{commit}").Run() == nil +} + +func (t tool) printDisk(label string) { + fmt.Fprintln(t.out, label) + _ = t.run("sh", "-c", "df -h / | tail -1") +} + +func dirEmpty(dir string) (bool, error) { + entries, err := os.ReadDir(dir) + if err != nil { + return false, err + } + return len(entries) == 0, nil +} + +// --- release --------------------------------------------------------------- + +// releaseResolve writes the version, tag and commit to release to $GITHUB_OUTPUT +// (or stdout), and a line to the job summary. What only the workflow knows comes +// in the environment: GITHUB_EVENT_NAME, VERSION_INPUT, GITHUB_REF_NAME, GITHUB_SHA. +func (t tool) releaseResolve() error { + in := ReleaseInput{ + Event: os.Getenv("GITHUB_EVENT_NAME"), + VersionInput: os.Getenv("VERSION_INPUT"), + RefName: os.Getenv("GITHUB_REF_NAME"), + PushedSHA: os.Getenv("GITHUB_SHA"), + Today: time.Now().UTC().Format("20060102"), + TagExists: func(tag string) bool { + return exec.Command("git", "rev-parse", "-q", "--verify", "refs/tags/"+tag).Run() == nil + }, + } + if in.Event == "" || in.Event == "workflow_dispatch" { + tags, err := runOut("git", "tag", "-l", "v"+in.Today+".*") + if err != nil { + return fmt.Errorf("list tags: %w", err) + } + in.TagsToday = len(strings.Fields(tags)) + if in.HeadSHA, err = runOut("git", "rev-parse", "HEAD"); err != nil { + return fmt.Errorf("resolve HEAD: %w", err) + } + } + rel, err := ResolveRelease(in) + if err != nil { + return exitError{code: 1, msg: "::error::" + err.Error()} + } + if err := t.appendTo("GITHUB_OUTPUT", fmt.Sprintf("version=%s\ntag=%s\nsha=%s\n", rel.Version, rel.Tag, rel.SHA)); err != nil { + return err + } + return t.appendTo("GITHUB_STEP_SUMMARY", fmt.Sprintf("Releasing %s (version %s) from %s\n", rel.Tag, rel.Version, rel.SHA)) +} + +// appendTo appends s to the file the environment variable names, or writes it to +// stdout outside a workflow. +func (t tool) appendTo(env, s string) error { + path := os.Getenv(env) + if path == "" { + _, err := fmt.Fprint(t.out, s) + return err + } + f, err := os.OpenFile(path, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o644) + if err != nil { + return fmt.Errorf("open %s: %w", env, err) + } + if _, err := f.WriteString(s); err != nil { + f.Close() + return fmt.Errorf("write %s: %w", env, err) + } + return f.Close() +} + +// --- metrics --------------------------------------------------------------- + +// metrics writes artifacts/metrics.json: startup time of the binary against the +// Node oracle (METRICS_RUNS runs, 30 by default) and the size of the binaries. +// Every measurement is best effort: one that cannot be taken is null. +// +// devtool metrics +func (t tool) metrics(args []string) error { + if len(args) != 4 { + return fmt.Errorf("usage: devtool metrics ") + } + version, bin, oracle, out := args[0], args[1], args[2], args[3] + runs := 30 + if v := os.Getenv("METRICS_RUNS"); v != "" { + n, err := strconv.Atoi(v) + if err != nil { + return fmt.Errorf("METRICS_RUNS %q: %w", v, err) + } + runs = n + } + quiet := func(name string, args ...string) func() { + return func() { _ = exec.Command(name, args...).Run() } + } + fmt.Fprintf(t.out, "metrics: timing tool = go, runs=%d\n", runs) + m := Metrics{ + Version: version, + GeneratedAt: time.Now().UTC().Format(time.RFC3339), + TimingTool: "go", + Runs: runs, + } + m.StartupMs.GoVersion = new(MeanMillis(runs, quiet(bin, "--version"))) + + // read-configuration against a minimal image-based workspace: variable + // substitution only, no Docker involved. + ws, err := os.MkdirTemp("", "devcontainer-metrics-") + if err != nil { + return err + } + defer os.RemoveAll(ws) + cfg := filepath.Join(ws, ".devcontainer", "devcontainer.json") + if err := os.MkdirAll(filepath.Dir(cfg), 0o755); err != nil { // mutate-exempt: a scratch workspace only this process's children read, removed on return + return err + } + if err := os.WriteFile(cfg, []byte(`{"image":"mcr.microsoft.com/devcontainers/base:ubuntu"}`+"\n"), 0o644); err != nil { // mutate-exempt: a scratch workspace only this process's children read, removed on return + return err + } + m.StartupMs.GoReadConfiguration = new(MeanMillis(runs, quiet(bin, "read-configuration", "--workspace-folder", ws))) + + if _, err := os.Stat(oracle); err == nil { + if _, err := exec.LookPath("node"); err == nil { + m.StartupMs.NodeVersion = new(MeanMillis(runs, quiet("node", oracle, "--version"))) + } + } + if m.StartupMs.NodeVersion == nil { + fmt.Fprintf(t.out, "metrics: node oracle not found at %s (run 'task reference') — skipping Node A/B\n", oracle) + } + + dist := filepath.Join(filepath.Dir(bin), "dist", filepath.Base(bin)) + m.SizesBytes = SizesBytes{ + LocalBinary: FileSize(bin), + LinuxAmd64Binary: FileSize(dist + "-linux-amd64"), + LinuxAmd64Gzip: GzipSize(dist + "-linux-amd64"), + LinuxArm64Binary: FileSize(dist + "-linux-arm64"), + LinuxArm64Gzip: GzipSize(dist + "-linux-arm64"), + } + if m.SizesBytes.LinuxAmd64Binary == nil { + fmt.Fprintln(t.out, "metrics: dist binaries absent — run 'task build:cross' for release sizes") + } + + data, err := json.MarshalIndent(m, "", " ") + if err != nil { + return err + } + if err := os.MkdirAll(filepath.Dir(out), 0o755); err != nil { + return err + } + if err := os.WriteFile(out, append(data, '\n'), 0o644); err != nil { + return err + } + fmt.Fprintf(t.out, "metrics: wrote %s\n%s\n", out, data) + return nil +} diff --git a/internal/devtools/command_test.go b/internal/devtools/command_test.go new file mode 100644 index 0000000..ef4e96a --- /dev/null +++ b/internal/devtools/command_test.go @@ -0,0 +1,608 @@ +package devtools + +import ( + "encoding/json" + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "syscall" + "testing" + "time" +) + +// TestMain fixes the umask a CI runner has, so the modes of the files the +// commands leave behind are the ones the tests assert. +func TestMain(m *testing.M) { + syscall.Umask(0o022) + os.Exit(m.Run()) +} + +// runMain runs one devtool command as the binary would and returns its exit code +// and what it wrote. +func runMain(t *testing.T, stdin string, args ...string) (code int, stdout, stderr string) { + t.Helper() + var o, e strings.Builder + code = Main(append([]string{"devtool"}, args...), strings.NewReader(stdin), &o, &e) + return code, o.String(), e.String() +} + +// fakeTools puts an executable per scripts key first on PATH, standing in for a +// host tool the command starts. Each logs its name and argv, one line per call, +// to the returned file, then runs its body. +func fakeTools(t *testing.T, scripts map[string]string) (log string) { + t.Helper() + dir := t.TempDir() + log = filepath.Join(dir, "calls.log") + for name, body := range scripts { + script := "#!/bin/sh\necho \"" + name + " $*\" >> '" + log + "'\n" + body + "\n" + if err := os.WriteFile(filepath.Join(dir, name), []byte(script), 0o755); err != nil { + t.Fatal(err) + } + } + t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) + return log +} + +func readFile(t *testing.T, path string) string { + t.Helper() + b, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + return string(b) +} + +func writeFile(t *testing.T, path, content string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatal(err) + } +} + +func assertMode(t *testing.T, path string, want os.FileMode) { + t.Helper() + fi, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if got := fi.Mode().Perm(); got != want { + t.Errorf("mode of %s = %o, want %o", path, got, want) + } +} + +// gitRepo makes an empty repository in a new directory and enters it, with no +// user or system configuration to sign or hook its commits. +func gitRepo(t *testing.T) { + t.Helper() + t.Chdir(t.TempDir()) + t.Setenv("GIT_CONFIG_GLOBAL", os.DevNull) + t.Setenv("GIT_CONFIG_NOSYSTEM", "1") + git(t, "init", "-q") +} + +func git(t *testing.T, args ...string) string { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Env = append(os.Environ(), + "GIT_AUTHOR_NAME=t", "GIT_AUTHOR_EMAIL=t@example.com", + "GIT_COMMITTER_NAME=t", "GIT_COMMITTER_EMAIL=t@example.com") + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + return strings.TrimSpace(string(out)) +} + +// commit adds file and commits it at epoch, returning the commit's hash. +func commit(t *testing.T, file string, epoch int64) string { + t.Helper() + writeFile(t, file, file) + git(t, "add", file) + date := "@" + strconv.FormatInt(epoch, 10) + " +0000" + t.Setenv("GIT_COMMITTER_DATE", date) + t.Setenv("GIT_AUTHOR_DATE", date) + git(t, "commit", "-q", "-m", file) + return git(t, "rev-parse", "HEAD") +} + +func TestMainUsageErrors(t *testing.T) { + if code, out, errOut := runMain(t, ""); code != 2 || out != "" || errOut != "usage: devtool [args...]\n" { + t.Errorf("no command: code=%d stdout=%q stderr=%q", code, out, errOut) + } + if code, out, errOut := runMain(t, "", "nope"); code != 2 || out != "" || errOut != "devtool: unknown command \"nope\"\n" { + t.Errorf("unknown command: code=%d stdout=%q stderr=%q", code, out, errOut) + } + code, out, errOut := runMain(t, "", "coverage-report") + if code != 1 || out != "" || errOut != "devtool coverage-report: usage: coverage-report [title]\n" { + t.Errorf("a command's error: code=%d stdout=%q stderr=%q", code, out, errOut) + } +} + +func TestParityAffectedReadsFilesFromStdin(t *testing.T) { + code, out, errOut := runMain(t, "internal/cli/exec.go\nREADME.md\n", "parity-affected") + if code != 0 || out != "exec\n" || errOut != "" { + t.Errorf("code=%d stdout=%q stderr=%q, want 0 \"exec\\n\" \"\"", code, out, errOut) + } +} + +func TestParityAffectedCommands(t *testing.T) { + gitRepo(t) + c0 := commit(t, "a.txt", 1_700_000_000) + c1 := commit(t, "README.md", 1_700_000_100) + commit(t, "internal/cli/exec.go", 1_700_000_200) + + full := "commands=all\nrun=true\n" + unknown := "unknown base → running the full matrix\n" + cases := []struct { + name string + args []string + stdout, stderr string + }{ + {"no base", nil, full, unknown}, + {"base that is no commit", []string{"0123456789abcdef0123456789abcdef01234567"}, full, unknown}, + {"base against HEAD", []string{c0}, "commands=exec\nrun=true\n", + "changed files:\nREADME.md\ninternal/cli/exec.go\naffected parity commands: exec\n"}, + {"base against head", []string{c0, c1}, "commands=none\nrun=false\n", + "changed files:\nREADME.md\naffected parity commands: none\n"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + code, out, errOut := runMain(t, "", append([]string{"parity-affected-commands"}, tc.args...)...) + if code != 0 || out != tc.stdout || errOut != tc.stderr { + t.Errorf("code=%d\nstdout=%q, want %q\nstderr=%q, want %q", code, out, tc.stdout, errOut, tc.stderr) + } + }) + } +} + +func TestDailyChangedCommand(t *testing.T) { + gitRepo(t) + const last = 1_700_000_000 + commit(t, "a.txt", last) + + cases := []struct { + name string + now string + args []string + stdout, stderr string + }{ + {"inside the default window", strconv.Itoa(last + 2*3600), nil, "run=true\n", + "HEAD commit is 2h old (window 25h)\n"}, + {"at the default window", strconv.Itoa(last + 25*3600), nil, "run=false\n", + "HEAD commit is 25h old (window 25h)\nno commits within the last 25h — skipping the full runtime matrix\n"}, + {"a window of its own", strconv.Itoa(last + 3*3600 + 3599), []string{"4"}, "run=true\n", + "HEAD commit is 3h old (window 4h)\n"}, + {"past a window of its own", strconv.Itoa(last + 4*3600), []string{"4"}, "run=false\n", + "HEAD commit is 4h old (window 4h)\nno commits within the last 4h — skipping the full runtime matrix\n"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Setenv("DAILY_NOW_EPOCH", tc.now) + code, out, errOut := runMain(t, "", append([]string{"daily-changed"}, tc.args...)...) + if code != 0 || out != tc.stdout || errOut != tc.stderr { + t.Errorf("code=%d\nstdout=%q, want %q\nstderr=%q, want %q", code, out, tc.stdout, errOut, tc.stderr) + } + }) + } + + t.Run("a window that is no number", func(t *testing.T) { + code, out, errOut := runMain(t, "", "daily-changed", "x") + want := "devtool daily-changed: window-hours \"x\": strconv.Atoi: parsing \"x\": invalid syntax\n" + if code != 1 || out != "" || errOut != want { + t.Errorf("code=%d stdout=%q stderr=%q, want 1 \"\" %q", code, out, errOut, want) + } + }) + + t.Run("wall clock", func(t *testing.T) { + t.Setenv("DAILY_NOW_EPOCH", "") + commit(t, "b.txt", time.Now().Unix()) + if code, out, _ := runMain(t, "", "daily-changed"); code != 0 || out != "run=true\n" { + t.Errorf("a commit made now: code=%d stdout=%q, want run=true", code, out) + } + }) +} + +const sampleReport = "| Package | Stmts | Coverage |\n|---|---:|---:|\n" + + "| internal/cli | 8 | 62.5% |\n" + + "| internal/docker | 6 | 100.0% |\n" + + "| **TOTAL** | **14** | **78.6%** |\n" + +// fakeGo stands in for `go tool covdata`: textfmt writes FAKE_PROFILE to its -o +// file, and merge leaves a file in its -o directory. +func fakeGo(t *testing.T) (log string) { + t.Helper() + t.Setenv("FAKE_PROFILE", sampleProfile) + return fakeTools(t, map[string]string{"go": `for a in "$@"; do case "$a" in -o=*) out="${a#-o=}";; esac; done +case "$3" in +textfmt) printf '%s' "$FAKE_PROFILE" > "$out" ;; +merge) echo merged > "$out/covmeta" ;; +esac`}) +} + +func TestCoverageReport(t *testing.T) { + t.Chdir(t.TempDir()) + dir := t.TempDir() + profile := filepath.Join(dir, "coverage.out") + writeFile(t, profile, sampleProfile) + empty := filepath.Join(dir, "empty") + if err := os.Mkdir(empty, 0o755); err != nil { + t.Fatal(err) + } + covdata := filepath.Join(dir, "e2e") + writeFile(t, filepath.Join(covdata, "covmeta"), "x") + log := fakeGo(t) + + cases := []struct { + name string + args []string + want string + }{ + {"profile", []string{profile}, "### Coverage — coverage (total 78.6%)\n\n" + sampleReport}, + {"profile with a title", []string{profile, "unit (hermetic)"}, "### Coverage — unit (hermetic) (total 78.6%)\n\n" + sampleReport}, + {"missing profile", []string{filepath.Join(dir, "none.out"), "e2e"}, "### Coverage — e2e\n\n_(no coverage data)_\n"}, + {"empty covdata", []string{empty, "e2e"}, "### Coverage — e2e\n\n_(no coverage data)_\n"}, + {"covdata", []string{covdata + "/", "e2e"}, "### Coverage — e2e (total 78.6%)\n\n" + sampleReport}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + code, out, errOut := runMain(t, "", append([]string{"coverage-report"}, tc.args...)...) + if code != 0 || out != tc.want || errOut != "" { + t.Errorf("code=%d stderr=%q\nstdout=%q\nwant %q", code, errOut, out, tc.want) + } + }) + } + want := "go tool covdata textfmt -i=" + covdata + "/ -o=" + covdata + ".out\n" + if got := readFile(t, log); got != want { + t.Errorf("go calls = %q, want %q", got, want) + } +} + +func TestCoverageGateCommand(t *testing.T) { + profile := filepath.Join(t.TempDir(), "coverage.out") + writeFile(t, profile, sampleProfile) + + code, out, errOut := runMain(t, "", "coverage-gate", profile, "TOTAL=70") + if code != 0 || out != "OK: TOTAL coverage 78.6% >= 70% floor\n" || errOut != "" { + t.Errorf("passing gate: code=%d stdout=%q stderr=%q", code, out, errOut) + } + code, out, errOut = runMain(t, "", "coverage-gate", profile, "TOTAL=70", "cli=70") + want := "OK: TOTAL coverage 78.6% >= 70% floor\nFAIL: cli coverage 62.5% is below the 70% floor\n" + if code != 1 || out != want || errOut != "" { + t.Errorf("failing gate: code=%d stdout=%q stderr=%q, want 1 %q \"\"", code, out, errOut, want) + } + code, _, errOut = runMain(t, "", "coverage-gate", profile) + if code != 1 || errOut != "devtool coverage-gate: usage: coverage-gate TOTAL= [pkg=min ...]\n" { + t.Errorf("no floor: code=%d stderr=%q", code, errOut) + } +} + +func TestCoverageMerge(t *testing.T) { + // An edit that took the title for a directory would leave it in the package. + t.Chdir(t.TempDir()) + t.Run("lanes into the default directory", func(t *testing.T) { + root := t.TempDir() + writeFile(t, filepath.Join(root, "a", "covmeta"), "a") + writeFile(t, filepath.Join(root, "b", "covmeta"), "b") + writeFile(t, filepath.Join(root, "notes.txt"), "not a lane") + writeFile(t, filepath.Join(root, "merged", "stale"), "the last merge") + if err := os.Mkdir(filepath.Join(root, "empty"), 0o755); err != nil { + t.Fatal(err) + } + log := fakeGo(t) + + code, out, errOut := runMain(t, "", "coverage-merge", root+"/") + merged := filepath.Join(root, "merged") + lanes := filepath.Join(root, "a") + "," + filepath.Join(root, "b") + want := "### Coverage — merged (all lanes) (total 78.6%)\n\n" + sampleReport + if code != 0 || out != want || errOut != "merging coverage lanes: "+lanes+"\n" { + t.Errorf("code=%d stderr=%q\nstdout=%q\nwant %q", code, errOut, out, want) + } + wantCalls := "go tool covdata merge -i=" + lanes + " -o=" + merged + "\n" + + "go tool covdata textfmt -i=" + merged + " -o=" + merged + ".out\n" + if got := readFile(t, log); got != wantCalls { + t.Errorf("go calls = %q, want %q", got, wantCalls) + } + if _, err := os.Stat(filepath.Join(merged, "stale")); !os.IsNotExist(err) { + t.Errorf("the last merge's file survived: %v", err) + } + assertMode(t, merged, 0o755) + }) + + t.Run("into a directory and title of its own", func(t *testing.T) { + root, out := t.TempDir(), filepath.Join(t.TempDir(), "all") + writeFile(t, filepath.Join(root, "a", "covmeta"), "a") + log := fakeGo(t) + + code, stdout, _ := runMain(t, "", "coverage-merge", root, out, "lanes") + if want := "### Coverage — lanes (total 78.6%)\n\n" + sampleReport; code != 0 || stdout != want { + t.Errorf("code=%d stdout=%q, want %q", code, stdout, want) + } + if got := readFile(t, log); !strings.HasPrefix(got, "go tool covdata merge -i="+filepath.Join(root, "a")+" -o="+out+"\n") { + t.Errorf("go calls = %q", got) + } + }) + + t.Run("no lane but the output directory", func(t *testing.T) { + root := t.TempDir() + writeFile(t, filepath.Join(root, "out", "covmeta"), "the last merge") + code, out, errOut := runMain(t, "", "coverage-merge", root, filepath.Join(root, "out")) + if want := "### Coverage — merged (all lanes)\n\n_(no lane data to merge)_\n"; code != 0 || out != want || errOut != "" { + t.Errorf("code=%d stdout=%q stderr=%q, want %q", code, out, errOut, want) + } + }) + + if code, _, errOut := runMain(t, "", "coverage-merge"); code != 1 || errOut != "devtool coverage-merge: usage: coverage-merge [out-dir] [title]\n" { + t.Errorf("no root: code=%d stderr=%q", code, errOut) + } +} + +// runnerTools fakes what preparing a runner starts: sudo (tee echoes its stdin, +// as the real one does), docker and the shell printing the disk. +func runnerTools(t *testing.T) (log string) { + t.Helper() + return fakeTools(t, map[string]string{ + "sudo": `if [ "$1" = tee ]; then cat; echo tee-stderr >&2; fi`, + "docker": `echo "driver=[fake]"`, + "sh": `echo "/dev/fake 100G 10G 90G 10% /"`, + }) +} + +const daemonJSON = `{"features":{"containerd-snapshotter":true}}` + +const runnerTail = "sudo tee /etc/docker/daemon.json\n" + + "sudo systemctl restart docker\n" + + "docker info -f driver={{.DriverStatus}}\n" + +func TestPrepareRunner(t *testing.T) { + t.Run("short of room", func(t *testing.T) { + log := runnerTools(t) + var out, errOut strings.Builder + if err := (tool{out: &out, errw: &errOut}).prepareRunner(RunnerDiskRoom - 1); err != nil { + t.Fatal(err) + } + wantCalls := "sudo rm -rf /usr/share/dotnet /opt/ghc /usr/local/lib/android /opt/hostedtoolcache/CodeQL /usr/share/swift /usr/local/share/boost /usr/lib/jvm\n" + + "sudo docker image prune -af\n" + + "sh -c df -h / | tail -1\n" + runnerTail + if got := readFile(t, log); got != wantCalls { + t.Errorf("calls = %q\nwant %q", got, wantCalls) + } + want := "disk after:\n/dev/fake 100G 10G 90G 10% /\n" + daemonJSON + "driver=[fake]\n" + if out.String() != want || errOut.String() != "tee-stderr\n" { + t.Errorf("stdout=%q, want %q\nstderr=%q", out.String(), want, errOut.String()) + } + }) + + t.Run("with room", func(t *testing.T) { + log := runnerTools(t) + var out, errOut strings.Builder + if err := (tool{out: &out, errw: &errOut}).prepareRunner(RunnerDiskRoom); err != nil { + t.Fatal(err) + } + if got := readFile(t, log); got != runnerTail { + t.Errorf("calls = %q, want %q", got, runnerTail) + } + want := "60 GB free: the runner has room, nothing deleted\n" + daemonJSON + "driver=[fake]\n" + if out.String() != want { + t.Errorf("stdout=%q, want %q", out.String(), want) + } + }) + + t.Run("the command", func(t *testing.T) { + log := runnerTools(t) + code, out, _ := runMain(t, "", "ci-prepare-runner") + if code != 0 || !strings.HasPrefix(out, "disk before:\n/dev/fake 100G 10G 90G 10% /\n") { + t.Errorf("code=%d stdout=%q", code, out) + } + if got := readFile(t, log); !strings.HasPrefix(got, "sh -c df -h / | tail -1\n") || !strings.HasSuffix(got, runnerTail) { + t.Errorf("calls = %q", got) + } + }) +} + +func TestDockerCacheExport(t *testing.T) { + log := fakeTools(t, map[string]string{"docker": `cat "$6/Dockerfile" >> "$FAKE_LOG" +[ "$3" = "--cache-to=type=local,dest=$6/c" ] || exit 9 +echo noise; echo noise >&2 +exit "$FAKE_EXIT"`}) + t.Setenv("FAKE_LOG", filepath.Join(t.TempDir(), "dockerfile")) + + t.Setenv("FAKE_EXIT", "0") + if code, out, errOut := runMain(t, "", "docker-cache-export"); code != 0 || out != "" || errOut != "" { + t.Errorf("a builder that exports: code=%d stdout=%q stderr=%q, want 0 and silence", code, out, errOut) + } + t.Setenv("FAKE_EXIT", "1") + if code, out, errOut := runMain(t, "", "docker-cache-export"); code != 1 || out != "" || errOut != "" { + t.Errorf("a builder that cannot: code=%d stdout=%q stderr=%q, want 1 and silence", code, out, errOut) + } + call := readFile(t, log) + if !strings.HasPrefix(call, "docker buildx build --cache-to=type=local,dest=") || !strings.Contains(call, " -t devcontainer-cache-export-probe ") { + t.Errorf("docker call = %q", call) + } + if got := readFile(t, os.Getenv("FAKE_LOG")); got != "FROM busybox\nFROM busybox\n" { + t.Errorf("Dockerfiles built = %q", got) + } +} + +func TestReleaseResolve(t *testing.T) { + today := time.Now().UTC().Format("20060102") + gitRepo(t) + head := commit(t, "a.txt", 1_700_000_000) + git(t, "tag", "v"+today+".01") + git(t, "tag", "v"+today+".02") + for _, k := range []string{"GITHUB_EVENT_NAME", "VERSION_INPUT", "GITHUB_REF_NAME", "GITHUB_SHA", "GITHUB_OUTPUT", "GITHUB_STEP_SUMMARY"} { + t.Setenv(k, "") + } + + t.Run("outside CI: the day's next CalVer to stdout", func(t *testing.T) { + code, out, errOut := runMain(t, "", "release-resolve") + if now := time.Now().UTC().Format("20060102"); now != today { + t.Skip("the day changed during the test") + } + v := today + ".03" + want := "version=" + v + "\ntag=v" + v + "\nsha=" + head + "\nReleasing v" + v + " (version " + v + ") from " + head + "\n" + if code != 0 || out != want || errOut != "" { + t.Errorf("code=%d stderr=%q\nstdout=%q\nwant %q", code, errOut, out, want) + } + }) + + t.Run("dispatch: its input to the workflow's files", func(t *testing.T) { + dir := t.TempDir() + output, summary := filepath.Join(dir, "output"), filepath.Join(dir, "summary") + writeFile(t, output, "earlier=step\n") + t.Setenv("GITHUB_OUTPUT", output) + t.Setenv("GITHUB_STEP_SUMMARY", summary) + t.Setenv("GITHUB_EVENT_NAME", "workflow_dispatch") + t.Setenv("VERSION_INPUT", "v1.2.3") + if code, out, errOut := runMain(t, "", "release-resolve"); code != 0 || out != "" || errOut != "" { + t.Errorf("code=%d stdout=%q stderr=%q", code, out, errOut) + } + if got, want := readFile(t, output), "earlier=step\nversion=1.2.3\ntag=v1.2.3\nsha="+head+"\n"; got != want { + t.Errorf("GITHUB_OUTPUT = %q, want %q", got, want) + } + if got, want := readFile(t, summary), "Releasing v1.2.3 (version 1.2.3) from "+head+"\n"; got != want { + t.Errorf("GITHUB_STEP_SUMMARY = %q, want %q", got, want) + } + assertMode(t, summary, 0o644) + }) + + t.Run("dispatch of a released tag", func(t *testing.T) { + t.Setenv("VERSION_INPUT", today+".01") + code, out, errOut := runMain(t, "", "release-resolve") + want := "::error::tag already exists: v" + today + ".01 — releases are immutable, bump the build number\n" + if code != 1 || out != "" || errOut != want { + t.Errorf("code=%d stdout=%q stderr=%q, want 1 \"\" %q", code, out, errOut, want) + } + }) + + t.Run("tag push, with no repository to ask", func(t *testing.T) { + t.Chdir(t.TempDir()) + t.Setenv("GITHUB_EVENT_NAME", "push") + t.Setenv("GITHUB_REF_NAME", "v9.9.9") + t.Setenv("GITHUB_SHA", "pushed") + code, out, errOut := runMain(t, "", "release-resolve") + want := "version=9.9.9\ntag=v9.9.9\nsha=pushed\nReleasing v9.9.9 (version 9.9.9) from pushed\n" + if code != 0 || out != want || errOut != "" { + t.Errorf("code=%d stdout=%q stderr=%q, want %q", code, out, errOut, want) + } + }) +} + +// metricsTools fakes the binary measured, in its own directory, and node; each +// logs its calls, and the binary's read-configuration logs the config it reads. +func metricsTools(t *testing.T) (bin, log string) { + t.Helper() + log = fakeTools(t, map[string]string{"node": ""}) + bin = filepath.Join(t.TempDir(), "devcontainer") + writeFile(t, bin, "#!/bin/sh\necho \"bin $*\" >> '"+log+"'\n"+ + `[ "$1" = read-configuration ] && cat "$3/.devcontainer/devcontainer.json" >> '`+log+"'\nexit 0\n") + if err := os.Chmod(bin, 0o755); err != nil { + t.Fatal(err) + } + return bin, log +} + +func readMetrics(t *testing.T, path, stdout string) Metrics { + t.Helper() + data := readFile(t, path) + if !strings.HasSuffix(stdout, "metrics: wrote "+path+"\n"+data) { + t.Errorf("stdout = %q does not end with what it wrote, %q", stdout, data) + } + var m Metrics + if err := json.Unmarshal([]byte(data), &m); err != nil { + t.Fatal(err) + } + return m +} + +func TestMetrics(t *testing.T) { + // An edit that swapped its arguments would write the version as a file here. + t.Chdir(t.TempDir()) + t.Run("everything measured", func(t *testing.T) { + bin, log := metricsTools(t) + writeFile(t, filepath.Join(filepath.Dir(bin), "dist", "devcontainer-linux-amd64"), strings.Repeat("a", 1000)) + writeFile(t, filepath.Join(filepath.Dir(bin), "dist", "devcontainer-linux-arm64"), strings.Repeat("ab", 1500)) + oracle := filepath.Join(t.TempDir(), "oracle.js") + writeFile(t, oracle, "") + out := filepath.Join(t.TempDir(), "artifacts", "metrics.json") + t.Setenv("METRICS_RUNS", "2") + before := time.Now().UTC().Truncate(time.Second) + + code, stdout, errOut := runMain(t, "", "metrics", "v1", bin, oracle, out) + if code != 0 || errOut != "" { + t.Fatalf("code=%d stderr=%q", code, errOut) + } + if !strings.HasPrefix(stdout, "metrics: timing tool = go, runs=2\nmetrics: wrote ") { + t.Errorf("stdout = %q", stdout) + } + m := readMetrics(t, out, stdout) + at, err := time.Parse(time.RFC3339, m.GeneratedAt) + if err != nil || at.Before(before) || at.After(time.Now()) { + t.Errorf("generated_at = %q (%v), want the time of the run", m.GeneratedAt, err) + } + if m.Version != "v1" || m.TimingTool != "go" || m.Runs != 2 { + t.Errorf("version=%q timing_tool=%q runs=%d", m.Version, m.TimingTool, m.Runs) + } + if m.StartupMs.GoVersion == nil || m.StartupMs.GoReadConfiguration == nil || m.StartupMs.NodeVersion == nil { + t.Errorf("startup_ms = %+v, want every command timed", m.StartupMs) + } + s := m.SizesBytes + dist := filepath.Join(filepath.Dir(bin), "dist", "devcontainer") + for name, got := range map[string]*int64{ + "local": s.LocalBinary, "amd64": s.LinuxAmd64Binary, "amd64 gzip": s.LinuxAmd64Gzip, + "arm64": s.LinuxArm64Binary, "arm64 gzip": s.LinuxArm64Gzip, + } { + want := map[string]*int64{ + "local": FileSize(bin), "amd64": new(int64(1000)), "amd64 gzip": GzipSize(dist + "-linux-amd64"), + "arm64": new(int64(3000)), "arm64 gzip": GzipSize(dist + "-linux-arm64"), + }[name] + if got == nil || *got != *want { + t.Errorf("%s size = %v, want %d", name, got, *want) + } + } + assertMode(t, out, 0o644) + assertMode(t, filepath.Dir(out), 0o755) + + calls := readFile(t, log) + config := `{"image":"mcr.microsoft.com/devcontainers/base:ubuntu"}` + "\n" + if n := strings.Count(calls, "bin --version\n"); n != 3 { + t.Errorf("--version ran %d times, want a warm-up and 2 runs", n) + } + if n := strings.Count(calls, config); n != 3 || strings.Count(calls, "bin read-configuration --workspace-folder ") != 3 { + t.Errorf("read-configuration ran %d times with the config, want 3:\n%s", n, calls) + } + if n := strings.Count(calls, "node "+oracle+" --version\n"); n != 3 { + t.Errorf("the oracle ran %d times, want 3", n) + } + }) + + t.Run("by default, without oracle or release binaries", func(t *testing.T) { + bin, log := metricsTools(t) + oracle := filepath.Join(t.TempDir(), "absent.js") + out := filepath.Join(t.TempDir(), "metrics.json") + t.Setenv("METRICS_RUNS", "") + + code, stdout, _ := runMain(t, "", "metrics", "dev", bin, oracle, out) + want := "metrics: timing tool = go, runs=30\n" + + "metrics: node oracle not found at " + oracle + " (run 'task reference') — skipping Node A/B\n" + + "metrics: dist binaries absent — run 'task build:cross' for release sizes\n" + if code != 0 || !strings.HasPrefix(stdout, want) { + t.Errorf("code=%d stdout=%q, want it to start with %q", code, stdout, want) + } + m := readMetrics(t, out, stdout) + if m.Runs != 30 || m.StartupMs.NodeVersion != nil || m.SizesBytes.LocalBinary == nil || m.SizesBytes.LinuxAmd64Binary != nil { + t.Errorf("runs=%d startup=%+v sizes=%+v", m.Runs, m.StartupMs, m.SizesBytes) + } + if n := strings.Count(readFile(t, log), "bin --version\n"); n != 31 { + t.Errorf("--version ran %d times, want 31", n) + } + }) + + if code, _, errOut := runMain(t, "", "metrics", "v1"); code != 1 || errOut != "devtool metrics: usage: devtool metrics \n" { + t.Errorf("one argument: code=%d stderr=%q", code, errOut) + } +} From 946343dec6b7ae8c77edf19fb164ffb0ef155432 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Sun, 25 Oct 2026 17:59:16 -0300 Subject: [PATCH 7/7] test(devtool): hold the coverage, parity and runner edits go tool mutate left six edits no test refused. Pct of a package with no statements is now tested (0, not NaN), and so is a cli file that is not a test and a test outside internal/cli, which both run the full parity matrix. ParseFloat's bitSize of 64 is exempt: any value but 32 means 64. The exec helper process now gets a GOCOVERDIR, since a binary built with coverage, as mutate builds it, warned on the helper's stderr and failed TestOSRunnerProcessContract before any edit; that hid the nil-stream edit in OSRunner.Run. A test with a single Env entry holds the len > 0 guard that the extra entry would otherwise leave untested. Co-Authored-By: Claude Opus 5.5 --- internal/devtools/coverage.go | 5 ++-- internal/devtools/coverage_test.go | 11 +++++++++ internal/devtools/parityaffected_test.go | 2 ++ internal/exec/exec_test.go | 29 +++++++++++++++++++----- 4 files changed, 39 insertions(+), 8 deletions(-) diff --git a/internal/devtools/coverage.go b/internal/devtools/coverage.go index ead2bb0..6172995 100644 --- a/internal/devtools/coverage.go +++ b/internal/devtools/coverage.go @@ -29,7 +29,8 @@ func (p PkgCov) Pct() float64 { } // Round to 1 decimal exactly as fmt "%.1f" would, so String() and numeric // comparisons never disagree at a boundary. - v, _ := strconv.ParseFloat(fmt.Sprintf("%.1f", float64(p.Covered)*100/float64(p.Total)), 64) + rounded := fmt.Sprintf("%.1f", float64(p.Covered)*100/float64(p.Total)) + v, _ := strconv.ParseFloat(rounded, 64) // mutate-exempt: ParseFloat reads any bitSize but 32 as 64 return v } @@ -156,7 +157,7 @@ func ParseFloors(specs []string) ([]Floor, error) { if !found || k == "" { return nil, fmt.Errorf("invalid floor %q: want =", s) } - min, err := strconv.ParseFloat(v, 64) + min, err := strconv.ParseFloat(v, 64) // mutate-exempt: ParseFloat reads any bitSize but 32 as 64 if err != nil { return nil, fmt.Errorf("invalid floor %q: %w", s, err) } diff --git a/internal/devtools/coverage_test.go b/internal/devtools/coverage_test.go index bdc477b..05965b8 100644 --- a/internal/devtools/coverage_test.go +++ b/internal/devtools/coverage_test.go @@ -117,3 +117,14 @@ func TestParseFloorsInvalid(t *testing.T) { } } } + +// TestPctOfAnEmptyPackage holds the guard a package without statements needs: its +// coverage is 0, not the NaN of 0/0 that the report and the gate would print. +func TestPctOfAnEmptyPackage(t *testing.T) { + if got := (PkgCov{Pkg: "p"}).Pct(); got != 0 { + t.Errorf("Pct of 0 statements = %v, want 0", got) + } + if got := (PkgCov{Pkg: "p", Covered: 1, Total: 1}).Pct(); got != 100 { + t.Errorf("Pct of 1/1 statements = %v, want 100", got) + } +} diff --git a/internal/devtools/parityaffected_test.go b/internal/devtools/parityaffected_test.go index a49ae8c..f7627b4 100644 --- a/internal/devtools/parityaffected_test.go +++ b/internal/devtools/parityaffected_test.go @@ -38,6 +38,8 @@ func TestParityAffected(t *testing.T) { {"Taskfile → all", []string{"Taskfile.yml"}, "all"}, {"go-cli workflow → all", []string{".github/workflows/go-cli.yml"}, "all"}, {"shared internal → all", []string{"internal/foo.go"}, "all"}, + {"shared cli file → all", []string{"internal/cli/shared.go"}, "all"}, + {"test outside cli → all", []string{"internal/docker/client_test.go"}, "all"}, {"cmd → all", []string{"cmd/devcontainer/main.go"}, "all"}, {"unknown path → all", []string{"totally-unknown-file"}, "all"}, {"full wins over leaf", []string{"internal/cli/exec.go", "docs/parity/parity-matrix.yaml"}, "all"}, diff --git a/internal/exec/exec_test.go b/internal/exec/exec_test.go index 26315e9..2391cdb 100644 --- a/internal/exec/exec_test.go +++ b/internal/exec/exec_test.go @@ -44,14 +44,17 @@ func helperArgs(args []string) []string { return nil } -func helperRunner(env ...string) OSRunner { +// helperRunner runs this test binary as the helper process. GOCOVERDIR keeps a +// binary built with coverage (as go tool mutate builds it) from warning on the +// helper's stderr that it has nowhere to write its counters. +func helperRunner(t *testing.T, env ...string) OSRunner { return OSRunner{Env: append([]string{ - "GO_WANT_EXEC_HELPER=1", + "GO_WANT_EXEC_HELPER=1", "GOCOVERDIR=" + t.TempDir(), }, env...)} } func TestOSRunnerProcessContract(t *testing.T) { - stdout, stderr, code, err := helperRunner("EXEC_HELPER_STDERR=warning").Run( + stdout, stderr, code, err := helperRunner(t, "EXEC_HELPER_STDERR=warning").Run( t.Context(), nil, os.Args[0], "-test.run=^TestHelperProcess$", "--", "echo", "one", "two", ) if err != nil { @@ -69,7 +72,7 @@ func TestOSRunnerProcessContract(t *testing.T) { } func TestOSRunnerNonZeroIsExitCodeNotTransportError(t *testing.T) { - _, _, code, err := helperRunner().Run( + _, _, code, err := helperRunner(t).Run( t.Context(), nil, os.Args[0], "-test.run=^TestHelperProcess$", "--", "exit", ) if err != nil { @@ -83,7 +86,7 @@ func TestOSRunnerNonZeroIsExitCodeNotTransportError(t *testing.T) { func TestOSRunnerHonorsCancellation(t *testing.T) { ctx, cancel := context.WithTimeout(t.Context(), 100*time.Millisecond) defer cancel() - _, _, code, err := helperRunner().Run( + _, _, code, err := helperRunner(t).Run( ctx, nil, os.Args[0], "-test.run=^TestHelperProcess$", "--", "wait", ) if !errors.Is(err, context.DeadlineExceeded) { @@ -108,7 +111,7 @@ func TestOSRunnerMissingBinaryIsTransportError(t *testing.T) { // output live while the return value still captures it. func TestOSRunnerStreamsToWriter(t *testing.T) { var live strings.Builder - stdout, _, code, err := helperRunner("EXEC_HELPER_STDERR=warn").Run( + stdout, _, code, err := helperRunner(t, "EXEC_HELPER_STDERR=warn").Run( t.Context(), &live, os.Args[0], "-test.run=^TestHelperProcess$", "--", "echo", "hi", ) if err != nil || code != 0 { @@ -123,3 +126,17 @@ func TestOSRunnerStreamsToWriter(t *testing.T) { t.Errorf("captured stdout = %q, want hi", got) } } + +// TestOSRunnerAppliesASingleEnvEntry holds that one entry is enough for Env to +// reach the child: without it the helper below would not act as the helper. +func TestOSRunnerAppliesASingleEnvEntry(t *testing.T) { + stdout, _, code, err := OSRunner{Env: []string{"GO_WANT_EXEC_HELPER=1"}}.Run( + t.Context(), nil, os.Args[0], "-test.run=^TestHelperProcess$", "--", "echo", "alone", + ) + if err != nil || code != 0 { + t.Fatalf("Run: err=%v code=%d", err, code) + } + if got := string(stdout); got != "alone" { + t.Errorf("stdout = %q, want alone", got) + } +}