Skip to content
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.

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

[TEST] skipping GA checks with labels #1498

Draft
wants to merge 113 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
113 commits
Select commit Hold shift + click to select a range
b7b4bc3
will this work
rocco8773 Mar 24, 2022
9df781f
add a skip for documentation
rocco8773 Mar 24, 2022
9737660
replace not with !
rocco8773 Mar 24, 2022
027d356
playing with conditional skipping
rocco8773 Mar 25, 2022
861b272
playing with conditional skipping
rocco8773 Mar 25, 2022
982f6af
playing with conditional skipping
rocco8773 Mar 25, 2022
49dc541
playing with conditional skipping
rocco8773 Mar 25, 2022
2e78675
playing with conditional skipping
rocco8773 Mar 25, 2022
cad80e1
playing with conditional skipping
rocco8773 Mar 25, 2022
29910ce
skip tests
rocco8773 Mar 25, 2022
22df54b
playing with skipping
rocco8773 Mar 25, 2022
a26b6fb
playing with skipping
rocco8773 Mar 25, 2022
1ed7ed2
playing with skipping
rocco8773 Mar 25, 2022
d632e7d
playing with skipping
rocco8773 Mar 25, 2022
b794529
playing with skipping
rocco8773 Mar 25, 2022
cc6dfb3
playing with skipping
rocco8773 Mar 25, 2022
bad66ce
playing with skipping
rocco8773 Mar 25, 2022
d41bd4d
playing with skipping
rocco8773 Mar 25, 2022
0b6dae7
playing with skipping
rocco8773 Mar 25, 2022
a59a8de
playing with skipping
rocco8773 Mar 25, 2022
ac0e8e2
playing with skipping
rocco8773 Mar 25, 2022
ecb3c29
playing with skipping
rocco8773 Mar 25, 2022
2555c9c
playing with skipping
rocco8773 Mar 25, 2022
7702528
playing with skipping
rocco8773 Mar 25, 2022
8f47fa3
playing with skipping
rocco8773 Mar 25, 2022
1b6550f
playing with skipping
rocco8773 Mar 25, 2022
a96d66d
playing with skipping
rocco8773 Mar 25, 2022
62ff1d7
playing with skipping
rocco8773 Mar 25, 2022
1203042
playing with skipping
rocco8773 Mar 25, 2022
d2a2d26
playing with skipping
rocco8773 Mar 25, 2022
394af81
playing with skipping
rocco8773 Mar 25, 2022
17091fd
playing with skipping
rocco8773 Mar 25, 2022
8bddbe6
playing with skipping
rocco8773 Mar 25, 2022
257f661
playing with skipping
rocco8773 Mar 25, 2022
e29d317
playing with skipping
rocco8773 Mar 25, 2022
9a5c987
playing with skipping
rocco8773 Mar 25, 2022
edb1845
playing with skipping
rocco8773 Mar 25, 2022
fd404f8
playing with skipping
rocco8773 Mar 25, 2022
3e18a3d
playing with skipping
rocco8773 Mar 25, 2022
750ff34
playing with skipping
rocco8773 Mar 25, 2022
a08cf6f
playing with skipping
rocco8773 Mar 25, 2022
5bc851a
playing with skipping
rocco8773 Mar 25, 2022
3415a92
playing with skipping
rocco8773 Mar 25, 2022
a54fb82
playing with skipping
rocco8773 Mar 25, 2022
f6d3a59
playing with skipping
rocco8773 Mar 25, 2022
481d223
playing with skipping
rocco8773 Mar 25, 2022
46f49f3
playing with skipping
rocco8773 Mar 25, 2022
c48aa7a
playing with skipping
rocco8773 Mar 25, 2022
b6c8713
playing with skipping
rocco8773 Mar 25, 2022
4a5a0de
playing with skipping
rocco8773 Mar 25, 2022
45b5351
playing with skipping
rocco8773 Mar 25, 2022
a673b43
playing with skipping
rocco8773 Mar 25, 2022
e394acd
playing with skipping
rocco8773 Mar 25, 2022
3007865
playing with skipping
rocco8773 Mar 25, 2022
5de928d
playing with skipping
rocco8773 Mar 25, 2022
9fba6b3
playing with skipping
rocco8773 Mar 25, 2022
1e45265
playing with skipping
rocco8773 Mar 25, 2022
b640871
playing with skipping
rocco8773 Mar 25, 2022
b0ee3cd
playing with skipping
rocco8773 Mar 25, 2022
35aba22
playing with skipping
rocco8773 Mar 25, 2022
bb03564
playing with skipping
rocco8773 Mar 25, 2022
1b02fbc
playing with skipping
rocco8773 Mar 25, 2022
6e84fd4
playing with skipping
rocco8773 Mar 25, 2022
eaa0494
playing with skipping
rocco8773 Mar 25, 2022
61254ac
playing with skipping
rocco8773 Mar 25, 2022
6a1e1e5
playing with skipping
rocco8773 Mar 25, 2022
29ed517
fix typo
rocco8773 Mar 25, 2022
ae446f3
PLAYING
rocco8773 Mar 25, 2022
6304e6d
PLAYING
rocco8773 Mar 25, 2022
c1e5a9e
PLAYING
rocco8773 Mar 25, 2022
f7d729d
PLAYING
rocco8773 Mar 25, 2022
a02ad93
PLAYING
rocco8773 Mar 25, 2022
6e83c02
PLAYING
rocco8773 Mar 25, 2022
42d5d70
PLAYING
rocco8773 Mar 25, 2022
17c5569
PLAYING
rocco8773 Mar 25, 2022
a2bbb92
PLAYING
rocco8773 Mar 25, 2022
466afc2
PLAYING
rocco8773 Mar 25, 2022
8cbabd0
PLAYING
rocco8773 Mar 25, 2022
c925366
PLAYING
rocco8773 Mar 25, 2022
6d5006c
PLAYING
rocco8773 Mar 25, 2022
eabafec
PLAYING
rocco8773 Mar 25, 2022
da99f18
PLAYING
rocco8773 Mar 25, 2022
7b5ce0b
PLAYING
rocco8773 Mar 25, 2022
4c9460b
PLAYING
rocco8773 Mar 25, 2022
7a5dfa8
PLAYING
rocco8773 Mar 25, 2022
232debb
PLAYING
rocco8773 Mar 25, 2022
30b9c2f
PLAYING
rocco8773 Mar 25, 2022
3b5f800
PLAYING
rocco8773 Mar 25, 2022
439f366
PLAYING
rocco8773 Mar 25, 2022
23a6e44
PLAYING
rocco8773 Mar 25, 2022
a0fe0e8
PLAYING
rocco8773 Mar 25, 2022
488b347
PLAYING
rocco8773 Mar 25, 2022
fdc7340
PLAYING
rocco8773 Mar 25, 2022
14aa200
PLAYING
rocco8773 Mar 25, 2022
d4e9046
PLAYING
rocco8773 Mar 25, 2022
067dd88
PLAYING
rocco8773 Mar 25, 2022
5951a0c
PLAYING
rocco8773 Mar 25, 2022
9f1984d
PLAYING
rocco8773 Mar 25, 2022
9f27e9f
PLAYING
rocco8773 Mar 25, 2022
0ee36a1
PLAYING
rocco8773 Mar 25, 2022
eb2ebff
PLAYING
rocco8773 Mar 25, 2022
e912c5b
PLAYING
rocco8773 Mar 25, 2022
e4d3a35
PLAYING
rocco8773 Mar 25, 2022
050d3ec
PLAYING
rocco8773 Mar 25, 2022
6dbd14a
PLAYING
rocco8773 Mar 25, 2022
7f52a9d
PLAYING
rocco8773 Mar 25, 2022
ed4308b
PLAYING
rocco8773 Mar 25, 2022
a8ec2d4
PLAYING
rocco8773 Mar 25, 2022
0e641a8
PLAYING
rocco8773 Mar 25, 2022
dcc9b94
PLAYING
rocco8773 Mar 25, 2022
1f2bfa2
PLAYING
rocco8773 Mar 25, 2022
9dabe81
PLAYING
rocco8773 Mar 25, 2022
a28abb3
PLAYING
rocco8773 Mar 25, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/labeler.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ on:

