-
Notifications
You must be signed in to change notification settings - Fork 284
Revert to running tests from from a temp dir when test-sources is unset #2420
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
Conversation
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 restores the v2.x behavior of running tests from a temporary directory when test-sources
is unset, replaces in-place test runs with a helpful failure file, and removes the prior iOS-only requirement for test-sources
by using the {project}
placeholder to detect misconfiguration.
- Introduces a
test_fail.py
resource that is written into the temp test directory to surface clear errors when no test sources are provided. - Updates nearly all test suites and documentation to use the
{project}
placeholder (e.g.,pytest {project}/tests
) instead of running tests in-place. - Refines iOS logic to allow unset
test-sources
by writing the failure file or checking for placeholders and raising a fatal error.
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
test/test_testing.py | Updated test commands to use {project}/test |
test/test_pyodide.py | Inject {project} placeholder into pytest invocation |
test/test_macos_archs.py | Removed default in-project test run; now temp-dir-only behavior |
test/test_ios.py | Added iOS-specific failure and placeholder checks |
test/test_emulation.py | Switched to {project}/test in emulation tests |
test/test_before_test.py | Updated before-test hook to use placeholder path |
test/test_abi_variants.py | Adjusted ABI variant tests to use {project}/test |
docs/options.md | Revised examples to show {project} usage |
docs/configuration.md | Updated CI snippets for {project} in test commands |
cibuildwheel/util/resources.py | Added constant for the new failure file resource |
cibuildwheel/resources/testing_temp_dir_file.py | New failure file template for test cwd errors |
cibuildwheel/platforms/windows.py | Switched to temp test cwd and failure file write |
cibuildwheel/platforms/pyodide.py | Mirrored temp-dir logic and failure file write |
cibuildwheel/platforms/macos.py | Aligned macOS temp test cwd and failure file integration |
cibuildwheel/platforms/linux.py | Added Linux temp-dir and failure file copy |
cibuildwheel/platforms/ios.py | Removed hard iOS test-sources error and added placeholder check |
Comments suppressed due to low confidence (3)
test/test_macos_archs.py:45
- Under the new temp-dir behavior, this test command neither sets
test-sources
nor uses the{project}
placeholder, so it will trigger the failure file rather than run successfully. Consider adding{project}
to the path or definingtest-sources
so the test continues to pass.
"CIBW_TEST_COMMAND": '''python -c "import platform; print('running tests on ' + platform.machine())"''',
test/test_ios.py:95
- This test expects to capture a failure from
utils.cibuildwheel_run
, but it isn’t wrapped inpytest.raises
. If the run raises an exception, the assertions will never execute. Wrap the call inwith pytest.raises(subprocess.CalledProcessError):
.
def test_no_test_sources(tmp_path, capfd):
cibuildwheel/platforms/linux.py:414
- There isn’t a dedicated test verifying that Linux builds correctly surface the failure file when
test-sources
is unset. Adding a test for this scenario would ensure the new temp-dir error behavior is covered.
container.copy_into(resources.TEST_FAIL_CWD_FILE, test_cwd / "test_fail.py")
Thanks for sorting the tests! |
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.
If you want to get this in and make a dev release (beta or rc), I can try it out in a couple of places (like NumPy with and without test sources).
I'll cut a new beta release now. |
...after updating dependencies to get v3.14.0b2! |
Fix #2406.
Alternative to #2419.
This approach restores the v2.x behaviour of running tests from a temp directory. That resolves the issues noted in #2406.
This also removes the ambiguity from #2363 (comment) , so I've removed the restriction that iOS requires test-sources to be set - in this world, we can use the inclusion of the
{project}
placeholder intest-command
as a signal that users haven't configured things correctly for iOS. Failing that, thetest_fail.py
script that a test runner (pytest) will find can prompt the user to usetest-sources
.