Skip to content
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

Do not use _default_root in Tkinter tests #66432

Closed
serhiy-storchaka opened this issue Aug 20, 2014 · 6 comments
Closed

Do not use _default_root in Tkinter tests #66432

serhiy-storchaka opened this issue Aug 20, 2014 · 6 comments
Assignees
Labels
tests Tests in the Lib/test dir topic-tkinter type-bug An unexpected behavior, bug, or error

Comments

@serhiy-storchaka
Copy link
Member

BPO 22236
Nosy @terryjreedy, @ned-deily, @zware, @serhiy-storchaka
Files
  • tkinter_no_default_root.patch
  • tkinter_no_default_root_2.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/serhiy-storchaka'
    closed_at = <Date 2014-08-24.06:52:13.927>
    created_at = <Date 2014-08-20.14:19:41.627>
    labels = ['tests', 'type-bug', 'expert-tkinter']
    title = 'Do not use _default_root in Tkinter tests'
    updated_at = <Date 2014-10-30.23:07:43.799>
    user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2014-10-30.23:07:43.799>
    actor = 'ned.deily'
    assignee = 'serhiy.storchaka'
    closed = True
    closed_date = <Date 2014-08-24.06:52:13.927>
    closer = 'serhiy.storchaka'
    components = ['Tests', 'Tkinter']
    creation = <Date 2014-08-20.14:19:41.627>
    creator = 'serhiy.storchaka'
    dependencies = []
    files = ['36421', '36428']
    hgrepos = []
    issue_num = 22236
    keywords = ['patch']
    message_count = 6.0
    messages = ['225570', '225592', '225612', '225795', '225797', '230308']
    nosy_count = 6.0
    nosy_names = ['terry.reedy', 'gpolo', 'ned.deily', 'python-dev', 'zach.ware', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue22236'
    versions = ['Python 2.7', 'Python 3.4', 'Python 3.5']

    @serhiy-storchaka
    Copy link
    Member Author

    Currently many Tkinter tests depends on tkinter._default_root. I.e. they reuse the same Tcl interpreter and main window. This can cause unexpected dependencies between tests. Proposed patch creates new root for every test, this makes tests mutually independent. It also fixes some bugs in NoDefaultRoot mode and get rid of 'can't invoke "event" command:' messages in tests. This patch is needed to run Tkinter tests in different "wantobjects" modes (bpo-21585).

    @serhiy-storchaka serhiy-storchaka added tests Tests in the Lib/test dir topic-tkinter type-bug An unexpected behavior, bug, or error labels Aug 20, 2014
    @terryjreedy
    Copy link
    Member

    For idle tests, I already avoid the default root and (intend to) create all widgets as a descendent of an explicit root. This allows explicit deletion and avoidance of error messages and memory leaks. This seems to cover the majority of changes.

    An explicit root is mostly true of idle code also, but there seem to be a few exceptions (as indicated by error messages). I presume that deleting and preventing the re-creation of default root, with the code in the patch, would cause a traceback revealing the location of such exceptions. That would have to be done before the offending code is run.

    I do not see any need to try to modify the tkinter module for each test function (and slow down tkinter tests). It seems that "tkinter.NoDefaultRoot()" should be moved to destroy_default_root and that function called just once at the top of the module.

    Creating and destroying root takes over .04 seconds on my machine.
    >>> timeit('root=Tk(); root.destroy()', 'from tkinter import Tk', number=100)
    4.222483729329078
    Doing it once per test method, with widgets added, will be noticeable.

    Unless a test modifies the root widget itself (other than its set of children), I would rename the AbstractTkinterTest methods to setUpClass and tearDownClass, and leave setUp for regeneration of the specific widget or widgets being tested. That is what we do for Idle gui test classes, and only that often because setUpModule and tearDownModule do not work on 2.7.

    -
    In one module, you have this change:
     # Make sure tkinter._fix runs to set up the environment
    -support.import_fresh_module('tkinter')
    +tkinter = support.import_fresh_module('tkinter')

    In test/test_idle, I test presence of tkinter with
    tk = import_module('tkinter') # imports _tkinter
    Perhaps this should use import_fresh_module instead. Tests seems to work, though there are occasional obscure interaction problems in test suite.

    @serhiy-storchaka
    Copy link
    Member Author

    In updated patch the root windows is created only once per test class.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Aug 24, 2014

    New changeset 32fdaf401e50 by Serhiy Storchaka in branch '2.7':
    Issue bpo-22236: Tkinter tests now don't reuse default root window. New root
    http://hg.python.org/cpython/rev/32fdaf401e50

    New changeset dd1dffe6f0d2 by Serhiy Storchaka in branch '3.4':
    Issue bpo-22236: Tkinter tests now don't reuse default root window. New root
    http://hg.python.org/cpython/rev/dd1dffe6f0d2

    New changeset 014060738f7f by Serhiy Storchaka in branch 'default':
    Issue bpo-22236: Tkinter tests now don't reuse default root window. New root
    http://hg.python.org/cpython/rev/014060738f7f

    @serhiy-storchaka
    Copy link
    Member Author

    Thanks Terry and Zachary for your reviews. I have committed the patch in haste because other issues depend on it.

    @ned-deily
    Copy link
    Member

    See bpo-22770 for details of a potential Tk crash that can occur on OS X when running tests as a result of these changes and for a workaround.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    tests Tests in the Lib/test dir topic-tkinter type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants