Skip to content

Commit d9183f0

Browse files
committed
cirrus.yml: implement skips based on source changes
We do not have to test everything for each PR, we can know based on the source if we changed (i.e. machine code) and only run the tests then. This implements it as skip conditions, due to the nature of yaml files we unfortunately cannot deduplicate everything, i.e. the is PR check and danger files apply to everything but as skip is only a single yaml string we cannot deduplicate parts of that string. If anyone knows a way to achieve this I like to hear it. For now I implemented this for int, system, bud and machine tests. Once we are more comfortable with this I plan on adding it to other tests as well. This will replace the current _bail_if_test_can_be_skipped logic as it covers more, marks tasks actually skipped in the github UI and works even for the windows/macos machine tests. Signed-off-by: Paul Holzinger <[email protected]>
1 parent a2bf49a commit d9183f0

File tree

2 files changed

+54
-0
lines changed

2 files changed

+54
-0
lines changed

.cirrus.yml

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -639,6 +639,15 @@ local_integration_test_task: &local_integration_test_task
639639
alias: local_integration_test
640640
# Docs: ./contrib/cirrus/CIModes.md
641641
only_if: *not_tag_branch_build_docs_machine
642+
# skip when: - it is a PR (we never want to skip on nightly tests); and
643+
# - no danger files are changed; and
644+
# - when no int test code is changed; and
645+
# - NOT (source code is changed AND NOT only test files)
646+
skip: &skip_int_test >-
647+
$CIRRUS_PR != '' &&
648+
!changesInclude('.cirrus.yml', 'Makefile', 'contrib/cirrus/**', 'vendor/**', 'hack/**', 'version/rawversion/*') &&
649+
!changesInclude('test/e2e/**', 'test/utils/**') &&
650+
!(changesInclude('**/*.go', '**/*.c') && !changesIncludeOnly('test/**', 'pkg/machine/e2e/**'))
642651
depends_on: *build
643652
matrix: *platform_axis
644653
# integration tests scale well with cpu as they are parallelized
@@ -680,6 +689,7 @@ container_integration_test_task:
680689
alias: container_integration_test
681690
# Docs: ./contrib/cirrus/CIModes.md
682691
only_if: *not_tag_branch_build_docs_machine
692+
skip: *skip_int_test
683693
depends_on: *build
684694
matrix: &fedora_vm_axis
685695
- env:
@@ -708,6 +718,7 @@ rootless_integration_test_task:
708718
alias: rootless_integration_test
709719
# Docs: ./contrib/cirrus/CIModes.md
710720
only_if: *not_tag_branch_build_docs_machine
721+
skip: *skip_int_test
711722
depends_on: *build
712723
matrix: *platform_axis
713724
gce_instance: *fastvm
@@ -731,6 +742,13 @@ podman_machine_task:
731742
$CIRRUS_CHANGE_TITLE !=~ '.*CI:DOCS.*' &&
732743
$CIRRUS_CHANGE_TITLE !=~ '.*CI:BUILD.*'
733744
) || $CIRRUS_CRON == "main"
745+
# skip when: - it is a PR (we never want to skip on nightly tests); and
746+
# - no danger files are changed; and
747+
# - no machine code files are changed
748+
skip: &skip_machine_test >-
749+
$CIRRUS_PR != '' &&
750+
!changesInclude('.cirrus.yml', 'Makefile', 'contrib/cirrus/**', 'vendor/**', 'hack/**', 'version/rawversion/*') &&
751+
!changesInclude('cmd/podman/machine/**', 'pkg/machine/**', '**/*machine*.go')
734752
depends_on: *build
735753
ec2_instance:
736754
image: "${VM_IMAGE_NAME}"
@@ -752,6 +770,7 @@ podman_machine_aarch64_task:
752770
name: *std_name_fmt
753771
alias: podman_machine_aarch64
754772
only_if: *machine_cron_not_tag_build_docs
773+
skip: *skip_machine_test
755774
depends_on: *build
756775
ec2_instance:
757776
<<: *standard_build_ec2_aarch64
@@ -773,6 +792,7 @@ podman_machine_windows_task:
773792
# Only run for non-docs/copr PRs and non-release branch builds
774793
# and never for tags. Docs: ./contrib/cirrus/CIModes.md
775794
only_if: *machine_cron_not_tag_build_docs
795+
skip: *skip_machine_test
776796
depends_on: *build
777797
ec2_instance:
778798
<<: *windows
@@ -797,6 +817,7 @@ podman_machine_mac_task:
797817
name: *std_name_fmt
798818
alias: podman_machine_mac
799819
only_if: *machine_cron_not_tag_build_docs
820+
skip: *skip_machine_test
800821
depends_on: *build
801822
persistent_worker: *mac_pw
802823
env:
@@ -849,6 +870,15 @@ local_system_test_task: &local_system_test_task
849870
$CIRRUS_CHANGE_TITLE !=~ '.*CI:DOCS.*' &&
850871
$CIRRUS_CHANGE_TITLE !=~ '.*CI:BUILD.*' &&
851872
$CIRRUS_CHANGE_TITLE !=~ '.*CI:MACHINE.*'
873+
# skip when: - it is a PR (we never want to skip on nightly tests); and
874+
# - no danger files are changed; and
875+
# - no system test code is changed; and
876+
# - NOT (source code is changed AND not only test files)
877+
skip: &skip_system_test >-
878+
$CIRRUS_PR != '' &&
879+
!changesInclude('.cirrus.yml', 'Makefile', 'contrib/cirrus/**', 'vendor/**', 'hack/**', 'version/rawversion/*') &&
880+
!changesInclude('test/system/**') &&
881+
!(changesInclude('**/*.go', '**/*.c') && !changesIncludeOnly('test/**', 'pkg/machine/e2e/**'))
852882
depends_on: *build
853883
matrix: *platform_axis
854884
gce_instance: *standardvm
@@ -866,6 +896,7 @@ local_system_test_aarch64_task: &local_system_test_task_aarch64
866896
# Don't create task for tags, or if using [CI:DOCS], [CI:BUILD]
867897
# Docs: ./contrib/cirrus/CIModes.md
868898
only_if: *not_tag_magic
899+
skip: *skip_system_test
869900
depends_on: *build
870901
persistent_worker: *mac_pw
871902
ec2_instance: *standard_build_ec2_aarch64
@@ -917,6 +948,7 @@ rootless_system_test_task:
917948
alias: rootless_system_test
918949
# Docs: ./contrib/cirrus/CIModes.md
919950
only_if: *not_tag_magic
951+
skip: *skip_system_test
920952
depends_on: *build
921953
matrix: *platform_axis
922954
gce_instance: *standardvm
@@ -968,6 +1000,13 @@ buildah_bud_test_task:
9681000
alias: buildah_bud_test
9691001
# Docs: ./contrib/cirrus/CIModes.md
9701002
only_if: *not_tag_magic
1003+
# skip when: - it is a PR (we never want to skip on nightly tests); and
1004+
# - no danger files are changed; and
1005+
# - no build source files are changed and no bud tests
1006+
skip: >-
1007+
$CIRRUS_PR != '' &&
1008+
!changesInclude('.cirrus.yml', 'Makefile', 'contrib/cirrus/**', 'vendor/**', 'hack/**', 'version/rawversion/*') &&
1009+
!changesInclude('**/*build*.go', 'test/buildah-bud/**')
9711010
depends_on: *build
9721011
env:
9731012
<<: *stdenvars

