From 7eb547faa5acbe09f65fefd522a894ccb5aebd9c Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Mon, 7 Jul 2025 12:11:57 +0000 Subject: [PATCH] fix: do not fail the job when `if: false` (#172) - log job result as info not as debug - add test --- v6.4.0 regression introduced in 4880b091a2dbf0cffaf2064227094eea11e0619b It did not fail a test because the [original fix](https://code.forgejo.org/forgejo/act/pulls/67/files) has tests only for the case where a step is skipped, not when a job is skipped. Closes forgejo/runner#660 Reviewed-on: https://code.forgejo.org/forgejo/act/pulls/172 Reviewed-by: Michael Kriese Co-authored-by: Earl Warren Co-committed-by: Earl Warren --- act/runner/run_context.go | 2 +- act/runner/runner_test.go | 39 +++++++++++-------- act/runner/testdata/skipjob/skipjob.yml | 11 ++++++ .../{skip/skip.yml => skipstep/skipstep.yml} | 4 +- 4 files changed, 36 insertions(+), 20 deletions(-) create mode 100644 act/runner/testdata/skipjob/skipjob.yml rename act/runner/testdata/{skip/skip.yml => skipstep/skipstep.yml} (77%) diff --git a/act/runner/run_context.go b/act/runner/run_context.go index fc7b77a8..c0f510d8 100644 --- a/act/runner/run_context.go +++ b/act/runner/run_context.go @@ -914,7 +914,7 @@ func (rc *RunContext) isEnabled(ctx context.Context) (bool, error) { if !runJob { rc.result("skipped") - l.WithField("jobResult", "skipped").Debugf("Skipping job '%s' due to '%s'", job.Name, job.If.Value) + l.WithField("jobResult", "skipped").Infof("Skipping job '%s' due to '%s'", job.Name, job.If.Value) return false, nil } diff --git a/act/runner/runner_test.go b/act/runner/runner_test.go index de88f017..74ff8418 100644 --- a/act/runner/runner_test.go +++ b/act/runner/runner_test.go @@ -520,7 +520,8 @@ func TestRunDifferentArchitecture(t *testing.T) { } type runSkippedHook struct { - found bool + resultKey string + found bool } func (h *runSkippedHook) Levels() []log.Level { @@ -528,8 +529,8 @@ func (h *runSkippedHook) Levels() []log.Level { } func (h *runSkippedHook) Fire(entry *log.Entry) error { - if v, ok := entry.Data["stepResult"]; ok { - h.found = (v == model.StepStatusSkipped) + if result, ok := entry.Data[h.resultKey]; ok { + h.found = (fmt.Sprintf("%s", result) == "skipped") } return nil } @@ -539,21 +540,25 @@ func TestRunSkipped(t *testing.T) { t.Skip("skipping integration test") } - tjfi := TestJobFileInfo{ - workdir: workdir, - workflowPath: "skip", - eventName: "push", - errorMessage: "", - platforms: platforms, + for _, what := range []string{"step", "job"} { + t.Run(what, func(t *testing.T) { + tjfi := TestJobFileInfo{ + workdir: workdir, + workflowPath: "skip" + what, + eventName: "push", + errorMessage: "", + platforms: platforms, + } + + h := &runSkippedHook{resultKey: what + "Result"} + ctx := common.WithLoggerHook(context.Background(), h) + + jobLoggerLevel := log.InfoLevel + tjfi.runTest(ctx, t, &Config{ContainerArchitecture: "linux/arm64", JobLoggerLevel: &jobLoggerLevel}) + + assert.True(t, h.found) + }) } - - h := &runSkippedHook{} - ctx := common.WithLoggerHook(context.Background(), h) - - jobLoggerLevel := log.InfoLevel - tjfi.runTest(ctx, t, &Config{ContainerArchitecture: "linux/arm64", JobLoggerLevel: &jobLoggerLevel}) - - assert.True(t, h.found) } type maskJobLoggerFactory struct { diff --git a/act/runner/testdata/skipjob/skipjob.yml b/act/runner/testdata/skipjob/skipjob.yml new file mode 100644 index 00000000..e425fd24 --- /dev/null +++ b/act/runner/testdata/skipjob/skipjob.yml @@ -0,0 +1,11 @@ +name: skipjob +on: push + +jobs: + checkjob: + runs-on: ubuntu-latest + + if: false + + steps: + - run: echo nothing diff --git a/act/runner/testdata/skip/skip.yml b/act/runner/testdata/skipstep/skipstep.yml similarity index 77% rename from act/runner/testdata/skip/skip.yml rename to act/runner/testdata/skipstep/skipstep.yml index 7f6d303a..8e67f0c9 100644 --- a/act/runner/testdata/skip/skip.yml +++ b/act/runner/testdata/skipstep/skipstep.yml @@ -1,8 +1,8 @@ -name: skip +name: skipstep on: push jobs: - check: + checkstep: runs-on: ubuntu-latest steps: - if: false