Skip to content

Refactor: move no_rerun from test_import and datetimetester to test.support #119659

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.

Sign up for GitHub

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

Jump to bottom
Closed
sobolevn opened this issue May 28, 2024 · 3 comments
Closed

Refactor: move no_rerun from test_import and datetimetester to test.support #119659

sobolevn opened this issue May 28, 2024 · 3 comments
Assignees
@sobolevn
Labels
tests Tests in the Lib/test dir type-feature A feature request or enhancement

Comments

@sobolevn
Copy link
Member

sobolevn commented May 28, 2024 •
edited by bedevere-app bot
Loading

Feature or enhancement

It is defined here:

cpython/Lib/test/test_import/__init__.py

Lines 123 to 139 in b407ad3

def no_rerun(reason):
"""Skip rerunning for a particular test.
WARNING: Use this decorator with care; skipping rerunning makes it
impossible to find reference leaks. Provide a clear reason for skipping the
test using the 'reason' parameter.
"""
def deco(func):
_has_run = False
def wrapper(self):
nonlocal _has_run
if _has_run:
self.skipTest(reason)
func(self)
_has_run = True
return wrapper
return deco

and here:

cpython/Lib/test/datetimetester.py

Lines 50 to 68 in b407ad3

# This is copied from test_import/__init__.py.
# XXX Move it to support/__init__.py.
def no_rerun(reason):
"""Skip rerunning for a particular test.
WARNING: Use this decorator with care; skipping rerunning makes it
impossible to find reference leaks. Provide a clear reason for skipping the
test using the 'reason' parameter.
"""
def deco(func):
_has_run = False
def wrapper(self):
nonlocal _has_run
if _has_run:
self.skipTest(reason)
func(self)
_has_run = True
return wrapper
return deco

Introduced in #119373

