Skip to content

Commit eae4177

Browse files
aledbfclaude
andcommitted
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 `<self> 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 <noreply@anthropic.com>
1 parent b37c98c commit eae4177

6 files changed

Lines changed: 575 additions & 6 deletions

‎internal/cli/features_test_runner.go‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -483,16 +483,27 @@ func makeWorkspaceAccessible(dir string) {
483483
})
484484
}
485485

486+
// execTestInContainer runs a test script in the test container with this
487+
// binary's `exec`, and reports whether it passed.
488+
//
489+
// mutate-lane: TestParityMatrix (features.test-single-scenario-success passes
490+
// only when the scenario script's success reaches the exit code through it).
486491
func execTestInContainer(containerId, testScript, workspaceFolder string) bool {
487492
self, _ := os.Executable()
493+
return testScriptCommand(self, containerId, testScript, workspaceFolder).Run() == nil
494+
}
495+
496+
// testScriptCommand is the `<self> exec` that sources the test library and a
497+
// test script in the container, with the script's output on this process's.
498+
func testScriptCommand(self, containerId, testScript, workspaceFolder string) *exec.Cmd {
488499
// The workspace is mounted at /workspaces/<name>; the test lib and scripts live
489500
// there, so cd into it (not its parent) before sourcing them.
490501
containerWs := "/workspaces/" + filepath.Base(workspaceFolder)
491502
cmd := exec.Command(self, "exec", "--container-id", containerId, "--", "bash", "-c",
492503
fmt.Sprintf("cd %s && source dev-container-features-test-lib && source %s && reportResults", containerWs, testScript))
493504
cmd.Stdout = os.Stdout
494505
cmd.Stderr = os.Stderr
495-
return cmd.Run() == nil
506+
return cmd
496507
}
497508

498509
func cleanupTestContainers(logger log.Logger) {
Lines changed: 219 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,219 @@
1+
package cli
2+
3+
import (
4+
"bytes"
5+
"os"
6+
"path/filepath"
7+
"slices"
8+
"strings"
9+
"testing"
10+
11+
"github.com/devcontainers/cli/internal/log"
12+
)
13+
14+
// fakeDockerOnPath puts a `docker` first on PATH whose `ps` prints ids and
15+
// whose `rm -f <id>` appends the id to the returned file, the way the runner's
16+
// cleanup calls the real one.
17+
func fakeDockerOnPath(t *testing.T, ids string) (psArgs, removed string) {
18+
t.Helper()
19+
dir := t.TempDir()
20+
psArgs, removed = filepath.Join(dir, "ps.args"), filepath.Join(dir, "removed")
21+
script := "#!/bin/sh\ncase \"$1\" in\n" +
22+
"ps) echo \"$@\" > '" + psArgs + "'; printf '" + ids + "' ;;\n" +
23+
"rm) echo \"$3\" >> '" + removed + "' ;;\n" +
24+
"esac\n"
25+
writeFile(t, filepath.Join(dir, "docker"), script, 0o755)
26+
t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH"))
27+
return psArgs, removed
28+
}
29+
30+
func readOrEmpty(t *testing.T, path string) string {
31+
t.Helper()
32+
data, err := os.ReadFile(path)
33+
if err != nil && !os.IsNotExist(err) {
34+
t.Fatal(err)
35+
}
36+
return string(data)
37+
}
38+
39+
func bufferLogger(buf *bytes.Buffer) log.Logger {
40+
return log.New(log.Options{Level: log.LevelInfo, Format: "text", Writer: buf})
41+
}
42+
43+
// Cleanup removes every container the test run labelled, and says how many.
44+
func TestCleanupTestContainersRemovesLabelledContainers(t *testing.T) {
45+
psArgs, removed := fakeDockerOnPath(t, `c1\n`)
46+
var logs bytes.Buffer
47+
cleanupTestContainers(bufferLogger(&logs))
48+
if got := readOrEmpty(t, psArgs); got != "ps -a --filter label=devcontainer.is_test_run=true --format {{.ID}}\n" {
49+
t.Errorf("docker ps args = %q", got)
50+
}
51+
if got := readOrEmpty(t, removed); got != "c1\n" {
52+
t.Errorf("removed = %q, want c1", got)
53+
}
54+
if !strings.Contains(logs.String(), "Cleaning up 1 test containers...") {
55+
t.Errorf("logs = %q", logs.String())
56+
}
57+
}
58+
59+
// With no labelled containers there is nothing to remove or announce.
60+
func TestCleanupTestContainersWithNone(t *testing.T) {
61+
_, removed := fakeDockerOnPath(t, ``)
62+
var logs bytes.Buffer
63+
cleanupTestContainers(bufferLogger(&logs))
64+
if got := readOrEmpty(t, removed); got != "" {
65+
t.Errorf("removed = %q, want nothing", got)
66+
}
67+
if strings.Contains(logs.String(), "Cleaning up") {
68+
t.Errorf("logs = %q, want no cleanup line", logs.String())
69+
}
70+
}
71+
72+
// The workspace a test script enters is opened to every user, files and
73+
// folders alike, so a non-root remote user can read it.
74+
func TestMakeWorkspaceAccessible(t *testing.T) {
75+
dir := t.TempDir()
76+
sub := filepath.Join(dir, "feature")
77+
writeFile(t, filepath.Join(sub, "test.sh"), "true\n", 0o600)
78+
if err := os.Chmod(sub, 0o700); err != nil {
79+
t.Fatal(err)
80+
}
81+
makeWorkspaceAccessible(dir)
82+
for _, p := range []string{dir, sub, filepath.Join(sub, "test.sh")} {
83+
info, err := os.Stat(p)
84+
if err != nil {
85+
t.Fatal(err)
86+
}
87+
if info.Mode().Perm() != 0o777 {
88+
t.Errorf("%s mode = %v, want 0777", p, info.Mode().Perm())
89+
}
90+
}
91+
}
92+
93+
// A test script runs through `<self> exec` in the test container, from the
94+
// mounted workspace, with its output on this process's stdout and stderr.
95+
func TestTestScriptCommand(t *testing.T) {
96+
cmd := testScriptCommand("/bin/devcontainer", "cid", "color/test.sh", "/tmp/devcontainer-test-1")
97+
want := []string{"/bin/devcontainer", "exec", "--container-id", "cid", "--", "bash", "-c",
98+
"cd /workspaces/devcontainer-test-1 && source dev-container-features-test-lib && source color/test.sh && reportResults"}
99+
if !slices.Equal(cmd.Args, want) {
100+
t.Errorf("args = %q, want %q", cmd.Args, want)
101+
}
102+
if cmd.Stdout != os.Stdout || cmd.Stderr != os.Stderr {
103+
t.Error("the script's output does not reach this process's stdout and stderr")
104+
}
105+
}
106+
107+
// testCollection writes a collection with Features a and b (each with a
108+
// test.sh), a _global scenario g, and a stray file in test/.
109+
func testCollection(t *testing.T) string {
110+
t.Helper()
111+
base := t.TempDir()
112+
for _, f := range []string{"a", "b"} {
113+
writeFile(t, filepath.Join(base, "src", f, "install.sh"), "#!/bin/sh\n", 0o755)
114+
writeFile(t, filepath.Join(base, "test", f, "test.sh"), "true\n", 0o755)
115+
}
116+
writeFile(t, filepath.Join(base, "test", "README.md"), "notes\n", 0o644)
117+
writeFile(t, filepath.Join(base, "test", "_global", "scenarios.json"), `{"g":{"image":"alpine"}}`, 0o644)
118+
writeFile(t, filepath.Join(base, "test", "_global", "g.sh"), "true\n", 0o755)
119+
return base
120+
}
121+
122+
func runTestCommand(t *testing.T, collection string, featuresList []string) (code int, stdout, logs string) {
123+
t.Helper()
124+
out := &captureOutput{}
125+
var buf bytes.Buffer
126+
code = runFeaturesTestCommand(out, bufferLogger(&buf), collection, featuresList, "",
127+
false, false, false, false, "alpine", "", false, true, false, nil)
128+
return code, out.out.String(), buf.String()
129+
}
130+
131+
// reportedNames lists the quoted names of the report's result lines, in order.
132+
func reportedNames(stdout string) []string {
133+
var names []string
134+
for _, line := range strings.Split(stdout, "\n") {
135+
if _, rest, ok := strings.Cut(line, "'"); ok && strings.Contains(line, ":") && !strings.Contains(line, "Folder") {
136+
name, _, _ := strings.Cut(rest, "'")
137+
names = append(names, name)
138+
}
139+
}
140+
return names
141+
}
142+
143+
// Without --features every Feature folder under test/ is tested (not _global,
144+
// not a stray file), then the global scenarios; containers are cleaned up.
145+
func TestRunFeaturesTestCommandDiscoversFeatures(t *testing.T) {
146+
_, removed := fakeDockerOnPath(t, `leftover\n`)
147+
fakeTestContainers(t, nil, nil, func(string, string) bool { return true })
148+
code, stdout, _ := runTestCommand(t, testCollection(t), nil)
149+
if code != 0 {
150+
t.Errorf("exit = %d, want 0\n%s", code, stdout)
151+
}
152+
if got, want := reportedNames(stdout), []string{"a", "b", "g"}; !slices.Equal(got, want) {
153+
t.Errorf("reported %q, want %q\n%s", got, want, stdout)
154+
}
155+
if got := readOrEmpty(t, removed); got != "leftover\n" {
156+
t.Errorf("removed = %q, want the labelled container", got)
157+
}
158+
}
159+
160+
// --features tests exactly the named Features, and skips the global scenarios.
161+
func TestRunFeaturesTestCommandNamedFeatures(t *testing.T) {
162+
fakeDockerOnPath(t, ``)
163+
fakeTestContainers(t, nil, nil, func(string, string) bool { return true })
164+
code, stdout, _ := runTestCommand(t, testCollection(t), []string{"a"})
165+
if code != 0 {
166+
t.Errorf("exit = %d, want 0\n%s", code, stdout)
167+
}
168+
if got, want := reportedNames(stdout), []string{"a"}; !slices.Equal(got, want) {
169+
t.Errorf("reported %q, want %q\n%s", got, want, stdout)
170+
}
171+
}
172+
173+
// A collection without both src/ and test/ fails with exit 1 and TS's message
174+
// on stdout, whichever of the two is missing.
175+
func TestRunFeaturesTestCommandRequiresSrcAndTest(t *testing.T) {
176+
for _, present := range []string{"src", "test"} {
177+
base := t.TempDir()
178+
if err := os.MkdirAll(filepath.Join(base, present), 0o755); err != nil {
179+
t.Fatal(err)
180+
}
181+
code, stdout, _ := runTestCommand(t, base, nil)
182+
want := "[-] Folder '" + base + "' does not contain the required 'src' and 'test' folders.\n"
183+
if code != 1 || !strings.HasSuffix(stdout, want) {
184+
t.Errorf("only %s/: exit = %d, stdout = %q", present, code, stdout)
185+
}
186+
}
187+
}
188+
189+
// With no Feature folder to test, the command fails with exit 1.
190+
func TestRunFeaturesTestCommandWithoutFeatures(t *testing.T) {
191+
base := t.TempDir()
192+
for _, d := range []string{"src", filepath.Join("test", "_global")} {
193+
if err := os.MkdirAll(filepath.Join(base, d), 0o755); err != nil {
194+
t.Fatal(err)
195+
}
196+
}
197+
fakeTestContainers(t, nil, nil, func(string, string) bool { return true })
198+
code, stdout, logs := runTestCommand(t, base, nil)
199+
if code != 1 || !strings.Contains(logs, "No features to test.") || strings.Contains(stdout, "TEST REPORT") {
200+
t.Errorf("exit = %d, logs = %q, stdout = %q", code, logs, stdout)
201+
}
202+
}
203+
204+
// A test/ folder that cannot be listed fails the command with exit 1.
205+
func TestRunFeaturesTestCommandUnreadableTestFolder(t *testing.T) {
206+
if os.Geteuid() == 0 {
207+
t.Skip("root lists any folder")
208+
}
209+
base := testCollection(t)
210+
testDir := filepath.Join(base, "test")
211+
if err := os.Chmod(testDir, 0o000); err != nil {
212+
t.Fatal(err)
213+
}
214+
t.Cleanup(func() { _ = os.Chmod(testDir, 0o755) })
215+
code, _, logs := runTestCommand(t, base, nil)
216+
if code != 1 || !strings.Contains(logs, "Failed to discover feature tests") {
217+
t.Errorf("exit = %d, logs = %q", code, logs)
218+
}
219+
}

‎internal/cli/outdated.go‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"encoding/json"
77
"fmt"
88
"os"
9-
"path/filepath"
109
"regexp"
1110
"slices"
1211
"sort"
@@ -437,11 +436,9 @@ func newUpgradeCmd() *cobra.Command {
437436

438437
cfg := loadResult.Config
439438

440-
// Read existing lockfile
439+
// The lockfile sits next to the configuration, whose path a loaded
440+
// configuration always carries.
441441
lockfilePath := cfg.ConfigFilePath
442-
if lockfilePath == "" {
443-
lockfilePath = filepath.Join(ws, "devcontainer.json")
444-
}
445442

446443
// Generate new lockfile from current features config
447444
// This is a simplified version — the full implementation

0 commit comments

Comments
 (0)