-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
GHA: Test MSYS2/CLANG64 #6888
base: main
Are you sure you want to change the base?
GHA: Test MSYS2/CLANG64 #6888
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
I've rerun the tests, and they've passed now. |
Thank you for this PR! What's the benefit in adding all of these environments? Our build matrix is pretty big right now, so let's be careful not to make it too much bigger. This increases the test jobs in this worfklow from 2 to 5, and total duration from ~12 to ~21 mins: That's on top of 61 checks at the moment (mostly but not all GHA), taking about 184 mins of GHA time (wall-clock time maybe about an hour?). For example, we aren't testing the GHA macOS/Ubuntu jobs with both architectures, and we will need to add some 3.12-dev coverage soonish. Not opposed to this PR, and I'm very much for increasing test coverage, but let's keep an eye on things so they stay manageable :) |
IMHO testing MINGW64 (GCC toolchain w/ legacy MSVCRT) and CLANG64 (LLVM toolchain w/ modern UCRT) should provide decent coverage if one is trying to reduce the number of jobs. There's little point in testing 32-bit, support for it is being phased out anyway... |
I've had a hardware issue preventing me from working on this until now...
I was going to suggest something similar to @kmilos's comment. I've now updated the test matrix to (and listed the others in a comment):
|
Any particular reason for not using the setup-msys2 action and all this extra manual work? |
IIRC this was made before the setup-msys2 action, and it seems like less work to just update it as is instead of switching to the setup-msys2 action. |
I think you'll find switching will pay off when it comes to flexibility and long term maintanance of these jobs. I might be able to help with this when I find some time. |
MSYS2 now has more environments: https://www.msys2.org/docs/environments/
Changes proposed in this pull request:
instead of MINGW32, GHA: Test MSYS2/CLANG64 #6888 (comment)Add xfail mark for Flaky test: test_qt_image_qapplication.py::test_sanity #6875 as it failed 9 times in a row.ci
scripts simplifies it a bit.