Mark one test from docker backend as platform-specific behaviour #21621
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
huonw
added
the
release-notes:not-required
|
actions/runner#1456 might have some useful tips for how to install docker properly, although might be best to wait for #21623 and then the switch back to GitHub-hosted runners (in 2.25) before landing this. |
huonw
added a commit
that referenced
this pull request
Nov 8, 2024
…haviour, test on macOS (#21610) This goes through many backends and marks one test as `platform_specific_behavior` to ensure the backends have some testing on platforms other than x86-64 Linux (macOS, and arm64 Linux). The particular value in doing this is validating the set-up code, e.g. got the right configuration for installing/downloading the tool, rather than validating the details of the behaviour. I think we generally assume most tools behave similar across platforms, once they're up and running. This PR focuses on non-JVM and non-JS backends, meaning raw binaries (`ExternalTool`m `TemplatedExternalTool`) and Python tools installed from PyPI (`PythonToolBase`, `PythonToolRequirementsBase`). It particularly focuses on the backends that don't require installing any additional software in CI, see #21620 and #21621 for additional backends that do require installing Go and Docker respectively. The impression I get is the JVM and JS tools are more platform-independent, often? For instance, their lockfiles/artifacts don't mention specific platforms, unlike Python wheels. I found these backends via the following shell command, plus other exploration: ```shell for file in $(rg --files-with-matches "class.*(PythonTool.*Base|ExternalTool)" --glob '*.py' . | sed 's@/[^/]*$@@' | sort | uniq); do rg -q "platform_specific_behavior" $file || echo $file done ``` I may've missed some! This increases the time to run tests in CI on the relevant platforms, by slight more than 2×. | platform | before | after | |--------------|------------------------|---------------| | Linux x86-64 | runs all tests always | no change | | Linux arm64 | ~3:30 | ~7:30 | | macOS x86-64 | ~5:30 | 12:00 - 13:30 | | macOS arm64 | not run in CI (#20993) | no change | Related: #21608, #20193, #20993
Said PR landed, is this good to un-DRAFT? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Continuing #21610 and #21620.