gh-143959: Split datetime tests requiring _datetime#143997
gh-143959: Split datetime tests requiring _datetime#143997VanshAgarwal24036 wants to merge 10 commits intopython:mainfrom
Conversation
|
Test-only change; no user-visible impact, so no NEWS entry is needed. |
Lib/test/test_datetime.py
Outdated
| fast_tests = import_fresh_module(TESTS, | ||
| fresh=['datetime', '_strptime'], | ||
| blocked=['_pydatetime']) | ||
| pure_tests = import_fresh_module( |
There was a problem hiding this comment.
You can now restore the old formatting.
There was a problem hiding this comment.
Thanks — I’ve restored the original formatting.
There was a problem hiding this comment.
It is not restored yet.
There was a problem hiding this comment.
I’ve now restored the original structure by keeping the two import_fresh_module() calls adjacent and moved the _datetime availability check outside that block.
|
It's not easy to test this change on CPython since _datetime is now a built-in module. I tried to build diff --git a/Modules/Setup b/Modules/Setup
index 7d816ead843..828368cf7c2 100644
--- a/Modules/Setup
+++ b/Modules/Setup
@@ -126,14 +126,14 @@ PYTHONPATH=$(COREPYTHONPATH)
# modules are to be built as shared libraries (see above for more
# detail; also note that *static* or *disabled* cancels this effect):
-#*shared*
+*shared*
# Modules that should always be present (POSIX and Windows):
#_asyncio _asynciomodule.c
#_bisect _bisectmodule.c
#_csv _csv.c
-#_datetime _datetimemodule.c
+_datetime _datetimemodule.c
#_decimal _decimal/_decimal.c
#_heapq _heapqmodule.c
#_interpchannels _interpchannelsmodule.c
diff --git a/Modules/Setup.bootstrap.in b/Modules/Setup.bootstrap.in
index 65a1fefe72e..3e7344382da 100644
--- a/Modules/Setup.bootstrap.in
+++ b/Modules/Setup.bootstrap.in
@@ -13,7 +13,7 @@ _signal signalmodule.c
_tracemalloc _tracemalloc.c
_suggestions _suggestions.c
# needs libm and on some platforms librt
-_datetime _datetimemodule.c
+#_datetime _datetimemodule.c
# modules used by importlib, deepfreeze, freeze, runpy, and sysconfig
_codecs _codecsmodule.cBut the build fails with: I tested the change with: diff --git a/Lib/test/test_datetime.py b/Lib/test/test_datetime.py
index 32556bcb184..5b6d52859e2 100644
--- a/Lib/test/test_datetime.py
+++ b/Lib/test/test_datetime.py
@@ -9,6 +9,7 @@
def load_tests(loader, tests, pattern):
try:
try:
+ raise ImportError
import _datetime
except ImportError:
has_datetime_ext = FalseOutput: |
|
Thanks for testing this, as well as the explanation. Yes, the intention is just this behavior: when |
|
I hacked Python to build again Result: test_datetime fails. I'm not convinced that this change works as expected. |
|
My hack to build Detailsdiff --git a/Modules/Setup b/Modules/Setup
index 7d816ead843..828368cf7c2 100644
--- a/Modules/Setup
+++ b/Modules/Setup
@@ -126,14 +126,14 @@ PYTHONPATH=$(COREPYTHONPATH)
# modules are to be built as shared libraries (see above for more
# detail; also note that *static* or *disabled* cancels this effect):
-#*shared*
+*shared*
# Modules that should always be present (POSIX and Windows):
#_asyncio _asynciomodule.c
#_bisect _bisectmodule.c
#_csv _csv.c
-#_datetime _datetimemodule.c
+_datetime _datetimemodule.c
#_decimal _decimal/_decimal.c
#_heapq _heapqmodule.c
#_interpchannels _interpchannelsmodule.c
diff --git a/Modules/Setup.bootstrap.in b/Modules/Setup.bootstrap.in
index 65a1fefe72e..3e7344382da 100644
--- a/Modules/Setup.bootstrap.in
+++ b/Modules/Setup.bootstrap.in
@@ -13,7 +13,7 @@ _signal signalmodule.c
_tracemalloc _tracemalloc.c
_suggestions _suggestions.c
# needs libm and on some platforms librt
-_datetime _datetimemodule.c
+#_datetime _datetimemodule.c
# modules used by importlib, deepfreeze, freeze, runpy, and sysconfig
_codecs _codecsmodule.c
diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c
index 8f64e572bd6..754e7c7da22 100644
--- a/Modules/_datetimemodule.c
+++ b/Modules/_datetimemodule.c
@@ -7690,6 +7690,12 @@ static PyModuleDef datetimemodule = {
PyMODINIT_FUNC
PyInit__datetime(void)
{
+ PyInterpreterState *interp = PyInterpreterState_Get();
+ PyStatus status = _PyDateTime_InitTypes(interp);
+ if (_PyStatus_EXCEPTION(status)) {
+ Py_ExitStatusException(status);
+ }
+
return PyModuleDef_Init(&datetimemodule);
}
diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c
index 88dbdb6d139..c7fdd3dfd27 100644
--- a/Python/pylifecycle.c
+++ b/Python/pylifecycle.c
@@ -776,11 +776,6 @@ pycore_init_types(PyInterpreterState *interp)
return status;
}
- status = _PyDateTime_InitTypes(interp);
- if (_PyStatus_EXCEPTION(status)) {
- return status;
- }
-
return _PyStatus_OK();
}
|
This is why I have not merged this PR yet. I consider also the idea of adding |
|
Thanks for investigating this and for conducting such an in-depth experiment. My plan would be to decide whether there should be Fast tests in the function load_tests(). This prevents us from having to use the Does this approach look OK to you? |
|
I think this CI failure unrelated to this change. The PR only modifies |
|
"Tests / Windows (free-threading) / Build and test (arm64)": test_external_inspection failed with |
|
|
||
| test_modules = [pure_tests, fast_tests] | ||
| test_suffixes = ["_Pure", "_Fast"] | ||
| test_modules = [pure_tests] |
There was a problem hiding this comment.
Now the accelerated module is not tested at all.
This PR splits the datetime tests into two groups:
_datetime_datetimeThe
_Fasttests are only loaded when_datetimeis available, allowingthe test suite to run correctly on builds where
_datetimeis not present.No test behavior is changed.