Uh oh!
There was an error while loading. Please reload this page.
bpo-10572: Move tkinter tests to /test - #18727
Conversation
berkerpeksag
left a comment
There was a problem hiding this comment.
Please do not touch sqlite3 tests. It needs more work than just moving things around.
Also, submitting separate PRs for each module would increase the chance of getting them accepted.
bedevere-bot
commented
Mar 2, 2020
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 |
| 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.
idomic
commented
Mar 16, 2020
I have made the requested changes; please review again |
bedevere-bot
commented
Mar 16, 2020
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.
bedevere-bot
commented
Sep 6, 2020
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. | |||
| 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?
erlend-aasland
commented
Jul 20, 2022
This was implemented in #94070. |
https://bugs.python.org/issue10572