classification
Title: Idle: Make test.support.requires('gui') skip when it should.
Type: behavior Stage: resolved
Components: Versions: Python 3.3, Python 3.4, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: terry.reedy Nosy List: JayKrish, Todd.Rovito, ned.deily, philwebster, python-dev, r.david.murray, terry.reedy, zach.ware
Priority: high Keywords: patch

Created on 2013-07-13 22:34 by terry.reedy, last changed 2013-07-31 00:14 by terry.reedy. This issue is now closed.

Files
File name Uploaded Description Edit
delete_gui.diff terry.reedy, 2013-07-13 22:34 review
Messages (14)
msg193015 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-07-13 22:34
This is a continuation of #18365, where it was discovered after the first set of pushes that
    test.support.requires('gui')
is currently insufficient (on non-Windows systems) for skipping gui tests on buildbots when they should be skipped, because the attempt to create a Tk root widget will raise TclError when there is no display available.

This bug appears to arise from the confluence of two bugs:

1. It appears that on some buildbots, support.use_resources contains 'gui' when it should not. The could be either from -ugui or -uall without  '.-gui' or ???.

2. test.support.requires starts with
     if resource == 'gui' and not _is_gui_available():
On windows, _is_gui_available() uses ctypes to determine that there really is a graphics screen (or something like that). On other systems, it just returns True, even when it should return False.

The problem was fixed for #18365 by wrapping the tkinter.Tk call with try:...except: TclErrror: raise SkipTest. Rather than put something like that workaround in every idle_test/test_xxx file, I would like to do the test once in test/test_idle and if it does not work, remove 'gui' from use_resources so requires('gui') will work properly. See patch.

test/test_ttkguionly does not use requires() (and therefore does not do the special Windows check). It first tries to import _tkinter, as does test_idle. It then calls tkinter.test.support.check_tk_availability. That either does a 'darwin'-ctypes check similar to the one for Windows in _is_gui_available or it tries to create a tk widget and looks for a TclError. test_ttkguionly then repeats the widget creation test.

This checking is ok for one file, but not for gui tests sprinkled throughout idle's test_xxx files. Perhaps requires('gui') not being dependable is why there is a separate test_ttkguionly file *and* a custom discovery function, tkinter.test.runtests.get_tests, to get only gui tests.

test/test_tk does the same except that is does not repeat the widget creation test. Both add 'gui' to use_resources when the file is run as main, though in a more convoluted manner than in test_idle. I am sure that removing 'gui' in test_idle cannot hurt since test_tk/ttk_gui-only skip anyway on a similar TclError. There is no test_turtle.py.

The check should really be done in regrtest, but I am not familiar with its code or the policy on patching it. Then tkinter tests could also dispense with their extra checks and more easily be converted to use unittest discovery. But that is not my concern here.
msg193017 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2013-07-14 01:49
See Issue8716 for the history of check_tk_availability().  At the time, the Tk tests were the only ones trying to create real Tk objects (and thus causing crashes on some buildbot configurations).  With the addition of IDLE tests that call Tk, it probably makes sense to refactor check_tk_availability() into the common test test_support/support helper module so it can be used globally.
msg193477 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-07-22 00:13
New changeset 23b0164b9c82 by Terry Jan Reedy in branch '2.7':
Issue #18441: Make test.support.requires('gui') skip when it should.
http://hg.python.org/cpython/rev/23b0164b9c82
msg193478 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-07-22 00:14
I took a look. Not fun ;-).

Pending a more permanent fix, I am applying a revised patch, first to 2.7 to make sure it works.
msg193481 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-07-22 01:25
New changeset 9f922270a929 by Terry Jan Reedy in branch '2.7':
Issue #18441: fix buildbot name-error for TclError.
http://hg.python.org/cpython/rev/9f922270a929
msg193482 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-07-22 01:59
New changeset e8b8279ca118 by Terry Jan Reedy in branch '2.7':
Backed out changeset: 23b0164b9c82 #18441 not working
http://hg.python.org/cpython/rev/e8b8279ca118
msg193484 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-07-22 02:22
(The fix was incomplete because 'delete' in the except clause should be 'remove'.) I finally realized that I could test the except clause by raising TclError explicitly just before it.

I reverted both changes because they do not seem to work, even with the additional fix (not committed). With or without patches,
python_d -m test -v test_idle  # runs 30 tests, as expected
python_d -m test -v -ugui test_idle  # run 40 tests, as expected
After raising TclError, the latter still runs 40 tests, which I did not expect.

