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: Test UndoDelegator #65902

Closed
SaimadhavHeblikar mannequin opened this issue Jun 10, 2014 · 8 comments
Closed

IDLE: Test UndoDelegator #65902

SaimadhavHeblikar mannequin opened this issue Jun 10, 2014 · 8 comments
Assignees
Labels
topic-IDLE type-feature A feature request or enhancement

Comments

@SaimadhavHeblikar
Copy link
Mannequin

SaimadhavHeblikar mannequin commented Jun 10, 2014

BPO 21703
Nosy @terryjreedy, @vstinner
Files
  • test-undodelegator.diff
  • test-undodelegator-refleak-fixed.diff: Fixes refleak.
  • 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 2016-05-17.03:35:36.790>
    created_at = <Date 2014-06-10.10:08:31.375>
    labels = ['expert-IDLE', 'type-feature']
    title = 'IDLE: Test UndoDelegator'
    updated_at = <Date 2016-05-17.03:35:36.789>
    user = 'https://bugs.python.org/SaimadhavHeblikar'

    bugs.python.org fields:

    activity = <Date 2016-05-17.03:35:36.789>
    actor = 'terry.reedy'
    assignee = 'terry.reedy'
    closed = True
    closed_date = <Date 2016-05-17.03:35:36.790>
    closer = 'terry.reedy'
    components = ['IDLE']
    creation = <Date 2014-06-10.10:08:31.375>
    creator = 'Saimadhav.Heblikar'
    dependencies = []
    files = ['35550', '35564']
    hgrepos = []
    issue_num = 21703
    keywords = ['patch']
    message_count = 8.0
    messages = ['220158', '220168', '220239', '220243', '220244', '265749', '265753', '265754']
    nosy_count = 5.0
    nosy_names = ['terry.reedy', 'vstinner', 'jesstess', 'python-dev', 'Saimadhav.Heblikar']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue21703'
    versions = ['Python 3.5', 'Python 3.6']

    @SaimadhavHeblikar
    Copy link
    Mannequin Author

    SaimadhavHeblikar mannequin commented Jun 10, 2014

    Adds test for UndoDelegator class in idlelib.UndoDelegator.

    With the help of Victor Stinner on IRC, I managed to reduce the refleak, but the current status is:
    saimadhav@debian:~/dev/34-cpython$ ./python -m test -R 3:3 -uall test_idle
    [1/1] test_idle
    beginning 6 repetitions
    123456
    ......
    test_idle leaked [237, 237, 237] references, sum=711
    test_idle leaked [95, 98, 97] memory blocks, sum=290
    1 test failed:
    test_idle

    Any hint on where the problem is?
    ---
    I also plan to cover other helper classes in the same UndoDelegator file.

    @SaimadhavHeblikar SaimadhavHeblikar mannequin added the topic-IDLE label Jun 10, 2014
    @vstinner
    Copy link
    Member

    You should call "cls.percolator.close()" in your tearDownClass() method, before cls.text.destroy().

    By the way, I prefer to assign an attribute to None instead of using "del obj.attr".

    @terryjreedy
    Copy link
    Member

    cls.percolator.close() cut leaks in half.

    test_idle leaked [237, 237, 237, 237] references, sum=948
    test_idle leaked [95, 97, 97, 97] memory blocks, sum=386

    test_idle leaked [130, 130, 130, 130] references, sum=520
    test_idle leaked [60, 62, 62, 62] memory blocks, sum=246

    I prefixed name with 'x' and all leaks disappeared. When I run the test, a tk box is left. My guess is that something is being created with tkinter._default_root as master. I do not think it is in UndoDelegator, so I would look at Percolator, WidgetDirector, Delegator, and the new test file.

    ---
    See review comment for increasing coverage to about 80%, which is very good.

    ---
    When unittest call precedes htest.run call, need exit=False or htest is skipped by unittest exiting process. Part of testing is running tests from module. Even with the addition, htest is not running right (buttons in main windows are not right). The might be an effect of the unittest not being properly terminated. It is still true after I added the missing re import. Htest runs fine by itself. See bpo-21624 for changes. Revised code:

    def _undo_delegator(parent):  # htest #
        import re
        import tkinter as tk
        from idlelib.Percolator import Percolator
        root = tk.Tk()
        root.title("Test UndoDelegator")
        width, height, x, y = list(map(int, re.split('[x+]', parent.geometry())))
        root.geometry("+%d+%d"%(x, y + 150))
    
        text = tk.Text(root)
        text.config(height=10)
        text.pack()
        text.focus_set()
        p = Percolator(text)
        d = UndoDelegator()
        p.insertfilter(d)
    
        undo = tk.Button(root, text="Undo", command=lambda:d.undo_event(None))
        undo.pack(side='left')
        redo = tk.Button(root, text="Redo", command=lambda:d.redo_event(None))
        redo.pack(side='left')
        dump = tk.Button(root, text="Dump", command=lambda:d.dump_event(None))
        dump.pack(side='left')
    
        root.mainloop()
    
    if __name__ == "__main__":
        import unittest
        unittest.main('idlelib.idle_test.test_undodelegator', verbosity=2,
                      exit=False)
        from idlelib.idle_test.htest import run
        run(_undo_delegator)

    @SaimadhavHeblikar
    Copy link
    Mannequin Author

    SaimadhavHeblikar mannequin commented Jun 11, 2014

    It was WidgetRedirector which was leaking.
    cls.percolator.redir.close() added in tearDownClass fixes the leak.

    saimadhav@debian:~/dev/34-cpython$ ./python -m test -R :: -uall test_idle
    [1/1] test_idle
    beginning 9 repetitions
    123456789
    .........
    1 test OK.

    The attached patch also ensures that when UndoDelegator.py is run, unittest is called with exit=False, so that htest is run.
    The htest display is also corrected, and works the same way as without unittest.(with correct buttons etc).

    Only problem remaining is when UndoDelegator is run,

    can't invoke "event" command: application has been destroyed
    while executing
    "event generate $w <<ThemeChanged>>"
    (procedure "ttk::ThemeChanged" line 6)
    invoked from within
    "ttk::ThemeChanged"

    @terryjreedy
    Copy link
    Member

    Great. I will review the next patch (with the change in the comment) tomorrow. The ttk theme changed message comes up often. I believe it is specific to debug builds. Perhaps you could keep track of when it occurs so we can make a guess at the cause. I believe I asked Serhiy once and he did not then know why. I believe I grepped at least one of tkinter and _tkinter for ttk and did not fine anything. It does not cause buildbot failures, but may be involved with one of the warnings. It does suggest at least a minor bug, though, and it would be nice to fix.

    The environment change warning, at least on windows, is probably from module tkinter._fix imported in tkinter.

    @terryjreedy
    Copy link
    Member

    Before looking at this, I ran the leak tests and found a worse problem in the test suite. I opened bpo-27044 and fixed the problem.

    I did not get the 'theme changed' message (msg220243) when running the leak test in the console, but did when running it either the file or test_file as __main__. In the former case, the process is started by test.regrtest, which ends up calling unittest. This is how buildbots run. In the latter case, the process is started by the idlelib file, which calls unittest.

    @terryjreedy terryjreedy self-assigned this May 17, 2016
    @terryjreedy terryjreedy added the type-feature A feature request or enhancement label May 17, 2016
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 17, 2016

    New changeset ce1b14ed5445 by Terry Jan Reedy in branch '3.5':
    Issue bpo-21703: Add test for undo delegator. Patch most by Saimadhav Heblikar .
    https://hg.python.org/cpython/rev/ce1b14ed5445

    @terryjreedy
    Copy link
    Member

    Solved 'theme' problem by adding to end of tearDownClass
    del cls.percolator, cls.text, cls.root

    @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
    topic-IDLE type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants