Skip to content

Test enable groups as specified by PR labels #2357

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

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open

Conversation

joerick
Copy link
Contributor

@joerick joerick commented Apr 13, 2025

Don't run all enable groups on PRs, but allow them to be enabled using PR labels.

This is mostly to keep the PR builds fast. As noted in #1538 (comment) , extra enable groups affect the CI times a lot.

The only CI affected by the labels is Github Actions. The others just run the default set, which probably isn't ideal, but I can't think of another way to do it!

joerick added 6 commits April 13, 2025 21:15
This was mostly for use in the `main` building case, because otherwise it's maybe a bit too easy to forget to update this file when adding an enable group
@joerick joerick mentioned this pull request Apr 13, 2025
@henryiii
Copy link
Contributor

When I've added this sort of thing in the past, it wouldn't trigger the expanded tests on the first push (since there's no label there), and then you have to add the label then push something to get the expanded testing.

@joerick
Copy link
Contributor Author

joerick commented Apr 15, 2025

I did set it to trigger on 'labelled', so hopefully that shouldn't be a problem? Once I get these tests to pass we can play with it.

@joerick joerick added the CI: PyPy Run the integration test suite with PyPy included label Apr 20, 2025
@joerick
Copy link
Contributor Author

joerick commented Apr 20, 2025

Cool, I added the PyPy tag and the tests reran. I'll update this table when it finishes:

Test no labels pypy
Get qemu emulated architectures (pull_request) 8s 6s
Linters (mypy, flake8, etc.) (pull_request) 1m 51s
Test cibuildwheel building Pyodide wheels (pull_request) 5m 5m
Test Linux aarch64 using qemu (pull_request) 17m 17m
Test Linux armv7l using qemu (pull_request) 16m 16m
Test Linux ppc64le using qemu (pull_request) 15m 15m
Test Linux s390x using qemu (pull_request) 18m 18m
Test on macos-13 (3.13) (pull_request) 23m 30m
Test on macos-15 (3.13) (pull_request) 15m 19m
Test on ubuntu-24.04-arm (3.13) (pull_request) 21m 22m
Test on ubuntu-latest (3.11) (pull_request) 31m 36m
Test on ubuntu-latest (3.13) (pull_request) 30m 36m
Test on windows-11-arm (3.13) (pull_request) 15m 14m
Test on windows-latest (3.13) (pull_request) 30m 41m

@joerick
Copy link
Contributor Author

joerick commented Apr 20, 2025

Looks like it's working based on the above table!

The remaining question is what to do with the other CIs. Perhaps the right approach is to run everything on main, but just the core set in PRs. That way we'll catch those incompatibilities eventually.

@joerick joerick marked this pull request as ready for review April 21, 2025 21:41
@joerick
Copy link
Contributor Author

joerick commented Apr 26, 2025

That's done. This is ready to review.

@henryiii
Copy link
Contributor

Should we merge #1538 first, then add graalpy here?

Also, I'd like to remove the deprecated PyPy's or put then behind a separate enable group. The original reason to add an enable group was to avoid depreciated PyPy's, but since it enables both current and deprecated PyPy's, it's not helpful for the original use case.

@joerick
Copy link
Contributor Author

joerick commented Apr 28, 2025

Should we merge #1538 first, then add graalpy here?

Yeah, that would make sense to me.

Also, I'd like to remove the deprecated PyPy's or put then behind a separate enable group.

Sounds good to me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: PyPy Run the integration test suite with PyPy included
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants