classification
Title: Do not use _default_root in Tkinter tests
Type: behavior Stage: resolved
Components: Tests, Tkinter Versions: Python 3.5, Python 3.4, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: gpolo, ned.deily, python-dev, serhiy.storchaka, terry.reedy, zach.ware
Priority: normal Keywords: patch

Created on 2014-08-20 14:19 by serhiy.storchaka, last changed 2014-10-30 23:07 by ned.deily. This issue is now closed.

Files
File name Uploaded Description Edit
tkinter_no_default_root.patch serhiy.storchaka, 2014-08-20 14:19 review
tkinter_no_default_root_2.patch serhiy.storchaka, 2014-08-21 19:39 review
Messages (6)
msg225570 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-08-20 14:19
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 (issue21585).
msg225592 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-08-20 22:42
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.
msg225612 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-08-21 19:39
In updated patch the root windows is created only once per test class.
msg225795 - (view) Author: Roundup Robot (python-dev) Date: 2014-08-24 06:14
New changeset 32fdaf401e50 by Serhiy Storchaka in branch '2.7':
Issue #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 #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 #22236: Tkinter tests now don't reuse default root window.  New root
http://hg.python.org/cpython/rev/014060738f7f
msg225797 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-08-24 06:52
Thanks Terry and Zachary for your reviews. I have committed the patch in haste because other issues depend on it.
msg230308 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2014-10-30 23:07
See Issue22770 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.
History
Date User Action Args
2014-10-30 23:07:43ned.deilysetnosy: + ned.deily
messages: + msg230308
2014-08-24 06:52:13serhiy.storchakasetstatus: open -> closed
messages: + msg225797

assignee: serhiy.storchaka
resolution: fixed
stage: patch review -> resolved
2014-08-24 06:14:09python-devsetnosy: + python-dev
messages: + msg225795
2014-08-23 20:33:06zach.warelinkissue22260 dependencies
2014-08-21 19:40:01serhiy.storchakalinkissue21585 dependencies
2014-08-21 19:39:13serhiy.storchakasetfiles: + tkinter_no_default_root_2.patch

messages: + msg225612
2014-08-20 22:42:57terry.reedysetmessages: + msg225592
2014-08-20 14:19:41serhiy.storchakacreate