-
Notifications
You must be signed in to change notification settings - Fork 278
ci: rework tests to break out ios/android tests #2519
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
base: main
Are you sure you want to change the base?
Conversation
What do you think about also introducing flaky on the ios tests? |
We could, or there's an action that reruns on failure; actually I see at least two:
I've used one of those before, not sure which one. |
0f0c1de
to
442ca72
Compare
94f43bb
to
2c8868f
Compare
Signed-off-by: Henry Schreiner <[email protected]>
Co-authored-by: Copilot <[email protected]>
Signed-off-by: Henry Schreiner <[email protected]>
Signed-off-by: Henry Schreiner <[email protected]>
Signed-off-by: Henry Schreiner <[email protected]>
2c8868f
to
58e8df5
Compare
Signed-off-by: Henry Schreiner <[email protected]>
Signed-off-by: Henry Schreiner <[email protected]>
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.
Pull Request Overview
This PR restructures the CI test workflow to separate iOS and Android tests into dedicated test runs, while maintaining existing coverage for other platforms. The changes enable platform-specific testing by introducing a platform matrix parameter and updating test configuration to handle different build frontends.
- Split test matrix to explicitly separate iOS/Android from native platform tests
- Updated test fixtures to intercept build calls for Android and iOS platforms
- Modified CI workflow to conditionally run certain steps based on platform type
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
unit_test/main_tests/conftest.py | Added Android and iOS platform imports and build interception for testing |
.github/workflows/test.yml | Restructured test matrix to separate platform-specific runs and added conditional logic for different platforms |
Signed-off-by: Henry Schreiner <[email protected]>
e030ea3
to
c0f0391
Compare
a33c655
to
ad42eb3
Compare
Signed-off-by: Henry Schreiner <[email protected]>
ad42eb3
to
d6cf2cf
Compare
Weird, android is getting stuck in the unit_tests and hanging, while that should be identical to native. And macOS android is failing to download. Not sure what is causing this yet, though. |
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.
The android unit test is probably failing due to something related to Docker - the docker tests are selected on that runner but binfmt isn't installed. I think my suggestion would be to not run the docker unit tests when testing android.
I haven't been able to figure out why the macOS android build is failing on Azure though.
parser.add_argument( | ||
"--platform", | ||
choices={"all", "native", "android", "ios", "pyodide"}, | ||
default="all", | ||
help="Either 'native' or 'android'/'ios'/'pyodide'", | ||
) |
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.
Given we already have sys.platform and CIBW_PLATFORM changing the behaviour in this script, having another thing called "platform" is confusing me! Maybe we call this --test-select
instead?
@mhsmith there's a test failure here that you might be interested in? On Azure pipelines, macOS x86_64, the android runner seems to fail while installing the android ndk. The error looks like this:
It seems to just stop unzipping the archive and error out. The output's a bit cluttered because pytest is capturing it, I'll try to make it easier to read... |
Could the runner be running out of disk space while unzipping? I hit that problem on GHA, though I think the symptoms were different. I worked around it here: cibuildwheel/.github/workflows/test.yml Lines 74 to 75 in 3ebb6bd
|
Ah, it looks like I might have accidentally worked around it! I moved the first android test into the serial phase of the tests and it disappeared. So I'm thinking that maybe the issue was that the |
Fail fast disabled, and trying to split out the android/ios runs.
The total time shouldn't be that much more than before, need to verify (a minute or two on sample builds is fine).