Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/features/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ func NewFeaturesCmd(globalFlags *flags.GlobalFlags) *cobra.Command {
featuresCmd.AddCommand(NewInfoCmd(globalFlags))
featuresCmd.AddCommand(NewResolveDepsCmd(globalFlags))
featuresCmd.AddCommand(NewGenerateDocsCmd(globalFlags))
featuresCmd.AddCommand(NewTestCmd(globalFlags))

return featuresCmd
}
372 changes: 372 additions & 0 deletions cmd/features/test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,372 @@
package features

import (
"encoding/json"
"fmt"
"os"
"os/exec"
"path/filepath"
"strings"

"github.com/devsy-org/devsy/cmd/flags"
"github.com/devsy-org/devsy/pkg/devcontainer/config"
"github.com/spf13/cobra"
)

const (
defaultBaseImage = "mcr.microsoft.com/devcontainers/base:ubuntu"
defaultRemoteUser = "root"
)

type TestCmd struct {
*flags.GlobalFlags

ProjectFolder string
Features string
BaseImage string
RemoteUser string
SkipScenarios bool
Quiet bool
PreserveTestContainers bool
}

type testResult struct {
FeatureID string `json:"featureId"`
Scenario string `json:"scenario,omitempty"`
Passed bool `json:"passed"`
Error string `json:"error,omitempty"`
}

func NewTestCmd(globalFlags *flags.GlobalFlags) *cobra.Command {
cmd := &TestCmd{GlobalFlags: globalFlags}
testCmd := &cobra.Command{
Use: "test",
Short: "Test dev container features in isolation",
Long: `Run lifecycle hook tests for dev container features.

Scans the project's src/ directory for features, builds test containers
from a base image, installs each feature, and runs the corresponding
test scripts from the test/ directory.`,
SilenceUsage: true,
SilenceErrors: true,
RunE: func(_ *cobra.Command, _ []string) error {
return cmd.Run()
},
}

testCmd.Flags().StringVar(
&cmd.ProjectFolder, "project-folder", "",
"Path to feature project containing src/ and test/ directories",
)
testCmd.Flags().StringVar(
&cmd.Features, "features", "",
"Comma-separated list of feature IDs to test (default: all)",
)
testCmd.Flags().StringVar(
&cmd.BaseImage, "base-image", defaultBaseImage,
"Base Docker image for test containers",
)
testCmd.Flags().StringVar(
&cmd.RemoteUser, "remote-user", defaultRemoteUser,
"User to run tests as",
)
testCmd.Flags().BoolVar(
&cmd.SkipScenarios, "skip-scenarios", false,
"Only run the global test script, skip per-feature scenario tests",
)
testCmd.Flags().BoolVar(
&cmd.Quiet, "quiet", false,
"Suppress verbose build output",
)
testCmd.Flags().BoolVar(
&cmd.PreserveTestContainers, "preserve-test-containers", false,
"Don't remove test containers after run (for debugging)",
)
_ = testCmd.MarkFlagRequired("project-folder")

return testCmd
}

func (cmd *TestCmd) Run() error {
projectFolder, err := filepath.Abs(cmd.ProjectFolder)
if err != nil {
return fmt.Errorf("resolve project folder: %w", err)
}

srcDir := filepath.Join(projectFolder, "src")
testDir := filepath.Join(projectFolder, "test")

features, err := cmd.discoverFeatures(srcDir)
if err != nil {
return err
}

features = cmd.filterFeatures(features)
if len(features) == 0 {
return fmt.Errorf("no features matched the filter %q", cmd.Features)
}

var results []testResult
for _, feat := range features {
featureResults := cmd.testFeature(feat, projectFolder, testDir)
results = append(results, featureResults...)
}

cmd.printResults(results)

for _, r := range results {
if !r.Passed {
return fmt.Errorf("one or more feature tests failed")
}
}
return nil
}

type featureEntry struct {
id string
config *config.FeatureConfig
}

func (cmd *TestCmd) discoverFeatures(srcDir string) ([]featureEntry, error) {
docs, err := scanFeatures(srcDir)
if err != nil {
return nil, err
}

features := make([]featureEntry, 0, len(docs))
for _, doc := range docs {
features = append(features, featureEntry{
id: doc.dir,
config: doc.config,
})
}
return features, nil
}

