-
Notifications
You must be signed in to change notification settings - Fork 348
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
PR check generator: add excludeOsAndVersionCombination
#2350
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,6 +27,12 @@ | |
"nightly-latest" | ||
] | ||
|
||
def is_version_and_os_excluded(version, os, exclude_params): | ||
for exclude_param in exclude_params: | ||
if exclude_param[0] == os and exclude_param[1] == version: | ||
return True | ||
return False | ||
|
||
# When updating the ruamel.yaml version here, update the PR check in | ||
# `.github/workflows/pr-checks.yml` too. | ||
header = """# Warning: This file is generated automatically, and should not be modified. | ||
|
@@ -56,27 +62,32 @@ def writeHeader(checkStream): | |
for file in (this_dir / 'checks').glob('*.yml'): | ||
with open(file, 'r') as checkStream: | ||
checkSpecification = yaml.load(checkStream) | ||
|
||
matrix = [] | ||
excludedVersionsAndOses = checkSpecification.get('excludeOsAndVersionCombination', []) | ||
for version in checkSpecification.get('versions', defaultTestVersions): | ||
runnerImages = ["ubuntu-latest", "macos-latest", "windows-latest"] | ||
if checkSpecification.get('operatingSystems', None): | ||
runnerImages = [image for image in runnerImages for operatingSystem in checkSpecification['operatingSystems'] | ||
if image.startswith(operatingSystem)] | ||
|
||
for runnerImage in runnerImages: | ||
# Prior to CLI v2.15.1, ARM runners were not supported by the build tracer. | ||
# "macos-latest" is now an ARM runner, so we run tests on the old CLIs on Intel runners instead. | ||
if version in ["stable-20230403", "stable-v2.13.4", "stable-v2.13.5", "stable-v2.14.6"] and runnerImage == "macos-latest": | ||
matrix.append({ | ||
'os': "macos-12", | ||
'version': version | ||
}) | ||
else: | ||
matrix.append({ | ||
'os': runnerImage, | ||
'version': version | ||
}) | ||
runnerImages = ["ubuntu-latest", "macos-latest", "windows-latest"] | ||
operatingSystems = checkSpecification.get('operatingSystems', ["ubuntu", "macos", "windows"]) | ||
|
||
for operatingSystem in operatingSystems: | ||
runnerImagesForOs = [image for image in runnerImages if image.startswith(operatingSystem)] | ||
|
||
for runnerImage in runnerImagesForOs: | ||
# Skip appending this combination to the matrix if it is explicitly excluded. | ||
if is_version_and_os_excluded(version, operatingSystem, excludedVersionsAndOses): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similarly here consider using the same order of OS and version as in the YAML. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, I actually misordered it myself when testing initially 😆 |
||
continue | ||
|
||
# Prior to CLI v2.15.1, ARM runners were not supported by the build tracer. | ||
# "macos-latest" is now an ARM runner, so we run tests on the old CLIs on Intel runners instead. | ||
if version in ["stable-20230403", "stable-v2.13.4", "stable-v2.13.5", "stable-v2.14.6"] and runnerImage == "macos-latest": | ||
matrix.append({ | ||
'os': "macos-12", | ||
'version': version | ||
}) | ||
else: | ||
matrix.append({ | ||
'os': runnerImage, | ||
'version': version | ||
}) | ||
|
||
useAllPlatformBundle = "false" # Default to false | ||
if checkSpecification.get('useAllPlatformBundle'): | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could rename to
excludedOsesAndVersions
for clarity since it's OSes first and versions second