From b14398eff3d3a2692c976e250c4b91fbbeed00a5 Mon Sep 17 00:00:00 2001
From: Markus Wolf <KnisterPeter@users.noreply.github.com>
Date: Tue, 10 Jan 2023 22:31:12 +0100
Subject: [PATCH] fix: preserve job result state in case of failure (#1519)

* fix: preserve job result state in case of failure

There is just one job field for the job result. This is also true for
matrix jobs. We need to preserve the failure state of a job to
have the whole job failing in case of one permuation of the matrix failed.

Closes #1518

* test: remove continue-on-error on job level

This feature is not yet supported by act and if implemented
would make this test invalid

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
---
 pkg/runner/job_executor.go                   | 16 ++++++++++++++--
 pkg/runner/runner_test.go                    |  1 +
 pkg/runner/testdata/matrix-exitcode/push.yml | 16 ++++++++++++++++
 3 files changed, 31 insertions(+), 2 deletions(-)
 create mode 100644 pkg/runner/testdata/matrix-exitcode/push.yml

diff --git a/pkg/runner/job_executor.go b/pkg/runner/job_executor.go
index 4ae7787..88c227f 100644
--- a/pkg/runner/job_executor.go
+++ b/pkg/runner/job_executor.go
@@ -130,17 +130,29 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo
 
 func setJobResult(ctx context.Context, info jobInfo, rc *RunContext, success bool) {
 	logger := common.Logger(ctx)
+
 	jobResult := "success"
-	jobResultMessage := "succeeded"
+	// we have only one result for a whole matrix build, so we need
+	// to keep an existing result state if we run a matrix
+	if len(info.matrix()) > 0 && rc.Run.Job().Result != "" {
+		jobResult = rc.Run.Job().Result
+	}
+
 	if !success {
 		jobResult = "failure"
-		jobResultMessage = "failed"
 	}
+
 	info.result(jobResult)
 	if rc.caller != nil {
 		// set reusable workflow job result
 		rc.caller.runContext.result(jobResult)
 	}
+
+	jobResultMessage := "succeeded"
+	if jobResult != "success" {
+		jobResultMessage = "failed"
+	}
+
 	logger.WithField("jobResult", jobResult).Infof("\U0001F3C1  Job %s", jobResultMessage)
 }
 
diff --git a/pkg/runner/runner_test.go b/pkg/runner/runner_test.go
index 5cf4837..ceae70c 100644
--- a/pkg/runner/runner_test.go
+++ b/pkg/runner/runner_test.go
@@ -170,6 +170,7 @@ func TestRunEvent(t *testing.T) {
 		{workdir, "remote-action-js", "push", "", map[string]string{"ubuntu-latest": "catthehacker/ubuntu:runner-latest"}, secrets}, // Test if this works with non root container
 		{workdir, "matrix", "push", "", platforms, secrets},
 		{workdir, "matrix-include-exclude", "push", "", platforms, secrets},
+		{workdir, "matrix-exitcode", "push", "Job 'test' failed", platforms, secrets},
 		{workdir, "commands", "push", "", platforms, secrets},
 		{workdir, "workdir", "push", "", platforms, secrets},
 		{workdir, "defaults-run", "push", "", platforms, secrets},
diff --git a/pkg/runner/testdata/matrix-exitcode/push.yml b/pkg/runner/testdata/matrix-exitcode/push.yml
new file mode 100644
index 0000000..0f5d335
--- /dev/null
+++ b/pkg/runner/testdata/matrix-exitcode/push.yml
@@ -0,0 +1,16 @@
+name: test
+
+on: push
+
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        val: ["success", "failure"]
+      fail-fast: false
+    steps:
+      - name: test
+        run: |
+          echo "Expected job result: ${{ matrix.val }}"
+          [[ "${{ matrix.val }}" = "success" ]] || exit 1