Linked PRs
@sobolevn sobolevn added type-feature A feature request or enhancement tests Tests in the Lib/test dir labels May 28, 2024
@sobolevn sobolevn self-assigned this May 28, 2024
sobolevn added a commit to sobolevn/cpython that referenced this issue May 28, 2024
@sobolevn
pythongh-119659: Move @no_rerun to test.support
0eea206
@bedevere-app bedevere-app bot mentioned this issue May 28, 2024
Merged
sobolevn added a commit that referenced this issue May 28, 2024
@sobolevn
gh-119659: Move @no_rerun to test.support (#119660)
2da0dc0
@sobolevn
Copy link
Member Author

aaaand I found a bug:

cpython/Lib/test/support/__init__.py

Lines 1192 to 1208 in 0518edc

def no_rerun(reason):
"""Skip rerunning for a particular test.
WARNING: Use this decorator with care; skipping rerunning makes it
impossible to find reference leaks. Provide a clear reason for skipping the
test using the 'reason' parameter.
"""
def deco(func):
_has_run = False
def wrapper(self):
nonlocal _has_run
if _has_run:
self.skipTest(reason)
func(self)
_has_run = True
return wrapper
return deco

this code does not work well when applied to classes.

proof (with @no_rerun decorator):

cpython/Lib/test/datetimetester.py

Lines 6385 to 6392 in 0518edc

@unittest.skipIf(_testcapi is None, 'need _testcapi module')
@no_rerun("the encapsulated datetime C API does not support reloading")
class CapiTest(unittest.TestCase):
def setUp(self):
# Since the C API is not present in the _Pure tests, skip all tests
if self.__class__.__name__.endswith('Pure'):
self.skipTest('Not relevant in pure Python') 

» ./python.exe -m test test_datetime -m test_utc_capi -v
== CPython 3.14.0a0 (heads/main:669175bf8ed, May 28 2024, 13:18:18) [Clang 15.0.0 (clang-1500.3.9.4)]
== macOS-14.4.1-arm64-arm-64bit-Mach-O little-endian
== Python build: debug
== cwd: /Users/sobolev/Desktop/cpython2/build/test_python_worker_98352æ
== CPU count: 12
== encodings: locale=UTF-8 FS=utf-8
== resources: all test resources are disabled, use -u option to unskip tests

Using random seed: 1630075014
0:00:00 load avg: 2.16 Run 1 test sequentially
0:00:00 load avg: 2.16 [1/1] test_datetime

----------------------------------------------------------------------
Ran 0 tests in 0.000s

NO TESTS RAN
test_datetime ran no tests

== Tests result: NO TESTS RAN ==

1 test run no tests:
    test_datetime

Total duration: 124 ms
Total tests: run=0 (filtered)
Total test files: run=1/1 (filtered) run_no_tests=1
Result: NO TESTS RAN

Without:

» ./python.exe -m test test_datetime -m test_utc_capi -v
== CPython 3.14.0a0 (heads/main:669175bf8ed, May 28 2024, 13:18:18) [Clang 15.0.0 (clang-1500.3.9.4)]
== macOS-14.4.1-arm64-arm-64bit-Mach-O little-endian
== Python build: debug
== cwd: /Users/sobolev/Desktop/cpython2/build/test_python_worker_98457æ
== CPU count: 12
== encodings: locale=UTF-8 FS=utf-8
== resources: all test resources are disabled, use -u option to unskip tests

Using random seed: 1077352783
0:00:00 load avg: 1.75 Run 1 test sequentially
0:00:00 load avg: 1.75 [1/1] test_datetime
test_utc_capi (test.datetimetester.CapiTest_Pure.test_utc_capi) ... skipped 'Not relevant in pure Python'
test_utc_capi (test.datetimetester.CapiTest_Fast.test_utc_capi) ... ok

----------------------------------------------------------------------
Ran 2 tests in 0.000s

OK (skipped=1)

== Tests result: SUCCESS ==

1 test OK.

Total duration: 130 ms
Total tests: run=2 (filtered) skipped=1
Total test files: run=1/1 (filtered)
Result: SUCCESS

Turns out it is not just "moving", I also have to fix it for both types and methods.

@sobolevn
Copy link
Member Author

Aaaand I found another one: test_datetime forgets about existing setUpClass and tearDownClass:

cpython/Lib/test/test_datetime.py

Lines 39 to 56 in 0518edc

for cls in test_classes:
cls.__name__ += suffix
cls.__qualname__ += suffix
@classmethod
def setUpClass(cls_, module=module):
cls_._save_sys_modules = sys.modules.copy()
sys.modules[TESTS] = module
sys.modules['datetime'] = module.datetime_module
if hasattr(module, '_pydatetime'):
sys.modules['_pydatetime'] = module._pydatetime
sys.modules['_strptime'] = module._strptime
@classmethod
def tearDownClass(cls_):
sys.modules.clear()
sys.modules.update(cls_._save_sys_modules)
cls.setUpClass = setUpClass
cls.tearDownClass = tearDownClass
tests.addTests(loader.loadTestsFromTestCase(cls))

@sobolevn
Copy link
Member Author

And the whole nonlocal _has_run idea simply does not work 🙈
Because on each run the test class is different.

Wow, what a ride! 😄

sobolevn added a commit to sobolevn/cpython that referenced this issue May 28, 2024
@sobolevn
pythongh-119659: Fix support.no_rerun decorator
6198a35
@bedevere-app bedevere-app bot mentioned this issue May 28, 2024
Closed
@bedevere-app bedevere-app bot mentioned this issue Jun 6, 2024
Merged
ericsnowcurrently added a commit that referenced this issue Jun 7, 2024
@ericsnowcurrently
gh-119659: Get the datetime CAPI Tests Running Again (gh-120180)
e6076d1 
The tests were accidentally disabled by 2da0dc0, which didn't handle classes correctly.

I considered updating no_rerun() to support classes, but the way test_datetime.py works would have made things fairly messy.  Plus, it looks like the refleaks we had encountered before have been resolved.
@sobolevn sobolevn closed this as completed Jun 7, 2024
@sobolevn sobolevn closed this as not planned Won't fix, can't repro, duplicate, stale Jun 7, 2024
@sobolevn sobolevn closed this as completed Jun 7, 2024
@sobolevn sobolevn mentioned this issue Jun 7, 2024
Closed
noahbkim pushed a commit to hudson-trading/cpython that referenced this issue Jul 11, 2024
@ericsnowcurrently @noahbkim
pythongh-119659: Get the datetime CAPI Tests Running Again (pythongh-…
eb8b713 
…120180)

The tests were accidentally disabled by 2da0dc0, which didn't handle classes correctly.

I considered updating no_rerun() to support classes, but the way test_datetime.py works would have made things fairly messy.  Plus, it looks like the refleaks we had encountered before have been resolved.
estyxx pushed a commit to estyxx/cpython that referenced this issue Jul 17, 2024
@sobolevn @estyxx
pythongh-119659: Move @no_rerun to test.support (python#119660)
e44c6ad
estyxx pushed a commit to estyxx/cpython that referenced this issue Jul 17, 2024
@ericsnowcurrently @estyxx
pythongh-119659: Get the datetime CAPI Tests Running Again (pythongh-…
32f2fe8 
…120180)

The tests were accidentally disabled by 2da0dc0, which didn't handle classes correctly.

I considered updating no_rerun() to support classes, but the way test_datetime.py works would have made things fairly messy.  Plus, it looks like the refleaks we had encountered before have been resolved.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sobolevn sobolevn

Labels
tests Tests in the Lib/test dir type-feature A feature request or enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@sobolevn