gh-143959 Make _datetime,_types optional for test_types

[youknowone]

• Issue: test_sys globally requires _datetime for cpython_only test #143959

[serhiy-storchaka]

Why these changes were needed?

[youknowone]

Current test only runs meaningful when _types optional module is implemented.
Usually Python unittest includes conditional handling to run tests under proper environment, but this test was exposed without it.
Adding checking _types split the test and verify the behavior regardless the environment has _types or not.

[serhiy-storchaka]

How did you test this?

Note that import_fresh_module() returns None when one of the "fresh" modules can not be imported.

[youknowone]

That's exactly the cause of the problem. The test doesn't work when they are None.

local patch

diff --git a/Lib/test/test_types.py b/Lib/test/test_types.py
index 39d57c5f5b6..3a7c2c5f3bf 100644
--- a/Lib/test/test_types.py+++ b/Lib/test/test_types.py@@ -23,8 +23,8 @@
import typing
import re
-c_types = import_fresh_module('types', fresh=['_types'])-py_types = import_fresh_module('types', blocked=['_types'])+c_types = import_fresh_module('types', fresh=['_no_types'])+py_types = import_fresh_module('types', blocked=['_no_types'])
T = typing.TypeVar("T")

./python.exe -m test test_types
Using random seed: 763234965
0:00:00 load avg: 5.65 Run 1 test sequentially in a single process
0:00:00 load avg: 5.65 [1/1] test_types
test test_types failed -- Traceback (most recent call last):
File "/Users/al03219714/Projects/cpython/Lib/test/test_types.py", line 48, in test_names
fornamein c_types.__all__:
^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute '__all__'. Did you mean '.__le__' instead of '.__all__'?
0:00:00 load avg: 5.65 [1/1/1] test_types failed (1 error)
== Tests result: FAILURE ==
1 test failed:
test_types
Total duration: 210 ms
Total tests: run=131 skipped=1
Total test files: run=1/1 failed=1
Result: FAILURE

btw, I am sorry for the late response.

[youknowone]

I deleted _types to check the changed tests pass.

diff --git a/Modules/Setup b/Modules/Setup
index 7d816ead843..8641aaeba61 100644
--- a/Modules/Setup+++ b/Modules/Setup@@ -149,7 +149,6 @@ PYTHONPATH=$(COREPYTHONPATH)
#_socket socketmodule.c
#_statistics _statisticsmodule.c
#_struct _struct.c
-#_types _typesmodule.c
#_typing _typingmodule.c
#_zoneinfo _zoneinfo.c
#array arraymodule.c

Also edited capsule test to be skipped when _types doesn't exist

[serhiy-storchaka]

I don't like that c_only_names and ignored are defined in several places.

Would not be easier to simply skip tests that require c_types if it is not available?

diff --git a/Lib/test/test_types.py b/Lib/test/test_types.py
index 39d57c5f5b6..33020f4080a 100644
--- a/Lib/test/test_types.py+++ b/Lib/test/test_types.py@@ -45,10 +45,6 @@ def test_names(self):
ignored = {'new_class', 'resolve_bases', 'prepare_class',
'get_original_bases', 'DynamicClassAttribute', 'coroutine'}
- for name in c_types.__all__:- if name not in c_only_names | ignored:- self.assertIs(getattr(c_types, name), getattr(py_types, name))-
all_names = ignored | {
'AsyncGeneratorType', 'BuiltinFunctionType', 'BuiltinMethodType',
'CapsuleType', 'CellType', 'ClassMethodDescriptorType', 'CodeType',
@@ -61,8 +57,13 @@ def test_names(self):
'NotImplementedType', 'SimpleNamespace', 'TracebackType',
'UnionType', 'WrapperDescriptorType',
}
- self.assertEqual(all_names, set(c_types.__all__))
self.assertEqual(all_names - c_only_names, set(py_types.__all__))
+ if c_types is not None:+ self.assertEqual(all_names, set(c_types.__all__))+ for name in c_types.__all__:+ if name not in c_only_names | ignored:+ self.assertIs(getattr(c_types, name), getattr(py_types, name))+
def test_truth_values(self):
if None: self.fail('None is true instead of false')

And since c_types and py_types are only used in this test, I would import them locally.

[AA-Turner]

Unclear how this is related to the linked issue? The test doesn't use cpython_only.

A

[serhiy-storchaka]

_datetime is only used in this test, so we can import it locally. We can use test.support.import_module() to skip the test.

[serhiy-storchaka]

We can use test.support.get_attribute(types, 'CapsuleType') to skip the test if CapsuleType is not available.

[serhiy-storchaka]

I don't like that c_only_names and ignored are defined in several places.

Would not be easier to simply skip tests that require c_types if it is not available?

diff --git a/Lib/test/test_types.py b/Lib/test/test_types.py
index 39d57c5f5b6..33020f4080a 100644
--- a/Lib/test/test_types.py+++ b/Lib/test/test_types.py@@ -45,10 +45,6 @@ def test_names(self):
ignored = {'new_class', 'resolve_bases', 'prepare_class',
'get_original_bases', 'DynamicClassAttribute', 'coroutine'}
- for name in c_types.__all__:- if name not in c_only_names | ignored:- self.assertIs(getattr(c_types, name), getattr(py_types, name))-
all_names = ignored | {
'AsyncGeneratorType', 'BuiltinFunctionType', 'BuiltinMethodType',
'CapsuleType', 'CellType', 'ClassMethodDescriptorType', 'CodeType',
@@ -61,8 +57,13 @@ def test_names(self):
'NotImplementedType', 'SimpleNamespace', 'TracebackType',
'UnionType', 'WrapperDescriptorType',
}
- self.assertEqual(all_names, set(c_types.__all__))
self.assertEqual(all_names - c_only_names, set(py_types.__all__))
+ if c_types is not None:+ self.assertEqual(all_names, set(c_types.__all__))+ for name in c_types.__all__:+ if name not in c_only_names | ignored:+ self.assertIs(getattr(c_types, name), getattr(py_types, name))+
def test_truth_values(self):
if None: self.fail('None is true instead of false')

And since c_types and py_types are only used in this test, I would import them locally.

[github-actions]

This PR is stale because it has been open for 30 days with no activity.