-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
gh-131290: ensure that test files can be executed as standalone scripts #131371
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
gh-131290: ensure that test files can be executed as standalone scripts #131371
Conversation
added environment variable value "PYTHONREGRTEST_UNICODE_GUARD0" when calling the test directly
added spec for test when calling directly
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
Misc/NEWS.d/next/Tests/2025-03-17-19-47-27.gh-issue-131290.NyCIXR.rst
Outdated
Show resolved
Hide resolved
Lib/test/test_pyclbr.py
Outdated
@@ -260,4 +260,12 @@ def test_module_has_no_spec(self): | |||
|
|||
|
|||
if __name__ == "__main__": | |||
import importlib.util | |||
|
|||
# Adding the __spec__ attribute to the __main__ module |
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.
Which test is failing because of the lack of __spec__
?
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.
test_others
user\cpython\Lib\test\test_pyclbr.py", line 226, in test_others
cm(
~~^
'pdb',
^^^^^^
# pyclbr does not handle elegantly `typing` or properties
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ignore=('Union', '_ModuleTarget', '_ScriptTarget', '_ZipTarget', 'curframe_locals'),
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
)
^
File "user\cpython\Lib\test\test_pyclbr.py", line 142, in checkModule
self.assertHaskey(dict, name, ignore)
~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^
File "user\cpython\Lib\test\test_pyclbr.py", line 39, in assertHaskey
self.assertIn(key, obj)
~~~~~~~~~~~~~^^^^^^^^^^
AssertionError: 'Pdb' not found in {'contextmanager': <pyclbr.Function object at 0x000001DA2A910690>}
----------------------------------------------------------------------
Ran 6 tests in 11.022s
FAILED (failures=1, errors=1)
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.
Can you make it a local solution for Pdb then? possibly extracting this test into a separate function?
Lib/test/test_regrtest.py
Outdated
os.environ['PYTHONREGRTEST_UNICODE_GUARD'] = 'some_value' # for test_unicode_guard_env | ||
unittest.main() |
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.
os.environ['PYTHONREGRTEST_UNICODE_GUARD'] = 'some_value' # for test_unicode_guard_env | |
unittest.main() | |
with EnvironmentVarGuard() as env: | |
# for test_unicode_guard_env | |
PYTHONREGRTEST_UNICODE_GUARD = "some_value" | |
unittest.main() |
and add some from test.support.os_hepler import EnvironmentVarGuard
top-level import. However, I don't know if this is correct to test test_unicode_guard_env
like this or if we shouldn't just skip if it we're running it from __main__
. Namely, decorate the test with @unittest.skipIf(__name__ == '__main__')
cc @vstinner
|
For regrtest, I would prefer this fix: diff --git a/Lib/test/test_regrtest.py b/Lib/test/test_regrtest.py
index 510c8f69631..7e317d5ab94 100644
--- a/Lib/test/test_regrtest.py
+++ b/Lib/test/test_regrtest.py
@@ -2546,4 +2546,5 @@ def test_test_result_get_state(self):
if __name__ == '__main__':
+ setup.setup_process()
unittest.main() |
Lib/test/test_pyclbr.py
Outdated
@@ -260,4 +260,12 @@ def test_module_has_no_spec(self): | |||
|
|||
|
|||
if __name__ == "__main__": | |||
import importlib.util | |||
|
|||
# Adding the __spec__ attribute to the __main__ module |
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.
Can you make it a local solution for Pdb then? possibly extracting this test into a separate function?
Misc/NEWS.d/next/Tests/2025-03-17-19-47-27.gh-issue-131290.NyCIXR.rst
Outdated
Show resolved
Hide resolved
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Misc/NEWS.d/next/Tests/2025-04-07-13-45-23.gh-issue-131290.MaximGit1.rst
Outdated
Show resolved
Hide resolved
Thanks for making the requested changes! : please review the changes made to this pull request. |
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'm not on my dev session so I can't check but can you check again that the entire test suite is now correctly runnable as standalone files? TiA.
Lib/test/test_pyclbr.py
Outdated
@@ -214,6 +240,13 @@ def compare(parent1, children1, parent2, children2): | |||
|
|||
compare(None, actual, None, expected) | |||
|
|||
def test_pdb_module(self): | |||
with temporary_main_spec(): |
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.
With the context manager, it's maybe better to actually put this one back in test_others(). We're also already using with warnings.catch_warnings()
. Sorry for this oversight!
def test_others(self):
cm = ...
...
with temporary_main_spec():
cm('pdb', ...)
ok i checked the tests here are the results
|
Co-authored-by: Bénédikt Tran <[email protected]>
I'm sorry I forgot about this one before leaving! I'll merge this by Sunday! |
Now tests are launched via ./python.exe and work correctly
./python Lib/test/...
#131290