jobs:
triage:
if: ${{ !(contains( github.event.pull_request_target.labels.*.name, 'GA Skip Labeler')) }}
runs-on: ubuntu-latest
steps:
- name: Add Labels to PR
Expand Down
82 changes: 80 additions & 2 deletions .github/workflows/testing.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,82 @@ on:
tags:
- v*
pull_request:
types:
- opened
- reopened
- synchronize
- labeled
- unlabeled
workflow_dispatch:

jobs:
skips-allowed:
name: Determine CI Skipping
runs-on: ubuntu-latest

env:
IS_PR: ${{ github.event_name == 'pull_request' }}
SKIP_DOCUMENTATION_LABEL: 'GA Skip Documentation'
SKIP_COMPREHENSIVE_LABEL: 'GA Skip Comprehensive Tests'
SKIPS_ALLOWED_LABEL: 'GA Skip Merge Allowed'

SKIP_DOCUMENTATION: false
SKIP_COMPREHENSIVE: false
SKIPS_ALLOWED: false

SKIPS_DONE: false

outputs:
SKIP_DOCUMENTATION: ${{ env.SKIP_DOCUMENTATION }}
SKIP_COMPREHENSIVE: ${{ env.SKIP_COMPREHENSIVE }}
SKIPS_ALLOWED: ${{ env.SKIPS_ALLOWED }}
SKIPS_DONE: ${{ env.SKIPS_DONE }}
Comment on lines +38 to +41
Copy link
Member

Choose a reason for hiding this comment

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

so if I understand it correctly, this skips-allowed job does the logic, puts the result into env vars, and this outputs field, which I'm seeing for the first time, lets you pass those env vars to the next stage. That's reasonable.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep. I originally tries this by defining env on the workflow level, but it turns out the workflow level env just acts as an initialization for the job level env. So, if I modify an environment variable in job1 then that doesn't carry over to job2. The only way I've found that I can pass this information between jobs is using outputs for the job.