func (cmd *TestCmd) filterFeatures(features []featureEntry) []featureEntry {
if cmd.Features == "" {
return features
}

filter := make(map[string]bool)
for f := range strings.SplitSeq(cmd.Features, ",") {
filter[strings.TrimSpace(f)] = true
}

var filtered []featureEntry
for _, feat := range features {
if filter[feat.id] {
filtered = append(filtered, feat)
}
}
return filtered
}

type testCase struct {
script string
scenario string
options map[string]string
}

func (cmd *TestCmd) testFeature(
feat featureEntry, projectFolder, testDir string,
) []testResult {
var results []testResult

featureTestDir := filepath.Join(testDir, feat.id)
globalTestScript := filepath.Join(featureTestDir, "test.sh")

if _, err := os.Stat(globalTestScript); err == nil {
tc := testCase{script: globalTestScript}
results = append(results, cmd.runTest(feat, projectFolder, tc))
}

if cmd.SkipScenarios {
return results
}

scenariosDir := filepath.Join(featureTestDir, "scenarios")
scenarios, err := os.ReadDir(scenariosDir)
if err != nil {
return results
}

for _, scenario := range scenarios {
if !scenario.IsDir() {
continue
}

scenarioDir := filepath.Join(scenariosDir, scenario.Name())
scenarioTestScript := filepath.Join(scenarioDir, "test.sh")

if _, err := os.Stat(scenarioTestScript); err != nil {
continue
}

tc := testCase{
script: scenarioTestScript,
scenario: scenario.Name(),
options: cmd.loadScenarioOptions(scenarioDir),
}
results = append(results, cmd.runTest(feat, projectFolder, tc))
}

return results
}

func (cmd *TestCmd) loadScenarioOptions(scenarioDir string) map[string]string {
scenarioJSON := filepath.Join(scenarioDir, "scenario.json")
data, err := os.ReadFile(scenarioJSON) // #nosec G304 -- path from project structure
if err != nil {
return nil
}

var scenario struct {
Options map[string]string `json:"options"`
}
if err := json.Unmarshal(data, &scenario); err != nil {
return nil
}
return scenario.Options
}

