Skip to content

fix CI step for publishing alternative providers results #13082

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open

fix CI step for publishing alternative providers results #13082

wants to merge 23 commits into from

Conversation

bentsku
Copy link
Contributor

@bentsku bentsku commented Sep 1, 2025 •
edited
Loading

Motivation

When adding a new SNS alternative provider with #13069, I've stumbled upon the fact that we did not publish the alternative providers test results, even if some of them were executed. See this comment: #13069 (review)

This is because the if condition for it was that all the needs jobs would need to be executed and not skipped. I've changed the condition to be that at least one is success or failure (so that not all of them are skipped).

There is a weird bug in the job if check that the needs.*.result call will return true for success in the if check but not if it is in the env one. I'm still not sure what it is is picking up, but manually specifying the job results we're looking for, as advised with https://docs.github.com/en/actions/reference/workflows-and-actions/expressions#example-matching-an-array-of-strings is now working. Not optimal, but at least it works... 😬

Edit: I've went with a second solution based on job output, if the job is skipped it doesn't set the output. This requires us to always properly set the job_status output, but it seems like the cleanest way for now. What we need is not properly supported by GitHub Actions out of the box.

This discussion is related to how to debug if conditions: https://github.com/orgs/community/discussions/20640

Changes

  • update the if check for the Publish Alternative Providers Test Results for at least one failure or success of the jobs it depends on, based on job outputs

TODO

Second time, with the new outputs logic:

@bentsku bentsku added this to the 4.8 milestone Sep 1, 2025
@bentsku bentsku self-assigned this Sep 1, 2025
@bentsku bentsku added semver: patch Non-breaking changes which can be included in patch releases skip-docs Pull request does not require documentation changes labels Sep 1, 2025
@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 1, 2025 •
edited
Loading

Test Results - Preflight, Unit

22 117 tests  ±0   20 379 ✅ ±0   6m 41s ⏱️ +12s
     1 suites ±0    1 738 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit a04930f. ± Comparison against base commit c426dbb.

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 1, 2025 •
edited
Loading

Test Results (MA/MR) - Preflight, Unit

22 117 tests   20 379 ✅  6m 30s ⏱️
     1 suites   1 738 💤
     1 files         0 ❌

Results for commit a04930f.

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 1, 2025 •
edited
Loading

Test Results (amd64) - Acceptance

7 tests  ±0   5 ✅ ±0   3m 7s ⏱️ -15s
1 suites ±0   2 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit a04930f. ± Comparison against base commit c426dbb.

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 1, 2025 •
edited
Loading

Test Results - Alternative Providers

379 tests   248 ✅  3m 33s ⏱️
  1 suites  131 💤
  1 files      0 ❌

Results for commit 5109396.

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 1, 2025 •
edited
Loading

Test Results (MA/MR) - Alternative Providers

379 tests   248 ✅  3m 34s ⏱️
  1 suites  131 💤
  1 files      0 ❌

Results for commit 5109396.

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 1, 2025 •
edited
Loading

Test Results (amd64, MA/MR) - Acceptance

7 tests   5 ✅  3m 5s ⏱️
1 suites  2 💤
1 files    0 ❌

Results for commit a04930f.

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 1, 2025 •
edited
Loading

Test Results (amd64, MA/MR) - Integration, Bootstrap

    5 files      5 suites   2h 21m 59s ⏱️
5 010 tests 4 408 ✅ 602 💤 0 ❌
5 016 runs  4 408 ✅ 608 💤 0 ❌

Results for commit a04930f.

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 1, 2025 •
edited
Loading

Test Results (amd64) - Integration, Bootstrap

    5 files      5 suites   2h 21m 53s ⏱️
5 010 tests 4 408 ✅ 602 💤 0 ❌
5 016 runs  4 408 ✅ 608 💤 0 ❌

Results for commit a04930f.

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 1, 2025

LocalStack Community integration with Pro

    2 files      2 suites   1h 43m 43s ⏱️
4 632 tests 4 189 ✅ 443 💤 0 ❌
4 634 runs  4 189 ✅ 445 💤 0 ❌

Results for commit 06d7d63.

@bentsku bentsku force-pushed the fix-alternative-provider-publish branch 2 times, most recently from bf84a94 to d452882 Compare September 2, 2025 20:24
@bentsku bentsku marked this pull request as ready for review September 2, 2025 22:21
silv-io
silv-io approved these changes Sep 3, 2025
View reviewed changes
Copy link
Member

@silv-io silv-io left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

It does look a bit unreadable now, but that's just how github expression syntax is :)

Hopefully the alternative providers won't scale too much in number :D

@bentsku
Copy link
Contributor Author

bentsku commented Sep 3, 2025

It does look a bit unreadable now, but that's just how github expression syntax is :)

Hopefully the alternative providers won't scale too much in number :D

@silv-io Agreed, I really don't like it either, we're lacking proper logic to evaluate arrays 😭 I'll also look into the outputs like we've discussed (setting outputs and trying to access them).

I'll wait until #13069 is merged to rebased and also add SNS in the list of conditions if I can't make it work with the outputs. I'll re-request your review if I change the logic to use outputs!

@bentsku bentsku marked this pull request as draft September 3, 2025 22:38
@bentsku bentsku force-pushed the fix-alternative-provider-publish branch from 4ae34c4 to 17a8c54 Compare September 4, 2025 13:22
@bentsku bentsku requested a review from silv-io September 5, 2025 15:41
@bentsku bentsku marked this pull request as ready for review September 5, 2025 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexrashed alexrashed Awaiting requested review from alexrashed alexrashed is a code owner

@k-a-il k-a-il Awaiting requested review from k-a-il k-a-il is a code owner

@silv-io silv-io Awaiting requested review from silv-io silv-io is a code owner

Assignees

@bentsku bentsku

Labels
semver: patch Non-breaking changes which can be included in patch releases skip-docs Pull request does not require documentation changes
Projects
None yet
Milestone
4.8
Development

Successfully merging this pull request may close these issues.
2 participants
@bentsku @silv-io