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

Idle: Make test.support.requires('gui') skip when it should. #62641

Closed
terryjreedy opened this issue Jul 13, 2013 · 14 comments
Closed

Idle: Make test.support.requires('gui') skip when it should. #62641

terryjreedy opened this issue Jul 13, 2013 · 14 comments
Assignees
Labels
type-bug An unexpected behavior, bug, or error

Comments

@terryjreedy
Copy link
Member

BPO 18441
Nosy @terryjreedy, @ned-deily, @bitdancer, @rovitotv, @zware
Files
  • delete_gui.diff
  • 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/terryjreedy'
    closed_at = <Date 2013-07-31.00:14:48.557>
    created_at = <Date 2013-07-13.22:34:07.473>
    labels = ['type-bug']
    title = "Idle: Make test.support.requires('gui') skip when it should."
    updated_at = <Date 2013-07-31.00:14:48.363>
    user = 'https://github.com/terryjreedy'

    bugs.python.org fields:

    activity = <Date 2013-07-31.00:14:48.363>
    actor = 'terry.reedy'
    assignee = 'terry.reedy'
    closed = True
    closed_date = <Date 2013-07-31.00:14:48.557>
    closer = 'terry.reedy'
    components = []
    creation = <Date 2013-07-13.22:34:07.473>
    creator = 'terry.reedy'
    dependencies = []
    files = ['30909']
    hgrepos = []
    issue_num = 18441
    keywords = ['patch']
    message_count = 14.0
    messages = ['193015', '193017', '193477', '193478', '193481', '193482', '193484', '193486', '193799', '193802', '193805', '193806', '193840', '193967']
    nosy_count = 8.0
    nosy_names = ['terry.reedy', 'ned.deily', 'r.david.murray', 'Todd.Rovito', 'python-dev', 'zach.ware', 'JayKrish', 'philwebster']
    pr_nums = []
    priority = 'high'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue18441'
    versions = ['Python 2.7', 'Python 3.3', 'Python 3.4']

    @terryjreedy
    Copy link
    Member Author

    This is a continuation of bpo-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 bpo-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.

    @terryjreedy terryjreedy self-assigned this Jul 13, 2013
    @terryjreedy terryjreedy added the type-bug An unexpected behavior, bug, or error label Jul 13, 2013
    @ned-deily
    Copy link
    Member

    See bpo-8716 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.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 22, 2013

    New changeset 23b0164b9c82 by Terry Jan Reedy in branch '2.7':
    Issue bpo-18441: Make test.support.requires('gui') skip when it should.
    http://hg.python.org/cpython/rev/23b0164b9c82

    @terryjreedy
    Copy link
    Member Author

    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.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 22, 2013

    New changeset 9f922270a929 by Terry Jan Reedy in branch '2.7':
    Issue bpo-18441: fix buildbot name-error for TclError.
    http://hg.python.org/cpython/rev/9f922270a929

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 22, 2013

    New changeset e8b8279ca118 by Terry Jan Reedy in branch '2.7':
    Backed out changeset: 23b0164b9c82 bpo-18441 not working
    http://hg.python.org/cpython/rev/e8b8279ca118

    @terryjreedy
    Copy link
    Member Author

    (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.

    @terryjreedy
    Copy link
    Member Author

    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.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 27, 2013

    New changeset dd9941f5fcda by Terry Jan Reedy in branch '2.7':
    Issue bpo-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 bpo-18441: Correct previous patch, which hg committed before I wanted it to.
    http://hg.python.org/cpython/rev/d9a53ab464ea

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 28, 2013

    New changeset 9a4c62c1a4c0 by Terry Jan Reedy in branch '2.7':
    Issue bpo-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 bpo-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 bpo-18441: whitespace
    http://hg.python.org/cpython/rev/6420dcd377f9

    @terryjreedy
    Copy link
    Member Author

    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.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 28, 2013

    New changeset c3936a52f215 by Terry Jan Reedy in branch '3.3':
    Issue bpo-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 bpo-18441: Move commented out code to issue message.
    http://hg.python.org/cpython/rev/858a72d91162

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 28, 2013

    New changeset c57b2a344097 by Terry Jan Reedy in branch '3.3':
    Issue bpo-18441: Remove check from test_text.py in 3.3,4 (already done in 2.7).
    http://hg.python.org/cpython/rev/c57b2a344097

    @terryjreedy
    Copy link
    Member Author

    This is working, so closing. I opened bpo-18604 for consolidating gui checks in test.support.

    @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
    type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants