classification
Title: IDLE: Test UndoDelegator
Type: enhancement Stage: resolved
Components: IDLE Versions: Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: terry.reedy Nosy List: Saimadhav.Heblikar, jesstess, python-dev, terry.reedy, vstinner
Priority: normal Keywords: patch

Created on 2014-06-10 10:08 by Saimadhav.Heblikar, last changed 2016-05-17 03:35 by terry.reedy. This issue is now closed.

Files
File name Uploaded Description Edit
test-undodelegator.diff Saimadhav.Heblikar, 2014-06-10 10:08 review
test-undodelegator-refleak-fixed.diff Saimadhav.Heblikar, 2014-06-11 07:28 Fixes refleak. review
Messages (8)
msg220158 - (view) Author: Saimadhav Heblikar (Saimadhav.Heblikar) * Date: 2014-06-10 10:08
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.
msg220168 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-06-10 13:52
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".
msg220239 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-06-11 06:43
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 #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)
msg220243 - (view) Author: Saimadhav Heblikar (Saimadhav.Heblikar) * Date: 2014-06-11 07:28
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"
msg220244 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-06-11 07:58
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.
msg265749 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-05-17 02:48
Before looking at this, I ran the leak tests and found a worse problem in the test suite.  I opened #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.
msg265753 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-05-17 03:33
New changeset ce1b14ed5445 by Terry Jan Reedy in branch '3.5':
Issue #21703: Add test for undo delegator.  Patch most by Saimadhav Heblikar .
https://hg.python.org/cpython/rev/ce1b14ed5445
msg265754 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-05-17 03:35
Solved 'theme' problem by adding to end of tearDownClass
        del cls.percolator, cls.text, cls.root
History
Date User Action Args
2016-05-17 03:35:36terry.reedysetstatus: open -> closed
resolution: fixed
messages: + msg265754

stage: commit review -> resolved
2016-05-17 03:33:05python-devsetnosy: + python-dev
messages: + msg265753
2016-05-17 02:48:34terry.reedysetversions: + Python 3.6, - Python 2.7, Python 3.4
messages: + msg265749

assignee: terry.reedy
type: enhancement
stage: commit review
2014-06-11 07:58:18terry.reedysetmessages: + msg220244
2014-06-11 07:28:47Saimadhav.Heblikarsetfiles: + test-undodelegator-refleak-fixed.diff

messages: + msg220243
2014-06-11 06:43:21terry.reedysetmessages: + msg220239
2014-06-10 13:52:26vstinnersetnosy: + vstinner
messages: + msg220168
2014-06-10 10:08:31Saimadhav.Heblikarcreate