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

Mark one test from mostly-non-JS/JVM backends as platform-specific behaviour, test on macOS #21610

Merged
merged 20 commits into from
Nov 8, 2024

Conversation

huonw
Copy link
Contributor

@huonw huonw commented Nov 4, 2024

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 (ExternalToolm 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:

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

Copy link
Contributor

@benjyw benjyw left a comment

Choose a reason for hiding this comment

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

Part of me thinks we should run all tests on all platforms and be done with it...

@huonw
Copy link
Contributor Author

huonw commented Nov 7, 2024

Yeah, agreed: #20193 😄

@huonw huonw merged commit d28e5c3 into main Nov 8, 2024
24 checks passed
@huonw huonw deleted the huonw/more-psb branch November 8, 2024 02:30
huonw added a commit that referenced this pull request Nov 8, 2024
…ecific behaviour, test on macOS (#21620)

Continuing #21610, this adds the `platform_specific_behavior` flag to
two additional backends (hadolint and golangci_lint), that require
installing Go on the CI machines.

(I think Hadolint requires Go to build the `dockerfile` Python package
on Linux arm64.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes:not-required PR doesn't require mention in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants