Update Android test-command handling #2590
Conversation
|
This PR is ready for review. |
cibuildwheel/platforms/android.py
Outdated
| test_args = shlex.split(test_command) | ||
| if test_args[:2] in [["python", "-c"], ["python", "-m"]]: | ||
| test_args[:3] = [test_args[1], test_args[2], "--"] | ||
| if test_args[0] == "python" and any(arg in test_args for arg in ["-c", "-m"]): |
There was a problem hiding this comment.
Python also supports merged short args, like python3 -Om calendar. Maybe a leading python should be stripped regardless of what follows? Also, what about python3?
Also, I think this would read nicely with pattern matching:
in_test_args = shlex.split(test_command)
match in_test_args:
case ["python" | "python3", *test_args]:
pass
case ["pytest", *_]:
# We transform some commands into the `python -m` form, but this is deprecated.
msg = (
f"Test command {test_command!r} is not supported on Android. "
"cibuildwheel will try to execute it as if it started with 'python -m'. "
"If this works, all you need to do is add that to your test command."
)
log.warning(msg)
test_args = ["-m", *in_test_args]
case _:
msg = (
f"Test command {test_command!r} is not supported on Android. "
f"Command must begin with 'python' and contain '-m' or '-c'."
)
raise errors.FatalError(msg)There was a problem hiding this comment.
Python also supports merged short args, like
python3 -Om calendar.
That's a good point, but the arguments are processed by the cpython test script, which, if it doesn't find a literal -m or -c, will insert -m test at the start of the command. This would be very confusing for cibuildwheel users.
The script could be improved to detect merged short args, but then this PR would be blocked for 2 months until the next Python release.
This PR also fixes a regression in cibuildwheel 3.2.1, which updated the Android section of build-platforms.toml to Python 3.14.0 (which has the new version of the test script), but didn't include the cibuildwheel side of the changes. This has broken Android testing on Python 3.14.
To make sure any similar problems are caught in the future, I’ve updated test_expected_wheels so it not only builds on all Python versions, but also tests on them.
Also, what about
python3?
I've updated the code to accept that.
…n 3.14, and add cross venv tests
|
To avoid merge conflicts, I've also included a fix for #2611. @freakboy3742: |
| if test_args[:2] in [["python", "-c"], ["python", "-m"]]: | ||
| test_args[:3] = [test_args[1], test_args[2], "--"] | ||
| if test_args[0] in ["python", "python3"] and any(arg in test_args for arg in ["-c", "-m"]): | ||
| del test_args[0] |
There was a problem hiding this comment.
This code looks a little odd, let's comment that.
| del test_args[0] | |
| # Pass the args to be parsed by the cpython testbed script. We only do this | |
| # when '-c' or '-m' is found in the command, because without those flags, the | |
| # testbed script will prepend '-m test', which isn't what the user wants. | |
| del test_args[0] |
3 participants
In python/cpython#138805 I'm updating the Android testbed so it can take all Python command line options, not just
-cand-m. However, this will be slightly backward incompatible for cibuildwheel, because the-cor-mmust be moved after the--separator on the testbed command line. So this PR will need to remain a draft until the next releases of Python 3.13 and 3.14, which are currently scheduled for October 7.