-
-
Notifications
You must be signed in to change notification settings - Fork 99
Fix Python 3.14 compatiblity #780
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?
Changes from all commits
50ef6df
a60337f
aee66f2
a6e4ca0
758a762
5570389
7ad5047
9eb022d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,12 +4,14 @@ | |
itself, useless for end-users' app testing. | ||
""" | ||
|
||
import contextlib | ||
import sys | ||
import threading | ||
import time | ||
|
||
import pytest | ||
|
||
from .._compat import IS_MACOS, IS_WINDOWS # noqa: WPS436 | ||
from .._compat import IS_MACOS, IS_WINDOWS | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like this is unrelated and can be submitted separately. |
||
from ..server import Gateway, HTTPServer | ||
from ..testing import ( # noqa: F401 # pylint: disable=unused-import | ||
get_server_client, | ||
|
@@ -20,6 +22,21 @@ | |
) | ||
|
||
|
||
# Python 3.14 compatibility: Force 'fork' multiprocessing on Unix | ||
# Python 3.14 changed default from 'fork' to 'forkserver' on Unix, | ||
# which can cause issues with pytest-xdist's parallel test execution. | ||
# This ensures compatibility with existing test fixtures and shared state. | ||
# Note: Windows doesn't support 'fork', so we skip this on Windows. | ||
# Ref: https://github.com/cherrypy/cheroot/issues/767 | ||
if sys.version_info >= (3, 14) and not IS_WINDOWS: | ||
with contextlib.suppress(ImportError): | ||
import multiprocessing | ||
|
||
with contextlib.suppress(RuntimeError): | ||
# Force fork method even if already set to forkserver | ||
multiprocessing.set_start_method('fork', force=True) | ||
Comment on lines
+25
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This does not seem to have any proof of the justification. Show me fact-checking and actual research. Unnamed possible issues is too broad to make sense of it. Also, post the actual CI failure log (with a link to the line in the log) that this is supposedly addressing. |
||
|
||
|
||
@pytest.fixture | ||
def http_request_timeout(): | ||
"""Return a common HTTP request timeout for tests with queries.""" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -60,7 +60,19 @@ filterwarnings = | |
# FIXME: Python 3.13 no longer ignores IOBase errors raised by the close(), | ||
# FIXME: which exposed a possible race condition in test_conn test cleanup. | ||
# Ref: https://github.com/cherrypy/cheroot/issues/734 | ||
ignore:Exception ignored in. <function IOBase.__del__:pytest.PytestUnraisableExceptionWarning | ||
# Python 3.14 changed error message format from "Exception ignored in." to | ||
# "Exception ignored while calling deallocator", so we use a regex that matches both | ||
ignore:Exception ignored.*IOBase.__del__:pytest.PytestUnraisableExceptionWarning | ||
Comment on lines
+63
to
+65
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ideally, this should be fixed. But since there was a suppression in place already, It's fine to keep it for now. |
||
|
||
# Python 3.14: PEP 765 - control flow statements in finally blocks | ||
# Added as defensive measure even though current code has no violations | ||
# Ref: https://github.com/cherrypy/cheroot/issues/767 | ||
ignore::SyntaxWarning | ||
Comment on lines
+67
to
+70
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This must not be ignored. Definitely not as a blanket suppression without justification. |
||
|
||
# Python 3.14: Free-threaded build context-aware warnings | ||
# Context-aware warnings may behave differently in free-threaded builds | ||
# Ref: https://github.com/cherrypy/cheroot/issues/767 | ||
ignore:.*context.*:RuntimeWarning | ||
Comment on lines
+72
to
+75
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't have any specifics and therefore is not a good/acceptable justification. |
||
|
||
# https://docs.pytest.org/en/stable/usage.html#creating-junitxml-format-files | ||
junit_duration_report = call | ||
|
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.
Why not just update the existing comment. Disconnected comments tend to get lost. Also, it's obvious that Python 3.11 is used from YAML. No need to repeat things too verbosely for the sake of verboseness.
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.
Besides, only the old version is not compatible with Python 3.12. It's unknown if a newer isn't compatible with 3.14. But since there's a release with 3.13 in Trove classifiers, it would make sense to bump the dependency version and maybe bump this to 3.13 if that work + verify if 3.14 could work too.