contrib/cirrus/cirrus_yaml_test.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,21 @@ def test_depends(self):
6060
msg=('No success aggregation task depends_on "{0}"'.format(task_name))
6161
self.assertIn(task_name, success_deps, msg=msg)
6262

63+
def test_skips(self):
64+
"""2024-06 PR#23030: ugly but necessary duplication in skip conditions. Prevent typos or unwanted changes."""
65+
beginning = "$CIRRUS_PR != '' && !changesInclude('.cirrus.yml', 'Makefile', 'contrib/cirrus/**', 'vendor/**', 'hack/**', 'version/rawversion/*') && "
66+
real_source_changes = " && !(changesInclude('**/*.go', '**/*.c') && !changesIncludeOnly('test/**', 'pkg/machine/e2e/**'))"
67+
68+
for task_name in self.ALL_TASK_NAMES:
69+
task = self.CIRRUS_YAML[task_name + '_task']
70+
if 'skip' in task:
71+
skip = task['skip']
72+
if 'changesInclude' in skip:
73+
msg = ('{0}: invalid skip'.format(task_name))
74+
self.assertEqual(skip[:len(beginning)], beginning, msg=msg+": beginning part is wrong")
75+
if 'changesIncludeOnly' in skip:
76+
self.assertEqual(skip[len(skip)-len(real_source_changes):], real_source_changes, msg=msg+": changesIncludeOnly() part is wrong")
77+
6378
def not_task(self):
6479
"""Ensure no task is named 'task'"""
6580
self.assertNotIn('task', self.ALL_TASK_NAMES)

0 commit comments

Comments
 (0)