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

test_tix cannot import _default_root after test_idle #71798

Closed
vadmium opened this issue Jul 25, 2016 · 14 comments
Closed

test_tix cannot import _default_root after test_idle #71798

vadmium opened this issue Jul 25, 2016 · 14 comments
Labels
tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error

Comments

@vadmium
Copy link
Member

vadmium commented Jul 25, 2016

BPO 27611
Nosy @terryjreedy, @vadmium, @zware, @serhiy-storchaka
PRs
  • [Do Not Merge] Convert Misc/NEWS so that it is managed by towncrier #552
  • Files
  • tix_default_root.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 = None
    closed_at = <Date 2016-09-25.14:45:24.220>
    created_at = <Date 2016-07-25.02:07:16.743>
    labels = ['type-bug', 'tests']
    title = 'test_tix cannot import _default_root after test_idle'
    updated_at = <Date 2017-03-31.16:36:37.012>
    user = 'https://github.com/vadmium'

    bugs.python.org fields:

    activity = <Date 2017-03-31.16:36:37.012>
    actor = 'dstufft'
    assignee = 'none'
    closed = True
    closed_date = <Date 2016-09-25.14:45:24.220>
    closer = 'serhiy.storchaka'
    components = ['Tests']
    creation = <Date 2016-07-25.02:07:16.743>
    creator = 'martin.panter'
    dependencies = []
    files = ['43871']
    hgrepos = []
    issue_num = 27611
    keywords = ['patch']
    message_count = 14.0
    messages = ['271206', '271219', '271220', '271221', '271225', '271226', '272822', '272823', '272839', '272844', '272846', '272860', '272862', '277376']
    nosy_count = 5.0
    nosy_names = ['terry.reedy', 'python-dev', 'martin.panter', 'zach.ware', 'serhiy.storchaka']
    pr_nums = ['552']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue27611'
    versions = ['Python 2.7', 'Python 3.5', 'Python 3.6']

    @vadmium
    Copy link
    Member Author

    vadmium commented Jul 25, 2016

    $ ./python -m unittest -v test.test_{idle,tix}
    . . .
    test_tix (unittest.loader._FailedTest) ... ERROR

    ======================================================================
    ERROR: test_tix (unittest.loader._FailedTest)
    ----------------------------------------------------------------------

    ImportError: Failed to import test module: test_tix
    Traceback (most recent call last):
      File "/media/disk/home/proj/python/cpython/Lib/unittest/loader.py", line 153, in loadTestsFromName
        module = __import__(module_name)
      File "/media/disk/home/proj/python/cpython/Lib/test/test_tix.py", line 11, in <module>
        from tkinter import tix, TclError
      File "/media/disk/home/proj/python/cpython/Lib/tkinter/tix.py", line 30, in <module>
        from tkinter import _cnfmerge, _default_root
    ImportError: cannot import name '_default_root'

    Without test_idle, test_tix is skipped for me:

    $ ./python -m unittest -v test.test_tix
    test_tix_available (test.test_tix.TestTix) ... skipped 'Tix not available'

    Reverting to before revision 064b29dde096 (bpo-24137) also fixes the failure.

    @vadmium vadmium added tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error labels Jul 25, 2016
    @serhiy-storchaka
    Copy link
    Member

    There is a bug in tix. _default_root shouldn't be imported, since this is modifiable attribute.

    @terryjreedy
    Copy link
    Member

    I could reverse the effect of test_idle calling tk.NoDefaultRoot by adding
    tk._support_default_root = 1
    tk._default_root = None
    at the very end, but I take Serhiy's comment as suggesting that there should be no need.

    @zware
    Copy link
    Member

    zware commented Jul 25, 2016

    I think there are two bugs to be fixed here:

    1. as Serhiy said, if tix needs _default_root, it should use it as tkinter._default_root instead of importing it directly

    2. test_idle should not permanently modify the tkinter namespace

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 25, 2016

    New changeset 5c76f787e695 by Terry Jan Reedy in branch 'default':
    Issue bpo-24137, issue bpo-27611: Restore tkinter after test_idle.
    https://hg.python.org/cpython/rev/5c76f787e695

    @terryjreedy
    Copy link
    Member

    bpo-24137 only patched default, so either too many versions were selected or there is another bug in test_tix. Martin and Serhiy can sort that out.

    The restore patch I pushed is based on this from tkinter.__init__
    -----

    _support_default_root = 1
    _default_root = None
    
    def NoDefaultRoot():
        """Inhibit setting of default root window.
    Call this function to inhibit that the first instance of
    Tk is used for windows without an explicit parent window.
    """
    global _support_default_root
    _support_default_root = 0
    global _default_root
    _default_root = None
    del _default_root
    

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Aug 16, 2016

    New changeset a6a248479b66 by Terry Jan Reedy in branch 'default':
    Issue bpo-27611, bpo-24137: Only change tkinter when easily restored.
    https://hg.python.org/cpython/rev/a6a248479b66

    @terryjreedy
    Copy link
    Member

    Ned Deily noticed the same problem when running with test.regrtest bpo-27714.

    I cannot reproduce this issue when running both tests either with unittest or test.regrtest.

    F:\Python\dev>36\pcbuild\win32\python_d.exe -m unittest test.test_tix test.test_idle test.test_tix
    .............................................................................................................................................................................................................................
    ----------------------------------------------------------------------
    Ran 221 tests in 3.959s

    OK
    F:\Python\dev>36\pcbuild\win32\python_d.exe -m test -ugui test_tix test_idle test_tix
    Run tests sequentially
    0:00:00 [1/3] test_tix
    0:00:00 [2/3] test_idle
    0:00:03 [3/3] test_tix
    All 3 tests OK.
    if __name__ == '__main__':
    unittest.main(verbosity=2, exit=False)
    + tk._support_default_root = 1
    + tk._default_root = None

    Zach, I don't know of any way to get either unittest or test.regrtest to run anything in test_idle after it runs the test collector *and* the tests. Do you?

    The addition of tk.NoDefaultRoot() to both IDLE and its tests was suggested by Serhiy in bpo-24137. Doing so exposed several issues in htests but no real bugs in IDLE itself. My patch to restore tkinter module only works when test_idle is run as main. So as far as that patch goes, tkinter should only changed when the test is run as main. I will move the call. I have already changed my check script to run test_idle this way.

    Otherwise, the easiest way to leave tkinter as it was when testing ends is to not change it. I am aware that tkinter tests avoid the by defining a mixin class with class methods that call and undo the call. It is used in 11 test classes within the tkinter and ttk tests. However, this refactoring would be harder to apply to IDLE tests and not sufficient in itself. It is at best a future project.

    I added a guard to IDLE's NoDefaultRoot call for when pyshell.main is broken into multiple testable functions.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Aug 16, 2016

    New changeset cddd633b959f by Terry Jan Reedy in branch '2.7':
    Issue bpo-27611: Don't import volatile attribute.
    https://hg.python.org/cpython/rev/cddd633b959f

    New changeset a42933335897 by Terry Jan Reedy in branch '3.5':
    Issue bpo-27611: Don't import volatile attribute.
    https://hg.python.org/cpython/rev/a42933335897

    @terryjreedy terryjreedy self-assigned this Aug 16, 2016
    @serhiy-storchaka
    Copy link
    Member

    Terry, you committed not full patch. The tix module still doesn't work after NoDefaultRoot().

    @terryjreedy
    Copy link
    Member

    What do you mean by 'does not work' and what did I omit?

    @serhiy-storchaka
    Copy link
    Member

    DisplayStyle() fails with AttributeError after NoDefaultRoot(). Should be made following changes:

    1. The refwindow option should have priority over _default_root.
    2. Add the master keyword-only parameter for creating styles without refwindow (3.6 only).

    @terryjreedy
    Copy link
    Member

    This issue is about test_tix failing when run after test_idle, due to the conjunction of two specific causes, one old (tix importing _default_root), one new (IDLE deleting and not restoring _default_root). The new cause exposed the old. Fixing either cause would have negated the conjunction, but at Zack's urging, I fixed both. I even did one extra bit of cleanup in moving the tkinter and os imports to the top where they belong. In my view, my patch was more than complete and this issue is fixed and should have remained closed and should be reclosed.

    Serhiy, when I wrote the tix patch, I was aware that tix remains buggy in that it will fail if a *user* imports tix, removes _default_root, and calls either of the functions that unconditionally access the attribute. Since test_tix only tests that the tix can be imported and that tix.Tk runs, there may be more bugs. And more cleanups might be needed. However, patching tix to work better is a different issue than this one. If you want, open a new issue, add some tests, and write the patch you outlined.

    @terryjreedy terryjreedy removed their assignment Aug 16, 2016
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 25, 2016

    New changeset 94a26aa1b1e0 by Serhiy Storchaka in branch '2.7':
    Issue bpo-27611: Fixed support of default root window in the Tix module.
    https://hg.python.org/cpython/rev/94a26aa1b1e0

    New changeset 7b458bcdab75 by Serhiy Storchaka in branch '3.5':
    Issue bpo-27611: Fixed support of default root window in the tkinter.tix module.
    https://hg.python.org/cpython/rev/7b458bcdab75

    New changeset ec3f5d21bec0 by Serhiy Storchaka in branch '3.6':
    Issue bpo-27611: Fixed support of default root window in the tkinter.tix module.
    https://hg.python.org/cpython/rev/ec3f5d21bec0

    New changeset 1c5e0dbcb2a0 by Serhiy Storchaka in branch 'default':
    Issue bpo-27611: Fixed support of default root window in the tkinter.tix module.
    https://hg.python.org/cpython/rev/1c5e0dbcb2a0

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

    No branches or pull requests

    4 participants