← 返回首页
bpo-10572: Move tkinter tests to /test by idomic · Pull Request #18727 · python/cpython · GitHub
Skip to content

Navigation Menu

Toggle navigation
Sign in
Appearance settings
Search or jump to...

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Include my email address so I can be contacted
Cancel Submit feedback

Saved searches

Use saved searches to filter your results more quickly

Cancel Create saved search
Appearance settings
Resetting focus

bpo-10572: Move tkinter tests to /test#18727

Closed
idomic wants to merge 3 commits into
python:mainfrom
idomic:bpo-10572-testlibs
Closed

bpo-10572: Move tkinter tests to /test#18727
idomic wants to merge 3 commits into
python:mainfrom
idomic:bpo-10572-testlibs

Conversation

idomic commented Mar 1, 2020 •
edited by terryjreedy
Loading

Copy link
Copy Markdown
Contributor

idomic changed the title Moving tests - clean start - tkinter, sqlite3, ctypes bpo-10572: Moving tests - clean start - tkinter, sqlite3, ctypes Mar 1, 2020
berkerpeksag requested changes Mar 2, 2020
View reviewed changes

berkerpeksag left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide 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

Copy link
Copy Markdown

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 I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

idomic force-pushed the bpo-10572-testlibs branch from d659ed4 to ffa202d Compare March 15, 2020 14:44
merwok reviewed Mar 15, 2020
View reviewed changes
Comment thread Lib/test/test_tk.py Outdated
support.requires('gui')

from tkinter.test import runtktests
from Lib.test.tkinter_test import runtktests

merwok Mar 15, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide 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)

idomic Mar 16, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide 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

Copy link
Copy Markdown
Contributor Author

I have made the requested changes; please review again

bedevere-bot commented Mar 16, 2020

Copy link
Copy Markdown

Thanks for making the requested changes!

@berkerpeksag: please review the changes made to this pull request.

terryjreedy requested changes Sep 6, 2020
View reviewed changes

terryjreedy left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide 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

Copy link
Copy Markdown

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 I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

merwok reviewed Sep 6, 2020
View reviewed changes
Comment thread Lib/test/test_tk.py
@@ -5,7 +5,7 @@
# Skip test if tk cannot be initialized.

merwok Sep 6, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

Lib/__init__.py should be deleted.

merwok reviewed Sep 6, 2020
View reviewed changes
Comment thread Lib/test/test_ttk_textonly.py
from test.tkinter_test import runtktests

def test_main():
support.run_unittest(

merwok Sep 6, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide 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 bpo-10572: Moving tests - clean start - tkinter, sqlite3, ctypes bpo-10572: Move tkinter tests to /test Jul 15, 2021
rhettinger removed their request for review May 3, 2022 06:30
voidspace mentioned this pull request Jun 20, 2022
Closed

erlend-aasland commented Jul 20, 2022

Copy link
Copy Markdown
Contributor

This was implemented in #94070.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

merwok merwok left review comments
terryjreedy terryjreedy requested changes
berkerpeksag Awaiting requested review from berkerpeksag
serhiy-storchaka Awaiting requested review from serhiy-storchaka

Assignees

No one assigned

Labels

awaiting changes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

Footer

© 2026 GitHub, Inc.