test

coder · mafredri · Mar 27, 2025 · Mar 24, 2025 · Mar 24, 2025 · Mar 25, 2025
commit 234fc252dca4e04399956ab8f27f4e6d07c602aa
diff --git a/agent/agent.go b/agent/agent.go
@@ -1121,11 +1121,9 @@ func (a *agent) handleManifest(manifestOK *checkpoint) func(ctx context.Context,
 			)
 			if a.experimentalDevcontainersEnabled {
 				var dcScripts []codersdk.WorkspaceAgentScript
-				scripts, dcScripts = agentcontainers.ExtractDevcontainerScripts(a.logger, expandPathToAbs, manifest.Devcontainers, scripts)
-				// The post-start scripts are used to autostart Dev Containers
-				// after the start scripts have completed. This is necessary
-				// because the Dev Container may depend on the workspace being
-				// initialized (git clone, etc).
+				scripts, dcScripts = agentcontainers.ExtractAndInitializeDevcontainerScripts(a.logger, expandPathToAbs, manifest.Devcontainers, scripts)
+				// See ExtractAndInitializeDevcontainerScripts for motivation
+				// behind running dcScripts as post start scripts.
 				scriptRunnerOpts = append(scriptRunnerOpts, agentscripts.WithPostStartScripts(dcScripts...))
 			}
 			err = a.scriptRunner.Init(scripts, aAPI.ScriptCompleted, scriptRunnerOpts...)

