mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2026-05-12 22:10:25 +00:00
feat: reusable workflow outer job is skipped if 'if:' block skips workflow (#12412)
Follow-up to https://code.forgejo.org/forgejo/runner/pulls/1509 -- improves the UX in Forgejo when a reusable workflow is skipped, marking the workflow as skipped rather than succeeded. ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. All work and communication must conform to Forgejo's [AI Agreement](https://codeberg.org/forgejo/governance/src/branch/main/AIAgreement.md). There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests for Go changes - I added test coverage for Go changes... - [x] in their respective `*_test.go` for unit tests. - [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I ran... - [x] `make pr-go` before pushing ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [x] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change. - [ ] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12412 Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.codeberg.org>
This commit is contained in:
parent
525a377c24
commit
c1ac671b55
4 changed files with 52 additions and 5 deletions
|
|
@ -111,6 +111,8 @@ func fullStepsOfEmptySteps(task *actions_model.ActionTask) []*actions_model.Acti
|
||||||
preStep.Status = task.Status
|
preStep.Status = task.Status
|
||||||
if preStep.Status.IsSuccess() {
|
if preStep.Status.IsSuccess() {
|
||||||
postStep.Status = actions_model.StatusSuccess
|
postStep.Status = actions_model.StatusSuccess
|
||||||
|
} else if preStep.Status.IsSkipped() {
|
||||||
|
postStep.Status = actions_model.StatusSkipped
|
||||||
} else {
|
} else {
|
||||||
postStep.Status = actions_model.StatusCancelled
|
postStep.Status = actions_model.StatusCancelled
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -156,6 +156,21 @@ func TestFullSteps(t *testing.T) {
|
||||||
{Name: postStepName, Status: actions_model.StatusSuccess, LogIndex: 90, LogLength: 10, Started: 10090, Stopped: 10100},
|
{Name: postStepName, Status: actions_model.StatusSuccess, LogIndex: 90, LogLength: 10, Started: 10090, Stopped: 10100},
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
// situation occurs with a reusable workflow's outer job which has no steps
|
||||||
|
name: "skipped task w/ zero steps",
|
||||||
|
task: &actions_model.ActionTask{
|
||||||
|
Steps: []*actions_model.ActionTaskStep{},
|
||||||
|
Status: actions_model.StatusSkipped,
|
||||||
|
Started: 0,
|
||||||
|
Stopped: 0,
|
||||||
|
LogLength: 0,
|
||||||
|
},
|
||||||
|
want: []*actions_model.ActionTaskStep{
|
||||||
|
{Name: preStepName, Status: actions_model.StatusSkipped, LogIndex: 0, LogLength: 0, Started: 0, Stopped: 0},
|
||||||
|
{Name: postStepName, Status: actions_model.StatusSkipped, LogIndex: 0, LogLength: 0, Started: 0, Stopped: 0},
|
||||||
|
},
|
||||||
|
},
|
||||||
}
|
}
|
||||||
for _, tt := range tests {
|
for _, tt := range tests {
|
||||||
t.Run(tt.name, func(t *testing.T) {
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
|
|
||||||
|
|
@ -77,7 +77,7 @@ func checkJobsOfRun(ctx context.Context, runID int64, recursionCount int) error
|
||||||
job.Status = status
|
job.Status = status
|
||||||
updateColumns := []string{"status"}
|
updateColumns := []string{"status"}
|
||||||
|
|
||||||
if status == actions_model.StatusWaiting {
|
if status.IsWaiting() {
|
||||||
behaviour, err := tryHandleIncompleteMatrix(ctx, job, jobs)
|
behaviour, err := tryHandleIncompleteMatrix(ctx, job, jobs)
|
||||||
switch behaviour {
|
switch behaviour {
|
||||||
case behaviourError:
|
case behaviourError:
|
||||||
|
|
@ -95,7 +95,7 @@ func checkJobsOfRun(ctx context.Context, runID int64, recursionCount int) error
|
||||||
// Stop processing any other jobs in this run.
|
// Stop processing any other jobs in this run.
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
} else if status == actions_model.StatusSuccess || status == actions_model.StatusFailure {
|
} else if status.IsSuccess() || status.IsFailure() || status.IsSkipped() {
|
||||||
// Transition to these states can be triggered by workflow call outer jobs
|
// Transition to these states can be triggered by workflow call outer jobs
|
||||||
additionalColumns, err := tryHandleWorkflowCallOuterJob(ctx, job)
|
additionalColumns, err := tryHandleWorkflowCallOuterJob(ctx, job)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|
@ -192,7 +192,7 @@ func (r *jobStatusResolver) resolve() map[int64]actions_model.Status {
|
||||||
if status != actions_model.StatusBlocked {
|
if status != actions_model.StatusBlocked {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
allDone, allSucceed, allSucceedOrSkip := true, true, true
|
allDone, allSucceed, allSucceedOrSkip, allSkip := true, true, true, true
|
||||||
for _, need := range r.needs[id] {
|
for _, need := range r.needs[id] {
|
||||||
needStatus := r.statuses[need]
|
needStatus := r.statuses[need]
|
||||||
if !needStatus.IsDone() {
|
if !needStatus.IsDone() {
|
||||||
|
|
@ -204,12 +204,15 @@ func (r *jobStatusResolver) resolve() map[int64]actions_model.Status {
|
||||||
if needStatus.In(actions_model.StatusFailure, actions_model.StatusCancelled) {
|
if needStatus.In(actions_model.StatusFailure, actions_model.StatusCancelled) {
|
||||||
allSucceedOrSkip = false
|
allSucceedOrSkip = false
|
||||||
}
|
}
|
||||||
|
if !needStatus.IsSkipped() {
|
||||||
|
allSkip = false
|
||||||
|
}
|
||||||
}
|
}
|
||||||
if allDone {
|
if allDone {
|
||||||
if isWorkflowCallOuterJob, _ := r.jobMap[id].IsWorkflowCallOuterJob(); isWorkflowCallOuterJob {
|
if isWorkflowCallOuterJob, _ := r.jobMap[id].IsWorkflowCallOuterJob(); isWorkflowCallOuterJob {
|
||||||
// If the dependent job was a workflow call outer job, then options aren't waiting/skipped, but rather
|
// If the dependent job was a workflow call outer job, then options aren't waiting/skipped, but rather
|
||||||
// success/failure. checkJobsOfRun will do additional work in these cases to "finish" the workflow call
|
// success/skip/failure. checkJobsOfRun will do additional work in these cases to "finish" the workflow
|
||||||
// job as well.
|
// call job as well.
|
||||||
if allSucceedOrSkip {
|
if allSucceedOrSkip {
|
||||||
isIncompleteMatrix, _, _ := r.jobMap[id].HasIncompleteMatrix()
|
isIncompleteMatrix, _, _ := r.jobMap[id].HasIncompleteMatrix()
|
||||||
isIncompleteWith, _, _, _ := r.jobMap[id].HasIncompleteWith()
|
isIncompleteWith, _, _, _ := r.jobMap[id].HasIncompleteWith()
|
||||||
|
|
@ -221,6 +224,12 @@ func (r *jobStatusResolver) resolve() map[int64]actions_model.Status {
|
||||||
// `tryHandleIncompleteMatrix` to be reparsed, replaced with a full job definition, with new
|
// `tryHandleIncompleteMatrix` to be reparsed, replaced with a full job definition, with new
|
||||||
// `needs` that contain its inner jobs:
|
// `needs` that contain its inner jobs:
|
||||||
ret[id] = actions_model.StatusWaiting
|
ret[id] = actions_model.StatusWaiting
|
||||||
|
} else if allSkip {
|
||||||
|
// All of the inner jobs are skipped -- this most likely occurs because an outer job's `if:`
|
||||||
|
// condition was false, and that condition was populated to all the inner jobs. Even if that's
|
||||||
|
// not the case, it's effectively true that the reusable workflow was skipped if all the inner
|
||||||
|
// jobs had their own `if:` conditions that were skipped.
|
||||||
|
ret[id] = actions_model.StatusSkipped
|
||||||
} else {
|
} else {
|
||||||
// This job is done by virtue of its inner jobs being done successfully.
|
// This job is done by virtue of its inner jobs being done successfully.
|
||||||
ret[id] = actions_model.StatusSuccess
|
ret[id] = actions_model.StatusSuccess
|
||||||
|
|
|
||||||
|
|
@ -179,6 +179,27 @@ __metadata:
|
||||||
3: actions_model.StatusSuccess,
|
3: actions_model.StatusSuccess,
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
name: "unblocked workflow call outer job with only skip",
|
||||||
|
jobs: actions_model.ActionJobList{
|
||||||
|
{ID: 1, JobID: "job1.innerjob1", Status: actions_model.StatusSkipped, Needs: []string{}},
|
||||||
|
{ID: 2, JobID: "job1.innerjob2", Status: actions_model.StatusSkipped, Needs: []string{}},
|
||||||
|
{ID: 3, JobID: "job1", Status: actions_model.StatusBlocked, Needs: []string{"job1.innerjob1", "job1.innerjob2"}, WorkflowPayload: []byte(
|
||||||
|
`
|
||||||
|
name: test
|
||||||
|
on: push
|
||||||
|
jobs:
|
||||||
|
job2:
|
||||||
|
if: false
|
||||||
|
uses: ./.forgejo/workflows/reusable.yml
|
||||||
|
__metadata:
|
||||||
|
workflow_call_id: b5a9f46f1f2513d7777fde50b169d323a6519e349cc175484c947ac315a209ed
|
||||||
|
`)},
|
||||||
|
},
|
||||||
|
want: map[int64]actions_model.Status{
|
||||||
|
3: actions_model.StatusSkipped,
|
||||||
|
},
|
||||||
|
},
|
||||||
{
|
{
|
||||||
name: "unblocked workflow call outer job, incomplete `with`",
|
name: "unblocked workflow call outer job, incomplete `with`",
|
||||||
jobs: actions_model.ActionJobList{
|
jobs: actions_model.ActionJobList{
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue