-
-
Notifications
You must be signed in to change notification settings - Fork 31.5k
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
bpo-40275: Use new test.support helper submodules in tests #21151
bpo-40275: Use new test.support helper submodules in tests #21151
Conversation
* tests of distutils * test_buffer * test_compile * test_filecmp * test_fileinput * test_readline * test_smtpnet * test_structmembers * test_tools
Lib/test/test_fileinput.py
Outdated
@@ -39,7 +41,7 @@ class BaseTests: | |||
# temp file's name. | |||
def writeTmp(self, content, *, mode='w'): # opening in text mode is the default | |||
fd, name = tempfile.mkstemp() | |||
self.addCleanup(support.unlink, name) | |||
self.addCleanup(safe_unlink, name) |
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 would prefer to use os_helper.unlink here, rather than changing the function name.
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.
done.
I use safe_unlink in here's reason is that test_fileinput have import unlink as safe_unlink.
if I import os_helper again, the imports looks like more duplicated~
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.
It's common to import a module and import some symbols of the same module. "Inconsistencies" are common, especially in tests which are old :-)
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.
For such PR which basically updates tests to point to new submodules, I prefer to leave the tests unchanged, don't change the coding style in the same PR.
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.
(Also, PR which only changes coding style are usually rejected. In short, the coding style cannot be updated in Python code base :-))
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.
(Also, PR which only changes coding style are usually rejected. In short, the coding style cannot be updated in Python code base :-))
Got it, Thanks. I have know the pain of code churn( not easy for finding the historical record ;-) )
|
…21151) Use new test.support helper submodules in tests: * distutils tests * test_buffer * test_compile * test_filecmp * test_fileinput * test_readline * test_smtpnet * test_structmembers * test_tools
…21151) Use new test.support helper submodules in tests: * distutils tests * test_buffer * test_compile * test_filecmp * test_fileinput * test_readline * test_smtpnet * test_structmembers * test_tools
Use test helpers in the following test cases:
https://bugs.python.org/issue40275