ci(build-test-tidy): introduce success condition jobs

[xmfcx]

Description

I had initially made this change a part of the previous PR:

• ci: introduce build-test-tidy-pr #9709

But we removed it to allow a simpler structure.

But I think we cannot avoid it due to how we add things to Status checks that are required from rulesets menu.

Problem

Issue is, you add the required "jobs" to this list.

You cannot add workflows here, only jobs. And if jobs are within jobs as in reusable workflows, you need to include both names.

For example, for build-and-test-differential job, here is the list of allowed jobs currently available in this repo:

Add build-and-test-differential to list

❌ We end up with:

In a workflow. If we check more closely:

It actually needs build-and-test-differential / build-and-test-differential to complete.

Add build-and-test-differential / build-and-test-differential to list

It is solved ✅

This was to demonstrate how it works. But now I will share the problem part.

Add build-and-test-differential-cuda / build-and-test-differential

Success

It works if it is not skipped ✅

Skipping

❌ We end up with:

This happened because it was skipped. And sub-workflow did not execute at all.

Add build-and-test-differential-cuda to the list

Success

❌ We end up with:

Because the full job name is not in the list.

Skipping

It works if it is skipped ✅

Solution

This means, we cannot rely on skipping counted as success with reusable workflows.

So I'm bringing back these jobs that will run at all times and notify you about the success condition.

Related links

Tested PRs:

• fix(perception): fix perception docs #9766 (non-cuda change)
• test(autoware_behavior_path_start_planner_module): refactor unit tests by using parameter initialization function #9768 (cuda change)

How was this PR tested?

Notes for reviewers

None.

Interface changes

None.

Effects on system behavior

None.

[github-actions]

Thank you for contributing to the Autoware project!

🚧 If your pull request is in progress, switch it to draft mode.

Please ensure:

• You've checked our contribution guidelines.
• Your PR follows our pull request guidelines.
• All required CI checks pass before marking the PR ready for review.

[xmfcx]

I'll improve the readability of the results.

[xmfcx]

Tests

Normal Package Modification

🟢 Good Change

• https://github.com/autowarefoundation/autoware.universe/actions/runs/12484079126 ✅

🔴 Bad Change

• https://github.com/autowarefoundation/autoware.universe/actions/runs/12483758294 ✅

CUDA Package Modification

🟢 Good Change

• https://github.com/autowarefoundation/autoware.universe/actions/runs/12483835817 ✅

🔴 Bad Change

• https://github.com/autowarefoundation/autoware.universe/actions/runs/12483964878 ✅

No Package Modification

With Label 🟢

• https://github.com/autowarefoundation/autoware.universe/actions/runs/12484178602 ✅

Without Label 🔴

• https://github.com/autowarefoundation/autoware.universe/actions/runs/12484163011 ✅

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 29.65%. Comparing base (24bcd0e) to head (eac42cf).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9769      +/-   ##
==========================================
- Coverage   29.65%   29.65%   -0.01%     
==========================================
  Files        1450     1450              
  Lines      108837   108846       +9     
  Branches    42740    42744       +4     
==========================================
  Hits        32279    32279              
- Misses      73387    73396       +9     
  Partials     3171     3171              

Flag	Coverage Δ		*Carryforward flag
differential	0.00% <ø> (?)
differential-cuda	0.00% <ø> (?)		Carriedforward from b6e4711
total	29.65% <ø> (ø)		Carriedforward from b6e4711

*This pull request uses carry forward flags. Click here to find out more.

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

[mitsudome-r]

@xmfcx
Thanks for the fix. I was curious so I took a little look at this.

It seems like another workaround is to just pass the if condition to the reusable workflow as an argument, and skip the job inside the reusable workflow. This way, you will always have the result from the sub-workflow, and the required status check won't stop at "expected" even if it is skipped. (I've created a test PR on my fork here mitsudome-r#10 if you want to take a look)

This keeps the workflow code simpler, and developers would have similar user experience to check failed conditions as before. However, it is a bit of hacky way to solve the issue because we have to unnecessarily pass the condition as an argument so I'm okay with your solution as well.

Maybe we can wait for developers' reactions, and if they feel uncomfortable with the current fix, we can consider using this approach.

[xmfcx]

@mitsudome-r In my opinion, counting skipping as a success condition hides intention and confuses even more. It's better to make things explicit than implicit. For example, cuda workflows are marked as required but they can be skipped and it just passes, this, to me is confusing. But if this build-test-pr is marked as required, we explicitly report our success condition to developers.

Right now, if the workflow fails, they can click on the failed workflow and they will be pointed to where to look.

I've checked your solution and yes, it would solve this problem too. And with much less code.
Still, I feel it hides the success condition by allowing skipped items counted as suceeded implicitly.

If you really want to keep it the old way with the small fix, I understand too, we can apply your solution. 👍

[xmfcx]

By the way, the reason we have to deal with these complicated success conditions is that we started running CUDA jobs conditionally. Due to this, clang-tidy also branched into 2. It is the most efficient way to use limited compute resources. I think it's unavoidable to have this complicated structure as long as we aim to support CUDA and non-CUDA builds in a large codebase that has large parts that explicitly doesn't require CUDA.

[mitsudome-r]

@xmfcx Thanks for your comment.
I agree that GitHub's specification of treating skipped workflow as success condition is confusing.

I just created the PR to the upstream just in case if we want to apply the modification: #9771.
Maybe we can come back and discuss this in the next Software WG call.