From 130a84f32da9e1bdba04eaf51e9e7a879f5e5cea Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Thu, 29 May 2025 12:46:08 -0400 Subject: [PATCH 1/4] feat: support multiple commands on iOS Signed-off-by: Henry Schreiner --- cibuildwheel/platforms/ios.py | 94 +++++++++++++++++++---------------- test/test_ios.py | 5 +- 2 files changed, 55 insertions(+), 44 deletions(-) diff --git a/cibuildwheel/platforms/ios.py b/cibuildwheel/platforms/ios.py index 660af67c1..90a1cad7c 100644 --- a/cibuildwheel/platforms/ios.py +++ b/cibuildwheel/platforms/ios.py @@ -7,7 +7,7 @@ import subprocess import sys import textwrap -from collections.abc import Sequence, Set +from collections.abc import Iterator, Sequence, Set from pathlib import Path from typing import assert_never @@ -42,6 +42,17 @@ from .macos import install_cpython as install_build_cpython +def split_command(lst: list[str]) -> Iterator[list[str]]: + items = list[str]() + for item in lst: + if item == "&&": + yield items + items = [] + else: + items.append(item) + yield items + + @dataclasses.dataclass(frozen=True, kw_only=True) class PythonConfiguration: version: str @@ -618,14 +629,15 @@ def build(options: Options, tmp_path: Path) -> None: ) raise errors.FatalError(msg) - test_command_parts = shlex.split(build_options.test_command) - if test_command_parts[0:2] != ["python", "-m"]: - first_part = test_command_parts[0] - if first_part == "pytest": - # pytest works exactly the same as a module, so we - # can just run it as a module. - log.warning( - unwrap_preserving_paragraphs(f""" + test_command_list = shlex.split(build_options.test_command) + for test_command_parts in split_command(test_command_list): + match test_command_parts: + case ["python", "-m", *rest]: + final_command = rest + case ["pytest", *rest]: + # pytest works exactly the same as a module, so we + # can just run it as a module. + msg = unwrap_preserving_paragraphs(f""" iOS tests configured with a test command which doesn't start with 'python -m'. iOS tests must execute python modules - other entrypoints are not supported. @@ -636,43 +648,39 @@ def build(options: Options, tmp_path: Path) -> None: Test command: {build_options.test_command!r} """) + log.warning(msg) + final_command = ["pytest", *rest] + case _: + msg = unwrap_preserving_paragraphs( + f""" + iOS tests configured with a test command which doesn't start + with 'python -m'. iOS tests must execute python modules - other + entrypoints are not supported. + + Test command: {build_options.test_command!r} + """ + ) + raise errors.FatalError(msg) + + try: + call( + "python", + testbed_path, + "run", + *(["--verbose"] if build_options.build_verbosity > 0 else []), + "--", + *final_command, + env=test_env, ) - else: - msg = unwrap_preserving_paragraphs( - f""" - iOS tests configured with a test command which doesn't start - with 'python -m'. iOS tests must execute python modules - other - entrypoints are not supported. - - Test command: {build_options.test_command!r} - """ - ) - raise errors.FatalError(msg) - else: - # the testbed run command actually doesn't want the - # python -m prefix - it's implicit, so we remove it - # here. - test_command_parts = test_command_parts[2:] - - try: - call( - "python", - testbed_path, - "run", - *(["--verbose"] if build_options.build_verbosity > 0 else []), - "--", - *test_command_parts, - env=test_env, - ) - failed = False - except subprocess.CalledProcessError: - failed = True + failed = False + except subprocess.CalledProcessError: + failed = True - log.step_end(success=not failed) + log.step_end(success=not failed) - if failed: - log.error(f"Test suite failed on {config.identifier}") - sys.exit(1) + if failed: + log.error(f"Test suite failed on {config.identifier}") + sys.exit(1) # We're all done here; move it to output (overwrite existing) if compatible_wheel is None: diff --git a/test/test_ios.py b/test/test_ios.py index 3d1674585..55d257c60 100644 --- a/test/test_ios.py +++ b/test/test_ios.py @@ -86,7 +86,7 @@ def test_ios_platforms(tmp_path, build_config, monkeypatch, capfd): "CIBW_BUILD": "cp313-*", "CIBW_XBUILD_TOOLS": "does-exist", "CIBW_TEST_SOURCES": "tests", - "CIBW_TEST_COMMAND": "python -m unittest discover tests test_platform.py", + "CIBW_TEST_COMMAND": "python -m this && python -m unittest discover tests test_platform.py", "CIBW_BUILD_VERBOSITY": "1", **build_config, }, @@ -102,6 +102,9 @@ def test_ios_platforms(tmp_path, build_config, monkeypatch, capfd): captured = capfd.readouterr() assert "'does-exist' will be included in the cross-build environment" in captured.out + # Make sure the first command ran + assert "Zen of Python" in captured.out + @pytest.mark.serial def test_no_test_sources(tmp_path, capfd): From d73d7132c48315eca40ef597856d8b6db8e627fe Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Fri, 30 May 2025 12:19:07 +0100 Subject: [PATCH 2/4] Move split_command into a util module --- cibuildwheel/platforms/ios.py | 15 ++------------- cibuildwheel/util/cmd.py | 17 ++++++++++++++++- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/cibuildwheel/platforms/ios.py b/cibuildwheel/platforms/ios.py index 90a1cad7c..67e59c5b1 100644 --- a/cibuildwheel/platforms/ios.py +++ b/cibuildwheel/platforms/ios.py @@ -7,7 +7,7 @@ import subprocess import sys import textwrap -from collections.abc import Iterator, Sequence, Set +from collections.abc import Sequence, Set from pathlib import Path from typing import assert_never @@ -25,7 +25,7 @@ from ..options import Options from ..selector import BuildSelector from ..util import resources -from ..util.cmd import call, shell +from ..util.cmd import call, shell, split_command from ..util.file import ( CIBW_CACHE_PATH, copy_test_sources, @@ -42,17 +42,6 @@ from .macos import install_cpython as install_build_cpython -def split_command(lst: list[str]) -> Iterator[list[str]]: - items = list[str]() - for item in lst: - if item == "&&": - yield items - items = [] - else: - items.append(item) - yield items - - @dataclasses.dataclass(frozen=True, kw_only=True) class PythonConfiguration: version: str diff --git a/cibuildwheel/util/cmd.py b/cibuildwheel/util/cmd.py index fcf725819..a528480e7 100644 --- a/cibuildwheel/util/cmd.py +++ b/cibuildwheel/util/cmd.py @@ -4,7 +4,7 @@ import subprocess import sys import typing -from collections.abc import Mapping +from collections.abc import Iterator, Mapping from typing import Final, Literal from ..errors import FatalError @@ -81,3 +81,18 @@ def shell( command = " ".join(commands) print(f"+ {command}") subprocess.run(command, env=env, cwd=cwd, shell=True, check=True) + + +def split_command(lst: list[str]) -> Iterator[list[str]]: + """ + Split a shell-style command, as returned by shlex.split, into a sequence + of commands, separated by '&&'. + """ + items = list[str]() + for item in lst: + if item == "&&": + yield items + items = [] + else: + items.append(item) + yield items From 5384fd83fc5d33562615bbb74264089e721db029 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Fri, 30 May 2025 12:19:33 +0100 Subject: [PATCH 3/4] (unrelated) fix test docstring --- test/test_ios.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/test_ios.py b/test/test_ios.py index 55d257c60..a7f8c3956 100644 --- a/test/test_ios.py +++ b/test/test_ios.py @@ -137,7 +137,10 @@ def test_no_test_sources(tmp_path, capfd): def test_ios_testing_with_placeholder(tmp_path, capfd): - """Build will run tests with the {project} placeholder.""" + """ + Tests with the {project} placeholder are not supported on iOS, because the test command + is run in the simulator. + """ skip_if_ios_testing_not_supported() project_dir = tmp_path / "project" From 5823596d7a3b04d88c3c64608b0aede220b2dd4b Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Fri, 30 May 2025 12:21:18 +0100 Subject: [PATCH 4/4] Implement short-circuit behaviour on test-command --- cibuildwheel/platforms/ios.py | 71 +++++++++++++++++------------------ test/test_ios.py | 30 +++++++++++++++ 2 files changed, 65 insertions(+), 36 deletions(-) diff --git a/cibuildwheel/platforms/ios.py b/cibuildwheel/platforms/ios.py index 67e59c5b1..19ef04186 100644 --- a/cibuildwheel/platforms/ios.py +++ b/cibuildwheel/platforms/ios.py @@ -619,39 +619,39 @@ def build(options: Options, tmp_path: Path) -> None: raise errors.FatalError(msg) test_command_list = shlex.split(build_options.test_command) - for test_command_parts in split_command(test_command_list): - match test_command_parts: - case ["python", "-m", *rest]: - final_command = rest - case ["pytest", *rest]: - # pytest works exactly the same as a module, so we - # can just run it as a module. - msg = unwrap_preserving_paragraphs(f""" - iOS tests configured with a test command which doesn't start - with 'python -m'. iOS tests must execute python modules - other - entrypoints are not supported. - - 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. - - Test command: {build_options.test_command!r} - """) - log.warning(msg) - final_command = ["pytest", *rest] - case _: - msg = unwrap_preserving_paragraphs( - f""" + try: + for test_command_parts in split_command(test_command_list): + match test_command_parts: + case ["python", "-m", *rest]: + final_command = rest + case ["pytest", *rest]: + # pytest works exactly the same as a module, so we + # can just run it as a module. + msg = unwrap_preserving_paragraphs(f""" iOS tests configured with a test command which doesn't start with 'python -m'. iOS tests must execute python modules - other entrypoints are not supported. + 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. + Test command: {build_options.test_command!r} - """ - ) - raise errors.FatalError(msg) + """) + log.warning(msg) + final_command = ["pytest", *rest] + case _: + msg = unwrap_preserving_paragraphs( + f""" + iOS tests configured with a test command which doesn't start + with 'python -m'. iOS tests must execute python modules - other + entrypoints are not supported. + + Test command: {build_options.test_command!r} + """ + ) + raise errors.FatalError(msg) - try: call( "python", testbed_path, @@ -661,15 +661,14 @@ def build(options: Options, tmp_path: Path) -> None: *final_command, env=test_env, ) - failed = False - except subprocess.CalledProcessError: - failed = True - - log.step_end(success=not failed) - - if failed: - log.error(f"Test suite failed on {config.identifier}") - sys.exit(1) + except subprocess.CalledProcessError: + # catches the first test command failure in the loop, + # implementing short-circuiting + log.step_end(success=False) + log.error(f"Test suite failed on {config.identifier}") + sys.exit(1) + + log.step_end() # We're all done here; move it to output (overwrite existing) if compatible_wheel is None: diff --git a/test/test_ios.py b/test/test_ios.py index a7f8c3956..b2d7e892a 100644 --- a/test/test_ios.py +++ b/test/test_ios.py @@ -165,6 +165,36 @@ def test_ios_testing_with_placeholder(tmp_path, capfd): assert "iOS tests cannot use placeholders" in captured.out + captured.err +@pytest.mark.serial +def test_ios_test_command_short_circuit(tmp_path, capfd): + skip_if_ios_testing_not_supported() + + project_dir = tmp_path / "project" + basic_project = test_projects.new_c_project() + basic_project.files.update(basic_project_files) + basic_project.generate(project_dir) + + with pytest.raises(subprocess.CalledProcessError): + # `python -m not_a_module` will fail, so `python -m this` should not be run. + utils.cibuildwheel_run( + project_dir, + add_env={ + "CIBW_PLATFORM": "ios", + "CIBW_BUILD": "cp313-*", + "CIBW_XBUILD_TOOLS": "", + "CIBW_TEST_SOURCES": "tests", + "CIBW_TEST_COMMAND": "python -m not_a_module && python -m this", + "CIBW_BUILD_VERBOSITY": "1", + }, + ) + + captured = capfd.readouterr() + + assert "No module named not_a_module" in captured.out + captured.err + # assert that `python -m this` was not run + assert "Zen of Python" not in captured.out + captured.err + + def test_missing_xbuild_tool(tmp_path, capfd): """Build will fail if xbuild-tools references a non-existent tool.""" skip_if_ios_testing_not_supported()