-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Fix "🐍 3 • windows-latest • mingw64" CI job; test_gil_scoped.py xfail macOS free-threading #5822
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: master
Are you sure you want to change the base?
Conversation
…2@v2 cannot be run twice anymore): https://github.com/pybind/pybind11/actions/runs/17394902023/job/49417376616?pr=5796 ``` Run msys2/setup-msys2@v2 with: msystem: mingw64 install: mingw-w64-x86_64-python-numpy mingw-w64-x86_64-python-scipy mingw-w64-x86_64-eigen3 path-type: minimal update: false pacboy: false release: true location: RUNNER_TEMP platform-check-severity: fatal cache: true env: PYTHONDEVMODE: 1 PIP_BREAK_SYSTEM_PACKAGES: 1 PIP_ONLY_BINARY: numpy FORCE_COLOR: 3 PYTEST_TIMEOUT: 300 VERBOSE: 1 CMAKE_COLOR_DIAGNOSTICS: 1 MSYSTEM: MINGW64 Error: Trying to install MSYS2 to D:\a\_temp\msys64 but that already exists, cannot continue. ```
Hi @henryiii, could you please review? This should stop the frequent CI / 🐍 (macos-13, 3.13t, -DCMAKE_CXX_STANDARD=11) / 🧪 (pull_request) failures. |
if env.WIN and env.PYPY: | ||
pytest.xfail("[TEST-GIL-SCOPED] Windows PyPy: " + msg) | ||
elif env.MACOS and not env.sys_is_gil_enabled(): | ||
pytest.xfail("[TEST-GIL-SCOPED] macOS free-threading: " + msg) |
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.
This doesn't always fail, does it? So strict=False
?
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 strict
flag only applies to @pytest.mark.xfail(...)
(the decorator/mark form). Inline pytest.xfail()
doesn't take strict
at all — it immediately aborts the test and reports XFAIL
. There’s no concept of XPASS
in this case, so strict
isn't relevant here.
To be sure I looked it up the good-old way:
https://docs.pytest.org/en/stable/how-to/skipping.html#xfail-mark-test-functions-as-expected-to-fail
Note that no other code is executed after the pytest.xfail() call, differently from the marker.
if env.PYPY and env.WIN: | ||
pytest.skip(msg) | ||
if env.WIN and env.PYPY: | ||
pytest.xfail("[TEST-GIL-SCOPED] Windows PyPy: " + msg) |
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.
This didn't run before, now it will. Guessing that's fine?
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.
See other comment: Note that no other code is executed after the pytest.xfail()
call, differently from the marker.
.github/workflows/ci.yml
Outdated
install_mingw64_only: "" | ||
- sys: mingw64 | ||
env: x86_64 | ||
install_mingw64_only: | |
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.
I'd name this "extra_install", I think, since it's there to install extra stuff. And you can leave it off on the other job, variables default to empty.
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.
Done: commit bb8a794
Thanks @henryiii! WDYT about disabling the two failing CIBW tests in this PR, too, until someone gets a chance to fix those? CIBW / iOS wheel macos-13 (pull_request) CIBW / iOS wheel macos-14 (pull_request) |
Ugh... It is not working as intended:
@henryiii I'm a bit lost, should I maybe checking for IIUC, it's failing even though the GIL is enabled? This has been happening with macOS ever since I added this test, but only fairly infrequently. |
Description
Apparently
msys2/setup-msys2@v2
cannot be run twice anymore:Suggested changelog entry: