bpo-10572: Move tkinter tests to /test #18727
Conversation
idomic
changed the title
|
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 |
idomic
force-pushed
the
bpo-10572-testlibs
branch
from
d659ed4 to
ffa202d
Compare
Lib/test/test_tk.py
Outdated
| @@ -5,7 +5,7 @@ | |||
| # Skip test if tk cannot be initialized. | |||
| support.requires('gui') | |||
|
|
|||
| from tkinter.test import runtktests | |||
| from Lib.test.tkinter_test import runtktests | |||
There was a problem hiding this comment.
This can’t be correct. Lib is not an importable Python package, it is a path containing modules and packages.
The new import should be from test.tkinter_test import runtktests.
(Or for what it’s worth, this nearly empty file could be removed, if the new package for tkinter tests would be named in a way that’s automatically found by unittest discover and runtests)
There was a problem hiding this comment.
Done, I think it might be better to separate the changes no?
Like the first stage - migrate the module. Second stage - remove redundant legacy files.
|
I have made the requested changes; please review again |
|
Thanks for making the requested changes! @berkerpeksag: please review the changes made to this pull request. |
There was a problem hiding this comment.
I don't think that the actual tkinter tests should be installed unless tkinter is installed. So I don't think that tkinter tests should be changed. I certainly would not do so unless Serhiy wants the move and approves of how it is done.
EDIT: So I am actually suggesting that this be closed and that you pick something else to move that is always installed.
|
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 |
| @@ -5,7 +5,7 @@ | |||
| # Skip test if tk cannot be initialized. | |||
There was a problem hiding this comment.
Lib/__init__.py should be deleted.
| @@ -3,7 +3,7 @@ | |||
| # Skip this test if _tkinter does not exist. | |||
| support.import_module('_tkinter') | |||
|
|
|||
| from tkinter.test import runtktests | |||
| from test.tkinter_test import runtktests | |||
|
|
|||
| def test_main(): | |||
| support.run_unittest( | |||
There was a problem hiding this comment.
After moving the tests, files like this (just contains an import and a wrapper to interface runtests with test_ttk) end up being a slightly confusing, mostly unneeded layer.
As long as many files are moved and history is broken, why not also clean up this tangle by merging the compat layer into the real test files?
terryjreedy
changed the title
|
This was implemented in #94070. |
https://bugs.python.org/issue10572