bug/issue #2448 - manage special bash options when no shell is defined (#2449)

* bash without "-o pipefail" option when "bash" is not explicitely
defined in the workflow
* bonus: fix inverted expected and actual in TestGetGitHubContext assertions

Author: sebastien-perpignane

pkg/model/workflow.go

@@ -572,6 +572,8 @@ type Step struct {
 	Uses               string            `yaml:"uses"`
 	Run                string            `yaml:"run"`
 	WorkingDirectory   string            `yaml:"working-directory"`
+	// WorkflowShell is the shell really configured in the job, directly at step level or higher in defaults.run.shell
+	WorkflowShell      string            `yaml:"-"`
 	Shell              string            `yaml:"shell"`
 	Env                yaml.Node         `yaml:"env"`
 	With               map[string]string `yaml:"with"`
@@ -614,8 +616,14 @@ func (s *Step) ShellCommand() string {
 
 	//Reference: https://github.com/actions/runner/blob/8109c962f09d9acc473d92c595ff43afceddb347/src/Runner.Worker/Handlers/ScriptHandlerHelpers.cs#L9-L17
 	switch s.Shell {
-	case "", "bash":
-		shellCommand = "bash --noprofile --norc -e -o pipefail {0}"
+	case "":
+		shellCommand = "bash -e {0}"
+	case "bash":
+		if s.WorkflowShell == "" {
+			shellCommand = "bash -e {0}"
+		} else {
+			shellCommand = "bash --noprofile --norc -e -o pipefail {0}"
+		}
 	case "pwsh":
 		shellCommand = "pwsh -command . '{0}'"
 	case "python":

pkg/model/workflow_test.go

@@ -397,15 +397,18 @@ func TestReadWorkflow_Strategy(t *testing.T) {
 func TestStep_ShellCommand(t *testing.T) {
 	tests := []struct {
 		shell string
+		workflowShell string
 		want  string
 	}{
-		{"pwsh -v '. {0}'", "pwsh -v '. {0}'"},
-		{"pwsh", "pwsh -command . '{0}'"},
-		{"powershell", "powershell -command . '{0}'"},
+		{"pwsh -v '. {0}'", "", "pwsh -v '. {0}'"},
+		{"pwsh", "", "pwsh -command . '{0}'"},
+		{"powershell", "", "powershell -command . '{0}'"},
+		{"bash", "", "bash -e {0}"},
+		{"bash", "bash", "bash --noprofile --norc -e -o pipefail {0}"},
 	}
 	for _, tt := range tests {
 		t.Run(tt.shell, func(t *testing.T) {
-			got := (&Step{Shell: tt.shell}).ShellCommand()
+			got := (&Step{Shell: tt.shell, WorkflowShell: tt.workflowShell}).ShellCommand()
 			assert.Equal(t, got, tt.want)
 		})
 	}

pkg/runner/run_context_test.go

@@ -387,15 +387,15 @@ func TestGetGitHubContext(t *testing.T) {
 		owner = o
 	}
 
-	assert.Equal(t, ghc.RunID, "1")
-	assert.Equal(t, ghc.RunNumber, "1")
-	assert.Equal(t, ghc.RetentionDays, "0")
-	assert.Equal(t, ghc.Actor, actor)
-	assert.Equal(t, ghc.Repository, repo)
-	assert.Equal(t, ghc.RepositoryOwner, owner)
-	assert.Equal(t, ghc.RunnerPerflog, "/dev/null")
-	assert.Equal(t, ghc.Token, rc.Config.Secrets["GITHUB_TOKEN"])
-	assert.Equal(t, ghc.Job, "job1")
+	assert.Equal(t, "1", ghc.RunID)
+	assert.Equal(t, "1", ghc.RunNumber)
+	assert.Equal(t, "0", ghc.RetentionDays)
+	assert.Equal(t, actor, ghc.Actor)
+	assert.Equal(t, repo, ghc.Repository)
+	assert.Equal(t, owner, ghc.RepositoryOwner)
+	assert.Equal(t, "/dev/null", ghc.RunnerPerflog)
+	assert.Equal(t, rc.Config.Secrets["GITHUB_TOKEN"], ghc.Token)
+	assert.Equal(t, "job1", ghc.Job)
 }
 
 func TestGetGithubContextRef(t *testing.T) {

pkg/runner/step_run.go

@@ -166,16 +166,18 @@ func (sr *stepRun) setupShell(ctx context.Context) {
 	step := sr.Step
 
 	if step.Shell == "" {
-		step.Shell = rc.Run.Job().Defaults.Run.Shell
+		step.WorkflowShell = rc.Run.Job().Defaults.Run.Shell
+	} else {
+		step.WorkflowShell = step.Shell
 	}
 
-	step.Shell = rc.NewExpressionEvaluator(ctx).Interpolate(ctx, step.Shell)
+	step.WorkflowShell = rc.NewExpressionEvaluator(ctx).Interpolate(ctx, step.WorkflowShell)
 
-	if step.Shell == "" {
-		step.Shell = rc.Run.Workflow.Defaults.Run.Shell
+	if step.WorkflowShell == "" {
+		step.WorkflowShell = rc.Run.Workflow.Defaults.Run.Shell
 	}
 
-	if step.Shell == "" {
+	if step.WorkflowShell == "" {
 		if _, ok := rc.JobContainer.(*container.HostEnvironment); ok {
 			shellWithFallback := []string{"bash", "sh"}
 			// Don't use bash on windows by default, if not using a docker container
@@ -196,6 +198,8 @@ func (sr *stepRun) setupShell(ctx context.Context) {
 			// Currently only linux containers are supported, use sh by default like actions/runner
 			step.Shell = "sh"
 		}
+	} else {
+		step.Shell = step.WorkflowShell
 	}
 }