Fix status merging with skipped pipelines

[qwerty287]

Split the skipped status into skipped (skipped due to condition) and canceled (skipped due to cancellation but hasn't started).

Still needs testing.

[qwerty287]

Seems working:
successful:

canceled (with step skipped, but workflow and pipeline is correctly marked as canceled):

[xoxys]

Needs frontend fix to get the failed icon in red again? Or is it intended to be gray in this case?

[qwerty287]

It is canceled, then it should be gray.

Before #6011 the canceled pipelines always had a failed state and thus were red, but this was the actual bug.

[xoxys]

OK then the handling is IMO still wrong. If a workflow fails and cancels other dependent workflows the entire pipeline should marked as failed, no?

[xoxys]

I still have questions 🙈

[qwerty287]

Not sure if I understand you correctly: If one workflow fails, all the other ones that haven't been started will get status skipped (in workflow_status.go, line 51). Then the final pipeline should be failed. The ones that already run will finish running, and the final status will get failed as well.

If you mean something different, it's also possible that I did a mistake. This full status system and how finishing workflows is handled is really complex and ununderstandable. Might need a big refactoring.

[xoxys]

My initial issue I posted is that exactly this case you decribed: One workflow failed, some other workflows were cancelled due to the failed one -> the entire pipeline should marked as failed. However this does not work anymore but was working before

In the secreenshot you can see that security-scan failed and docs was cancelled due to this failure but the entire pipeline is now marked as skipped and should still be marked as failed.

The second screenshot shows the exact same pipeline a few weeks earlier (with an older wp version) where it is working correctly.

At least that what I tried to describe in chat and #6164 (comment)

[qwerty287]

Yes, but isn't this what I fixed here?

Working as well for me (on-fail shouldn't run on fails… this was my first test with runs_on, thus the name):

[codecov]

Codecov Report

❌ Patch coverage is 44.44444% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 31.95%. Comparing base (05f55fb) to head (db44760).
⚠️ Report is 15 commits behind head on main.

Files with missing lines	Patch %	Lines
server/pipeline/cancel.go	0.00%	10 Missing ⚠️
server/pipeline/status.go	70.00%	2 Missing and 1 partial ⚠️
server/model/const.go	0.00%	1 Missing ⚠️
server/rpc/rpc.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6176      +/-   ##
==========================================
- Coverage   31.98%   31.95%   -0.04%     
==========================================
  Files         423      423              
  Lines       28568    28563       -5     
==========================================
- Hits         9138     9126      -12     
- Misses      18587    18595       +8     
+ Partials      843      842       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[xoxys]

Ok nice. Then I just got confused by your screenshots :D

[xoxys]

Thanks for re-checking!

[qwerty287]

Sorry, I actually haven't tested that before.

I thought your point was about runs_on: [failure]… But the fix is the same for both