It seems that 'gui' is not being removed from use_resources. Since debug print()s in test_idle do not show up on the console, I cannot easily see what is going on.
msg193486 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-07-22 02:34
More noise. Much of what I said above was incorrect due to version mixup. The one extra fix I did not commit might have been enough, but I will wait until I have tested it.
msg193799 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-07-27 23:09
New changeset dd9941f5fcda by Terry Jan Reedy in branch '2.7':
Issue #18441: Make test.support.requires('gui') skip when it should.
http://hg.python.org/cpython/rev/dd9941f5fcda

New changeset d9a53ab464ea by Terry Jan Reedy in branch '2.7':
Issue #18441: Correct previous patch, which hg committed before I wanted it to.
http://hg.python.org/cpython/rev/d9a53ab464ea
msg193802 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-07-28 02:29
New changeset 9a4c62c1a4c0 by Terry Jan Reedy in branch '2.7':
Issue #18441: add Mac (darwin) gui check. This is not needed today, but has been
http://hg.python.org/cpython/rev/9a4c62c1a4c0

New changeset ba5c264d67ea by Terry Jan Reedy in branch '2.7':
Issue #18441: Comment out code that will not compile because the standard
http://hg.python.org/cpython/rev/ba5c264d67ea

New changeset 6420dcd377f9 by Terry Jan Reedy in branch '2.7':
Issue #18441: whitespace
http://hg.python.org/cpython/rev/6420dcd377f9
msg193805 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-07-28 03:37
I tried to run the currently unnecessary Mac 'darwin' check in 2.7 with
        if sys.platform == 'darwin':
            from lib-tk.test.runtktests import check_tk_availability
            # tkinter.test.suppport in 3.x
            try:
                check_tk_availability()
            except unittest.SkipTest:
                raise tk.TclError

This fails because 'lib-tk' is not a legal module name. There are workarounds in test/test_tk.py and test_ttk_guionly, but I decided not to bother with them now because I would rather work on moving the function to test.support (and perhaps the package could be given a legal name). If, before that is done, someone adds a Mac buildbot that will not run gui tests but requests that they be run anyway, the above could be added with the different changes needed for 2.7 and 3.x.
msg193806 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-07-28 04:01
New changeset c3936a52f215 by Terry Jan Reedy in branch '3.3':
Issue #18441: Make test.support.requires('gui') skip when it should.
http://hg.python.org/cpython/rev/c3936a52f215

New changeset 858a72d91162 by Terry Jan Reedy in branch '2.7':
Issue #18441: Move commented out code to issue message.
http://hg.python.org/cpython/rev/858a72d91162
msg193840 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-07-28 20:26
New changeset c57b2a344097 by Terry Jan Reedy in branch '3.3':
Issue #18441: Remove check from test_text.py in 3.3,4 (already done in 2.7).
http://hg.python.org/cpython/rev/c57b2a344097
msg193967 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-07-31 00:14
This is working, so closing. I opened #18604 for consolidating gui checks in test.support.
History
Date User Action Args
2013-07-31 00:14:48terry.reedysetstatus: open -> closed
resolution: fixed
messages: + msg193967

stage: patch review -> resolved
2013-07-28 20:26:15python-devsetmessages: + msg193840
2013-07-28 04:01:24python-devsetmessages: + msg193806
2013-07-28 03:37:10terry.reedysetmessages: + msg193805
2013-07-28 02:29:34python-devsetmessages: + msg193802
2013-07-27 23:09:42python-devsetmessages: + msg193799
2013-07-22 04:12:28zach.waresetnosy: + zach.ware
2013-07-22 02:34:21terry.reedysetmessages: + msg193486
stage: commit review -> patch review
2013-07-22 02:22:07terry.reedysetmessages: + msg193484
2013-07-22 01:59:01python-devsetmessages: + msg193482
2013-07-22 01:25:27python-devsetmessages: + msg193481
2013-07-22 00:14:07terry.reedysetmessages: + msg193478
2013-07-22 00:13:47python-devsetnosy: + python-dev
messages: + msg193477
2013-07-16 02:12:05Todd.Rovitosetnosy: + JayKrish
2013-07-16 02:11:46Todd.Rovitosetnosy: + philwebster
2013-07-16 02:11:21Todd.Rovitosetnosy: + Todd.Rovito
2013-07-14 01:49:04ned.deilysetnosy: + ned.deily
messages: + msg193017
2013-07-13 22:34:07terry.reedycreate