diff --git a/agent/agentcontainers/devcontainer.go b/agent/agentcontainers/devcontainer.go
@@ -18,39 +18,49 @@ fi
 devcontainer up %s
 `
 
-// DevcontainerStartupScript returns a script that starts a devcontainer.
-func DevcontainerStartupScript(dc codersdk.WorkspaceAgentDevcontainer, script codersdk.WorkspaceAgentScript) codersdk.WorkspaceAgentScript {
-	var args []string
-	args = append(args, fmt.Sprintf("--workspace-folder %q", dc.WorkspaceFolder))
-	if dc.ConfigPath != "" {
-		args = append(args, fmt.Sprintf("--config %q", dc.ConfigPath))
-	}
-	cmd := fmt.Sprintf(devcontainerUpScriptTemplate, strings.Join(args, " "))
-	script.RunOnStart = false
-	script.Script = cmd
-	return script
-}
-
-func ExtractDevcontainerScripts(
+// ExtractAndInitializeDevcontainerScripts extracts devcontainer scripts from
+// the given scripts and devcontainers. The devcontainer scripts are removed
+// from the returned scripts so that they can be run separately.
+//
+// Dev Containers have an inherent dependency on start scripts, since they
+// initialize the workspace (e.g. git clone, npm install, etc). This is
+// important if e.g. a Coder module to install @devcontainer/cli is used.
+func ExtractAndInitializeDevcontainerScripts(
 	logger slog.Logger,
 	expandPath func(string) (string, error),
 	devcontainers []codersdk.WorkspaceAgentDevcontainer,
 	scripts []codersdk.WorkspaceAgentScript,
-) (other []codersdk.WorkspaceAgentScript, devcontainerScripts []codersdk.WorkspaceAgentScript) {
-	for _, dc := range devcontainers {
-		dc = expandDevcontainerPaths(logger, expandPath, dc)
-		for _, script := range scripts {
+) (filteredScripts []codersdk.WorkspaceAgentScript, devcontainerScripts []codersdk.WorkspaceAgentScript) {
+ScriptLoop:
+	for _, script := range scripts {
+		for _, dc := range devcontainers {
 			// The devcontainer scripts match the devcontainer ID for
 			// identification.
 			if script.ID == dc.ID {
-				devcontainerScripts = append(devcontainerScripts, DevcontainerStartupScript(dc, script))
-			} else {
-				other = append(other, script)
+				dc = expandDevcontainerPaths(logger, expandPath, dc)
+				devcontainerScripts = append(devcontainerScripts, devcontainerStartupScript(dc, script))
+				continue ScriptLoop
 			}
 		}
+
+		filteredScripts = append(filteredScripts, script)
 	}
 
-	return other, devcontainerScripts
+	return filteredScripts, devcontainerScripts
+}
+
+func devcontainerStartupScript(dc codersdk.WorkspaceAgentDevcontainer, script codersdk.WorkspaceAgentScript) codersdk.WorkspaceAgentScript {
+	var args []string
+	args = append(args, fmt.Sprintf("--workspace-folder %q", dc.WorkspaceFolder))
+	if dc.ConfigPath != "" {
+		args = append(args, fmt.Sprintf("--config %q", dc.ConfigPath))
+	}
+	cmd := fmt.Sprintf(devcontainerUpScriptTemplate, strings.Join(args, " "))
+	script.Script = cmd
+	// Disable RunOnStart, scripts have this set so that when devcontainers
+	// have not been enabled, a warning will be surfaced in the agent logs.
+	script.RunOnStart = false
+	return script
 }
 
 func expandDevcontainerPaths(logger slog.Logger, expandPath func(string) (string, error), dc codersdk.WorkspaceAgentDevcontainer) codersdk.WorkspaceAgentDevcontainer {

diff --git a/agent/agentcontainers/devcontainer_test.go b/agent/agentcontainers/devcontainer_test.go
@@ -0,0 +1,226 @@
+package agentcontainers_test
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+	"github.com/google/uuid"
+	"github.com/stretchr/testify/require"
+
+	"cdr.dev/slog/sloggers/slogtest"
+	"github.com/coder/coder/v2/agent/agentcontainers"
+	"github.com/coder/coder/v2/codersdk"
+)
+
+func TestExtractAndInitializeDevcontainerScripts(t *testing.T) {
+	t.Parallel()
+
+	scriptIDs := []uuid.UUID{uuid.New(), uuid.New()}
+	devcontainerIDs := []uuid.UUID{uuid.New(), uuid.New()}
+
+	type args struct {
+		expandPath    func(string) (string, error)
+		devcontainers []codersdk.WorkspaceAgentDevcontainer
+		scripts       []codersdk.WorkspaceAgentScript
+	}
+	tests := []struct {
+		name                    string
+		args                    args
+		wantFilteredScripts     []codersdk.WorkspaceAgentScript
+		wantDevcontainerScripts []codersdk.WorkspaceAgentScript
+	}{
+		{
+			name: "no scripts",
+			args: args{
+				expandPath:    nil,
+				devcontainers: nil,
+				scripts:       nil,
+			},
+			wantFilteredScripts:     nil,
+			wantDevcontainerScripts: nil,
+		},
+		{
+			name: "no devcontainers",
+			args: args{
+				expandPath:    nil,
+				devcontainers: nil,
+				scripts: []codersdk.WorkspaceAgentScript{
+					{ID: scriptIDs[0]},
+					{ID: scriptIDs[1]},
+				},
+			},
+			wantFilteredScripts: []codersdk.WorkspaceAgentScript{
+				{ID: scriptIDs[0]},
+				{ID: scriptIDs[1]},
+			},
+			wantDevcontainerScripts: nil,
+		},
+		{
+			name: "no scripts match devcontainers",
+			args: args{
+				expandPath: nil,
+				devcontainers: []codersdk.WorkspaceAgentDevcontainer{
+					{ID: devcontainerIDs[0]},
+					{ID: devcontainerIDs[1]},
+				},
+				scripts: []codersdk.WorkspaceAgentScript{
+					{ID: scriptIDs[0]},
+					{ID: scriptIDs[1]},
+				},
+			},
+			wantFilteredScripts: []codersdk.WorkspaceAgentScript{
+				{ID: scriptIDs[0]},
+				{ID: scriptIDs[1]},
+			},
+			wantDevcontainerScripts: nil,
+		},
+		{
+			name: "scripts match devcontainers and sets RunOnStart=false",
+			args: args{
+				expandPath: nil,
+				devcontainers: []codersdk.WorkspaceAgentDevcontainer{
+					{ID: devcontainerIDs[0], WorkspaceFolder: "workspace1"},
+					{ID: devcontainerIDs[1], WorkspaceFolder: "workspace2"},
+				},
+				scripts: []codersdk.WorkspaceAgentScript{
+					{ID: scriptIDs[0], RunOnStart: true},
+					{ID: scriptIDs[1], RunOnStart: true},
+					{ID: devcontainerIDs[0], RunOnStart: true},
+					{ID: devcontainerIDs[1], RunOnStart: true},
+				},
+			},
+			wantFilteredScripts: []codersdk.WorkspaceAgentScript{
+				{ID: scriptIDs[0], RunOnStart: true},
+				{ID: scriptIDs[1], RunOnStart: true},
+			},
+			wantDevcontainerScripts: []codersdk.WorkspaceAgentScript{
+				{
+					ID:         devcontainerIDs[0],
+					Script:     "devcontainer up --workspace-folder \"workspace1\"",
+					RunOnStart: false,
+				},
+				{
+					ID:         devcontainerIDs[1],
+					Script:     "devcontainer up --workspace-folder \"workspace2\"",
+					RunOnStart: false,
+				},
+			},
+		},
+		{
+			name: "scripts match devcontainers with config path",
+			args: args{
+				expandPath: nil,
+				devcontainers: []codersdk.WorkspaceAgentDevcontainer{
+					{
+						ID:              devcontainerIDs[0],
+						WorkspaceFolder: "workspace1",
+						ConfigPath:      "config1",
+					},
+					{
+						ID:              devcontainerIDs[1],
+						WorkspaceFolder: "workspace2",
+						ConfigPath:      "config2",
+					},
+				},
+				scripts: []codersdk.WorkspaceAgentScript{
+					{ID: devcontainerIDs[0]},
+					{ID: devcontainerIDs[1]},
+				},
+			},
+			wantFilteredScripts: []codersdk.WorkspaceAgentScript{},
+			wantDevcontainerScripts: []codersdk.WorkspaceAgentScript{
+				{
+					ID:         devcontainerIDs[0],
+					Script:     "devcontainer up --workspace-folder \"workspace1\" --config \"config1\"",
+					RunOnStart: false,
+				},
+				{
+					ID:         devcontainerIDs[1],
+					Script:     "devcontainer up --workspace-folder \"workspace2\" --config \"config2\"",
+					RunOnStart: false,
+				},
+			},
+		},
+		{
+			name: "scripts match devcontainers with expand path",
+			args: args{
+				expandPath: func(s string) (string, error) {
 					return "expanded/" + s, nil
+				},
+				devcontainers: []codersdk.WorkspaceAgentDevcontainer{
+					{
+						ID:              devcontainerIDs[0],
+						WorkspaceFolder: "workspace1",
+						ConfigPath:      "config1",
+					},
+					{
+						ID:              devcontainerIDs[1],
+						WorkspaceFolder: "workspace2",
+						ConfigPath:      "config2",
+					},
+				},
+				scripts: []codersdk.WorkspaceAgentScript{
+					{ID: devcontainerIDs[0], RunOnStart: true},
+					{ID: devcontainerIDs[1], RunOnStart: true},
+				},
+			},
+			wantFilteredScripts: []codersdk.WorkspaceAgentScript{},
+			wantDevcontainerScripts: []codersdk.WorkspaceAgentScript{
+				{
+					ID:         devcontainerIDs[0],
+					Script:     "devcontainer up --workspace-folder \"expanded/workspace1\" --config \"expanded/config1\"",
+					RunOnStart: false,
+				},
+				{
+					ID:         devcontainerIDs[1],
+					Script:     "devcontainer up --workspace-folder \"expanded/workspace2\" --config \"expanded/config2\"",
+					RunOnStart: false,
+				},
+			},
+		},
+	}
+	// nolint:foo
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			logger := slogtest.Make(t, nil)
+			if tt.args.expandPath == nil {
+				tt.args.expandPath = func(s string) (string, error) {
+					return s, nil
+				}
+			}
+			gotFilteredScripts, gotDevcontainerScripts := agentcontainers.ExtractAndInitializeDevcontainerScripts(
+				logger,
+				tt.args.expandPath,
+				tt.args.devcontainers,
+				tt.args.scripts,
+			)
+
+			if diff := cmp.Diff(tt.wantFilteredScripts, gotFilteredScripts, cmpopts.EquateEmpty()); diff != "" {
+				t.Errorf("ExtractAndInitializeDevcontainerScripts() gotFilteredScripts mismatch (-want +got):\n%s", diff)
+			}
+
+			// Preprocess the devcontainer scripts to remove scripting part.
+			for i := range gotDevcontainerScripts {
+				gotDevcontainerScripts[i].Script = textGrep("devcontainer up", gotDevcontainerScripts[i].Script)
+				require.NotEmpty(t, gotDevcontainerScripts[i].Script, "devcontainer up script not found")
+			}
+			if diff := cmp.Diff(tt.wantDevcontainerScripts, gotDevcontainerScripts); diff != "" {
+				t.Errorf("ExtractAndInitializeDevcontainerScripts() gotDevcontainerScripts mismatch (-want +got):\n%s", diff)
+			}
+		})
+	}
+}
+
+// textGrep returns matching lines from multiline string.
+func textGrep(want, got string) (filtered string) {
+	var lines []string
+	for _, line := range strings.Split(got, "\n") {
+		if strings.Contains(line, want) {
+			lines = append(lines, line)
+		}
+	}
+	return strings.Join(lines, "\n")
+}