IDLE: Test UndoDelegator

[SaimadhavHeblikar]

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]

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.

[vstinner]

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]

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]

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]

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]

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.

[python-dev]

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]

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