Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Don't fail CI when a job was skipped #10529

Merged
merged 5 commits into from
Aug 5, 2021
Merged

Don't fail CI when a job was skipped #10529

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fb5f20c
Don't fail CI when a job was skipped
erikjohnston Aug 4, 2021
d27d2d0
Newsfile
erikjohnston Aug 4, 2021
8a94ea5
Negative conditions are HARD and should be BANNED
erikjohnston Aug 4, 2021
2d589f0
Look at current job status
erikjohnston Aug 4, 2021
6b05267
Allow newsfile lint to be skipped
erikjohnston Aug 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ on:
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

jobs:
lint:
runs-on: ubuntu-latest
Expand Down Expand Up @@ -374,7 +374,7 @@ jobs:
rc=0
results=$(jq -r 'to_entries[] | [.key,.value.result] | join(" ")' <<< $NEEDS_CONTEXT)
while read job result ; do
if [ "$result" != "success" ]; then
if [ "$result" != "success" ] || [ "$result" != "skipped" ]; then
richvdh marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

hrm, this looks risky.

If a job is cancelled, all the steps will be skipped. We don't really want to accept such test runs.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, looks like some stuff returns cancelled, and others skipped? I guess that's the difference between those that were already running and those that hadn't? c.f. https://github.com/matrix-org/synapse/runs/3241552601

Copy link
Member

Choose a reason for hiding this comment

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

yeah. I'm a bit worries about a race condition where the run is cancelled at just the right time so that the first set of jobs run to completion, but the second set all fail.

Could we only allow lint-newsfile to be skipped for now?

Copy link
Member Author

Choose a reason for hiding this comment

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

Could we only allow lint-newsfile to be skipped for now?

I have done this because all other options I can see are awful

echo "::set-failed ::Job $job returned $result"
rc=1
fi
Expand Down
1 change: 1 addition & 0 deletions changelog.d/10529.misc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix CI to not break when run against branches rather than pull requests.