Fix wrong status of `Set up Job` when first step is skipped (#32120)

Fix #32089

diff --git a/modules/actions/task_state.go b/modules/actions/task_state.go
index 31a74be3fd360b0a942ece5427ffb659e5206239..1f36e021a59aa06a4e8e58adb09ad80b8ba155e2 100644 (file)
--- a/modules/actions/task_state.go
+++ b/modules/actions/task_state.go
@@ -18,8 +18,32 @@ func FullSteps(task *actions_model.ActionTask) []*actions_model.ActionTaskStep {
                return fullStepsOfEmptySteps(task)
        }
 
-       firstStep := task.Steps[0]
+       // firstStep is the first step that has run or running, not include preStep.
+       // For example,
+       // 1. preStep(Success) -> step1(Success) -> step2(Running) -> step3(Waiting) -> postStep(Waiting): firstStep is step1.
+       // 2. preStep(Success) -> step1(Skipped) -> step2(Success) -> postStep(Success): firstStep is step2.
+       // 3. preStep(Success) -> step1(Running) -> step2(Waiting) -> postStep(Waiting): firstStep is step1.
+       // 4. preStep(Success) -> step1(Skipped) -> step2(Skipped) -> postStep(Skipped): firstStep is nil.
+       // 5. preStep(Success) -> step1(Cancelled) -> step2(Cancelled) -> postStep(Cancelled): firstStep is nil.
+       var firstStep *actions_model.ActionTaskStep
+       // lastHasRunStep is the last step that has run.
+       // For example,
+       // 1. preStep(Success) -> step1(Success) -> step2(Running) -> step3(Waiting) -> postStep(Waiting): lastHasRunStep is step1.
+       // 2. preStep(Success) -> step1(Success) -> step2(Success) -> step3(Success) -> postStep(Success): lastHasRunStep is step3.
+       // 3. preStep(Success) -> step1(Success) -> step2(Failure) -> step3 -> postStep(Waiting): lastHasRunStep is step2.
+       // So its Stopped is the Started of postStep when there are no more steps to run.
+       var lastHasRunStep *actions_model.ActionTaskStep
+
        var logIndex int64
+       for _, step := range task.Steps {
+               if firstStep == nil && (step.Status.HasRun() || step.Status.IsRunning()) {
+                       firstStep = step
+               }
+               if step.Status.HasRun() {
+                       lastHasRunStep = step
+               }
+               logIndex += step.LogLength
+       }
 
        preStep := &actions_model.ActionTaskStep{
                Name:      preStepName,
@@ -28,32 +52,17 @@ func FullSteps(task *actions_model.ActionTask) []*actions_model.ActionTaskStep {
                Status:    actions_model.StatusRunning,
        }
 
-       if firstStep.Status.HasRun() || firstStep.Status.IsRunning() {
+       // No step has run or is running, so preStep is equal to the task
+       if firstStep == nil {
+               preStep.Stopped = task.Stopped
+               preStep.Status = task.Status
+       } else {
                preStep.LogLength = firstStep.LogIndex
                preStep.Stopped = firstStep.Started
                preStep.Status = actions_model.StatusSuccess
-       } else if task.Status.IsDone() {
-               preStep.Stopped = task.Stopped
-               preStep.Status = actions_model.StatusFailure
-               if task.Status.IsSkipped() {
-                       preStep.Status = actions_model.StatusSkipped
-               }
        }
        logIndex += preStep.LogLength
 
-       // lastHasRunStep is the last step that has run.
-       // For example,
-       // 1. preStep(Success) -> step1(Success) -> step2(Running) -> step3(Waiting) -> postStep(Waiting): lastHasRunStep is step1.
-       // 2. preStep(Success) -> step1(Success) -> step2(Success) -> step3(Success) -> postStep(Success): lastHasRunStep is step3.
-       // 3. preStep(Success) -> step1(Success) -> step2(Failure) -> step3 -> postStep(Waiting): lastHasRunStep is step2.
-       // So its Stopped is the Started of postStep when there are no more steps to run.
-       var lastHasRunStep *actions_model.ActionTaskStep
-       for _, step := range task.Steps {
-               if step.Status.HasRun() {
-                       lastHasRunStep = step
-               }
-               logIndex += step.LogLength
-       }
        if lastHasRunStep == nil {
                lastHasRunStep = preStep
        }

diff --git a/modules/actions/task_state_test.go b/modules/actions/task_state_test.go
index 28213d781befe967dfbd4858e6eaee5ee0f26f47..ff0fd5719511ee625294b26dd70ecd4efe138c3c 100644 (file)
--- a/modules/actions/task_state_test.go
+++ b/modules/actions/task_state_test.go
@@ -137,6 +137,25 @@ func TestFullSteps(t *testing.T) {
                                {Name: postStepName, Status: actions_model.StatusSkipped, LogIndex: 0, LogLength: 0, Started: 0, Stopped: 0},
                        },
                },
+               {
+                       name: "first step is skipped",
+                       task: &actions_model.ActionTask{
+                               Steps: []*actions_model.ActionTaskStep{
+                                       {Status: actions_model.StatusSkipped, LogIndex: 0, LogLength: 0, Started: 0, Stopped: 0},
+                                       {Status: actions_model.StatusSuccess, LogIndex: 10, LogLength: 80, Started: 10010, Stopped: 10090},
+                               },
+                               Status:    actions_model.StatusSuccess,
+                               Started:   10000,
+                               Stopped:   10100,
+                               LogLength: 100,
+                       },
+                       want: []*actions_model.ActionTaskStep{
+                               {Name: preStepName, Status: actions_model.StatusSuccess, LogIndex: 0, LogLength: 10, Started: 10000, Stopped: 10010},
+                               {Status: actions_model.StatusSkipped, LogIndex: 0, LogLength: 0, Started: 0, Stopped: 0},
+                               {Status: actions_model.StatusSuccess, LogIndex: 10, LogLength: 80, Started: 10010, Stopped: 10090},
+                               {Name: postStepName, Status: actions_model.StatusSuccess, LogIndex: 90, LogLength: 10, Started: 10090, Stopped: 10100},
+                       },
+               },
        }
        for _, tt := range tests {
                t.Run(tt.name, func(t *testing.T) {