steps:
- name: Determine Skips
id: step1
if: ${{ env.IS_PR }}
StanczakDominik marked this conversation as resolved.
Show resolved Hide resolved
run: |
SKIP=${{ contains(github.event.pull_request.labels.*.name, env.SKIP_DOCUMENTATION_LABEL) }}
echo "SKIP_DOCUMENTATION=${SKIP}" >> $GITHUB_ENV

SKIP=${{ contains(github.event.pull_request.labels.*.name, env.SKIP_COMPREHENSIVE_LABEL) }}
echo "SKIP_COMPREHENSIVE=${SKIP}" >> $GITHUB_ENV

SKIP=${{ contains(github.event.pull_request.labels.*.name, env.SKIPS_ALLOWED_LABEL) }}
echo "SKIPS_ALLOWED=${SKIP}" >> $GITHUB_ENV

- name: Skip Documentation? ${{ env.SKIP_DOCUMENTATION }}
run: |
echo "Documentation can be skipped: ${{ env.SKIP_DOCUMENTATION }}"
if ${{ env.SKIP_DOCUMENTATION }}; then
echo "SKIPS_DONE=true" >> $GITHUB_ENV
fi
- name: Skip Comprehensive Tests? ${{ env.SKIP_COMPREHENSIVE }}
run: |
echo "Comprehensive tests can be skipped: ${{ env.SKIP_COMPREHENSIVE }}"
if ${{ env.SKIP_COMPREHENSIVE }}; then
echo "SKIPS_DONE=true" >> $GITHUB_ENV
fi

mergeble-with-skips:
name: Skipping and Merge Allowed?
runs-on: ubuntu-latest
needs: skips-allowed

env:
SKIPS_ALLOWED: ${{ needs.skips-allowed.outputs.SKIPS_ALLOWED }}
SKIPS_DONE: ${{ needs.skips-allowed.outputs.SKIPS_DONE }}

steps:
- name: Skip Tests Allowed? ${{ env.SKIPS_ALLOWED }}
run: |
echo "Is skipping tests allowed: ${{ env.SKIPS_ALLOWED }}"
if [[ "$SKIPS_ALLOWED" == "false" && "$SKIPS_DONE" == "true" ]]; then
echo "Some tests are skipped, but skipping needs to be removed before merging!!"
Copy link
Member

Choose a reason for hiding this comment

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

I'd love to make this message more actionable by doing something like this:

Suggested change
echo "Some tests are skipped, but skipping needs to be removed before merging!!"
echo "Some tests are skipped - either unskip them before merging or add $SKIPS_ALLOWED_LABEL if you're willing to live with the consequences!!"

but I expect it won't work because SKIPS_ALLOWED_LABEL is "scoped" locally in another job... right?

Copy link
Member Author

Choose a reason for hiding this comment

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

I believe this would work. I originally didn't write something like this because I only wanted us to know about the "skips allows" functionality. Only we should be adding that label and not the common contributor.

exit 1
fi

initial-tests:
name: ${{ matrix.name }}
runs-on: ${{ matrix.os }}
Expand Down Expand Up @@ -46,7 +119,10 @@ jobs:
comprehensive-tests:
name: ${{ matrix.name }}
runs-on: ${{ matrix.os }}
needs: initial-tests

if: ${{ needs.skips-allowed.outputs.SKIP_COMPREHENSIVE == 'false' }}
needs: [initial-tests, skips-allowed]
Comment on lines +123 to +124
Copy link
Member

Choose a reason for hiding this comment

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

So if SKIP_COMPREHENSIVE is false, this needs both of these... oh, no, right, this runs conditionally only if that var is false, correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct


strategy:
fail-fast: false
matrix:
Expand Down Expand Up @@ -78,7 +154,6 @@ jobs:
python: 3.9
toxenv: py39


steps:
- name: Checkout code
uses: actions/checkout@v3
Expand All @@ -101,6 +176,8 @@ jobs:
with:
file: ./coverage.xml
documentation:
if: ${{ needs.skips-allowed.outputs.SKIP_DOCUMENTATION == 'false' }}
needs: skips-allowed
Copy link
Member

Choose a reason for hiding this comment

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

What I don't understand is... does this only run if you're enabling skips-allowed via GA Skip Merge Allowed? Suppose I don't skip anything, does this still run?

Copy link
Member Author

Choose a reason for hiding this comment

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

The skips-allowed job always runs regardless of labeling, and will never fail. I breakout that "merge-able failure" into a separate job since if it fails any dependent jobs would automatically skip.

Since the outputs are defined using the job's env, the outputs are automatically initialized with the env values.

name: Documentation
runs-on: ubuntu-latest
strategy:
Expand All @@ -120,6 +197,7 @@ jobs:
run: sudo apt-get install graphviz pandoc
- name: Run tests
run: tox -e build_docs -- -q

build-n-publish:
name: Packaging
runs-on: ubuntu-18.04
Expand Down