func (cmd *TestCmd) runTest(
feat featureEntry, projectFolder string, tc testCase,
) testResult {
result := testResult{
FeatureID: feat.id,
Scenario: tc.scenario,
}

testScriptRel, relErr := filepath.Rel(projectFolder, tc.script)
if relErr != nil {
result.Passed = false
result.Error = relErr.Error()
return result
}

dockerfile := cmd.generateDockerfileWithTest(feat, tc.options, testScriptRel)

containerName := fmt.Sprintf("devsy-test-%s", feat.id)
if tc.scenario != "" {
containerName = fmt.Sprintf("devsy-test-%s-%s", feat.id, tc.scenario)
}
Comment on lines +250 to +253

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Pre-existing container with the same name will silently fail docker run

Container names are deterministic (devsy-test-<featureId>[-<scenario>]). If a container with that name already exists — from a previous --preserve-test-containers run or an aborted execution — docker run --name X fails with "container name already in use", producing a confusing error in the results table.

Add a pre-flight removal before docker run:

🐛 Proposed fix
 func (cmd *TestCmd) dockerRun(imageName, containerName string) error {
+	// Remove any stale container with the same name before creating a new one.
+	_ = exec.Command("docker", "rm", "-f", containerName).Run() // `#nosec` G204
+
 	args := []string{"run", "--name", containerName}

Also applies to: 325-330

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/features/test.go` around lines 250 - 253, The deterministic containerName
(built from feat.id and tc.scenario) can collide with an existing container and
cause `docker run --name` to fail silently; before invoking the docker run that
uses containerName, add a pre-flight removal step that attempts to stop/remove
any existing container with that name (e.g., run a safe `docker rm -f
<containerName>`/equivalent via the existing shell/exec helper and ignore “not
found” errors but surface other failures), and apply the same pre-flight removal
in the other location that builds containerName (the second block around lines
325–330) so both places remove any stale container before running a new one.


imageName := containerName + ":latest"

buildErr := cmd.dockerBuild(dockerfile, projectFolder, imageName)
if buildErr != nil {
result.Passed = false
result.Error = buildErr.Error()
return result
}

runErr := cmd.dockerRun(imageName, containerName)
if runErr != nil {
result.Passed = false
result.Error = runErr.Error()
} else {
result.Passed = true
}

if !cmd.PreserveTestContainers {
cmd.dockerRemove(containerName)
}

return result
}

func (cmd *TestCmd) generateDockerfileWithTest(
feat featureEntry, options map[string]string, testScriptRelPath string,
) string {
var b strings.Builder

fmt.Fprintf(&b, "FROM %s\n", cmd.BaseImage)

featureSrcDir := filepath.Join("src", feat.id)
fmt.Fprintf(&b, "COPY %s /tmp/build-features/%s\n", featureSrcDir, feat.id)
Comment on lines +286 to +287

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Dockerfile COPY paths use OS-specific separators

filepath.Join and filepath.Rel produce backslash-delimited paths on Windows. When embedded verbatim in the Dockerfile COPY instruction, Docker rejects them. Use filepath.ToSlash to normalize both paths.

🐛 Proposed fix
-	featureSrcDir := filepath.Join("src", feat.id)
+	featureSrcDir := filepath.ToSlash(filepath.Join("src", feat.id))
 	fmt.Fprintf(&b, "COPY %s /tmp/build-features/%s\n", featureSrcDir, feat.id)
 	if testScriptRelPath != "" {
-		fmt.Fprintf(&b, "COPY %s /tmp/test.sh\n", testScriptRelPath)
+		fmt.Fprintf(&b, "COPY %s /tmp/test.sh\n", filepath.ToSlash(testScriptRelPath))

Also applies to: 301-301

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/features/test.go` around lines 286 - 287, The COPY path uses OS-specific
separators (featureSrcDir from filepath.Join) which can produce backslashes on
Windows and break Docker; update the code around featureSrcDir and any other
places using filepath.Join or filepath.Rel (e.g., the second occurrence around
line ~301) to call filepath.ToSlash on the result before embedding in the
Dockerfile COPY (e.g., change featureSrcDir := filepath.Join("src", feat.id) to
featureSrcDir := filepath.ToSlash(filepath.Join("src", feat.id)) and likewise
wrap any Rel outputs used in fmt.Fprintf, so the strings passed into
fmt.Fprintf(&b, "COPY %s /tmp/build-features/%s\n", ...) are normalized to
forward slashes).


for k, v := range options {
envKey := strings.ReplaceAll(strings.ToUpper(feat.id), "-", "_") + "_" + strings.ToUpper(k)
fmt.Fprintf(&b, "ENV %s=%q\n", envKey, v)
}

fmt.Fprintf(
&b,
"RUN chmod +x /tmp/build-features/%s/install.sh && /tmp/build-features/%s/install.sh\n",
feat.id, feat.id,
)

if testScriptRelPath != "" {
fmt.Fprintf(&b, "COPY %s /tmp/test.sh\n", testScriptRelPath)
b.WriteString("RUN chmod +x /tmp/test.sh\n")
}

if cmd.RemoteUser != defaultRemoteUser {
fmt.Fprintf(&b, "USER %s\n", cmd.RemoteUser)
}

return b.String()
}

func (cmd *TestCmd) dockerBuild(dockerfile, contextDir, imageName string) error {
args := []string{"build", "-t", imageName, "-f", "-", contextDir}
dockerCmd := exec.Command("docker", args...) // #nosec G204 -- args built from trusted inputs
dockerCmd.Stdin = strings.NewReader(dockerfile)

if !cmd.Quiet {
dockerCmd.Stdout = os.Stdout
dockerCmd.Stderr = os.Stderr
}

return dockerCmd.Run()
}

func (cmd *TestCmd) dockerRun(imageName, containerName string) error {
args := []string{"run", "--name", containerName}
if !cmd.PreserveTestContainers {
args = append(args, "--rm")
}
args = append(args, imageName, "/tmp/test.sh")

dockerCmd := exec.Command("docker", args...) // #nosec G204 -- args built from trusted inputs

if !cmd.Quiet {
dockerCmd.Stdout = os.Stdout
dockerCmd.Stderr = os.Stderr
}

return dockerCmd.Run()
Comment on lines +325 to +339

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Quiet=true makes failures completely undiagnosable

When --quiet is set, neither stdout nor stderr is wired, so result.Error ends up as just "exit status 1". The printed results table (FAIL: my-feature — exit status 1) gives users no indication of what the test script actually reported.

Even in quiet mode, capturing stderr and including it in result.Error is essential:

🐛 Proposed fix
 func (cmd *TestCmd) dockerRun(imageName, containerName string) error {
 	args := []string{"run", "--name", containerName}
 	if !cmd.PreserveTestContainers {
 		args = append(args, "--rm")
 	}
 	args = append(args, imageName, "/tmp/test.sh")

 	dockerCmd := exec.Command("docker", args...) // `#nosec` G204

 	if !cmd.Quiet {
 		dockerCmd.Stdout = os.Stdout
 		dockerCmd.Stderr = os.Stderr
+		return dockerCmd.Run()
 	}

-	return dockerCmd.Run()
+	// In quiet mode, still capture stderr so failures are diagnosable.
+	var stderr strings.Builder
+	dockerCmd.Stderr = &stderr
+	if err := dockerCmd.Run(); err != nil {
+		return fmt.Errorf("%w\n%s", err, strings.TrimSpace(stderr.String()))
+	}
+	return nil
 }

The same pattern applies to dockerBuild for build-time failures.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func (cmd *TestCmd) dockerRun(imageName, containerName string) error {
args := []string{"run", "--name", containerName}
if !cmd.PreserveTestContainers {
args = append(args, "--rm")
}
args = append(args, imageName, "/tmp/test.sh")
dockerCmd := exec.Command("docker", args...) // #nosec G204 -- args built from trusted inputs
if !cmd.Quiet {
dockerCmd.Stdout = os.Stdout
dockerCmd.Stderr = os.Stderr
}
return dockerCmd.Run()
func (cmd *TestCmd) dockerRun(imageName, containerName string) error {
args := []string{"run", "--name", containerName}
if !cmd.PreserveTestContainers {
args = append(args, "--rm")
}
args = append(args, imageName, "/tmp/test.sh")
dockerCmd := exec.Command("docker", args...) // `#nosec` G204
if !cmd.Quiet {
dockerCmd.Stdout = os.Stdout
dockerCmd.Stderr = os.Stderr
return dockerCmd.Run()
}
// In quiet mode, still capture stderr so failures are diagnosable.
var stderr strings.Builder
dockerCmd.Stderr = &stderr
if err := dockerCmd.Run(); err != nil {
return fmt.Errorf("%w\n%s", err, strings.TrimSpace(stderr.String()))
}
return nil
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/features/test.go` around lines 325 - 339, The dockerRun function
currently drops stderr when cmd.Quiet is true, making failures unhelpful; update
dockerRun (and apply same pattern to dockerBuild) to always capture stderr into
a buffer (e.g., bytes.Buffer) so the captured stderr can be inspected and
appended to the returned error or result.Error, while still wiring Stdout/Stderr
to os.Stdout/os.Stderr when !cmd.Quiet; ensure dockerCmd.Stderr is set to the
buffer unconditionally (and also to os.Stderr when not quiet) and wrap or
augment dockerCmd.Run()'s error with the buffer contents so exit failures
include the test script/build stderr.

}

func (cmd *TestCmd) dockerRemove(containerName string) {
rmCmd := exec.Command("docker", "rm", "-f", containerName) // #nosec G204
_ = rmCmd.Run()

rmiCmd := exec.Command("docker", "rmi", "-f", containerName+":latest") // #nosec G204
_ = rmiCmd.Run()
}

func (cmd *TestCmd) printResults(results []testResult) {
w := os.Stdout
_, _ = fmt.Fprintln(w, "\n=== Feature Test Results ===")
passed := 0
failed := 0

for _, r := range results {
label := r.FeatureID
if r.Scenario != "" {
label = fmt.Sprintf("%s/%s", r.FeatureID, r.Scenario)
}

if r.Passed {
_, _ = fmt.Fprintf(w, " PASS: %s\n", label)
passed++
} else {
_, _ = fmt.Fprintf(w, " FAIL: %s — %s\n", label, r.Error)
failed++
}
}

_, _ = fmt.Fprintf(w, "\nTotal: %d passed, %d failed\n", passed, failed)
}
Loading
Loading