classification
Title: Idle: test AutoExpand.py
Type: enhancement Stage: resolved
Components: IDLE, Tests Versions: Python 3.5, Python 3.4, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: JayKrish, Saimadhav.Heblikar, Todd.Rovito, jesstess, philwebster, python-dev, terry.reedy, zach.ware
Priority: normal Keywords: patch

Created on 2013-06-24 18:29 by JayKrish, last changed 2014-06-05 23:44 by zach.ware. This issue is now closed.

Files
File name Uploaded Description Edit
test-autoexpand.diff Saimadhav.Heblikar, 2014-06-01 15:07 review
test-autoexpand1.diff Saimadhav.Heblikar, 2014-06-02 14:16 Patch in response to msg219549 review
test-autoexp-18292.diff terry.reedy, 2014-06-03 05:47 review
test-autoexpand2.diff Saimadhav.Heblikar, 2014-06-03 11:56 review
test-autoexpand3.diff Saimadhav.Heblikar, 2014-06-04 06:22 Restructured the tests from test-autoexpand2.diff. Additional test for state reset review
Messages (11)
msg191795 - (view) Author: R. Jayakrishnan (JayKrish) * Date: 2013-06-24 18:29
Continuing the IDLE unittest framework initiated in #15392. Writing test for AutoExpand.py
msg193359 - (view) Author: R. Jayakrishnan (JayKrish) * Date: 2013-07-19 13:08
I am trying to improve the _decode function of mock Text class because most of the Idletests needs the mock text and requires some more features to handle indexes.
Created an issue IDLE:Improvements- Improving Mock_Text
http://bugs.python.org/issue18504
msg219504 - (view) Author: Saimadhav Heblikar (Saimadhav.Heblikar) * Date: 2014-06-01 15:07
Attached patch adds unittest for idlelib`s AutoExpand.

Depends on issue18504 for Text's mocking abilities.
msg219549 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-06-02 05:51
For tests that use a Text widget, I want the first version to be a gui test using tkinter.Text. This removes mock Text as an issue in writing the tests. I will not commit without running the test 'live' at least once.

Once the file (or at least a TestCase) is complete, we can then consider replacing the real Text with the mock. The gui code is commented out, rather than deleted, in case there is ever a need to switch back. See test_searchengine.py (#18489), where this was done,  (Even partial conversions are worthwhile if complete conversion is not possible.) Did I forget to say this in idle_test/README.txt? (I know I forget to mention this directly before now, Sorry ;-).

Some modules import tkinter and instantiate widgets either upon import or during testing, (This is different from taking a widget instance as an argument in .__init__.) If so  a gui-free test requires that the module be monkey-patched with mocks. For SearchEngine, the only widgets are XyzVars and TkMessageBox, for which we do have mocks. AutoExpand does not import tkinter, so it is fine as is.

In your patch, comment out "from idlelib.idle_test.mock_tk import Text" and add the needed gui code. See the commented out code in 
test_searchengine.
msg219661 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-06-03 05:47
Great start. The initial coveraqe is 90%, only missing the 5 lines in getwords in the bodies of
            if dict.get(w):
                continue
,,,
        for w in wafter: 
            if dict.get(w): 
                continue 
            words.append(w) 
            dict[w] = w 

I would like to cover those because I would like to later replace the dict instance dict (bad name!) with a set, 'seen'. 'dict.get(w)' would become 'w in seen'; 'dict[w] = w' would become 'seen.add(w)'

Two comments: 1. Using 'previous' to detect the action of 'expand' is really clever. 

2. As is typical, you wrote both too much and too little. Too much because some of the subtests are redundant, testing the same code paths repeatedly. Every subtest should have a reason. Too little because some code paths are skipped.  To cover with both before and after, with repeats and missed: insert 'aa ab xy aa ac\n ad ae xy ae ab\n', Then insert 'a' at the beginning of the second line ((2,0) as I remember). Words should be ['ac', 'aa', 'ab', 'ad' 'ae' 'a']. Expand/previous should produce those in that order. Also do a test with no before and some after. You have already done some before and no after.

I uploaded my version. See Rietveld diff for changes. Some notes:

Leave out '-' after 'test-' in file name. Makes coverage call easier.
Todays test.support patch made a line is each file obsolete.
Must keep reference to root to destroy it properly.
I added conditionals so we can easily switch between gui and mock.
These should be added to previous tests that run both ways.
test_get_words had a bug; assertCountEqual instead of assertEqual.

getprevword only sees ascii chars when finding the word prefix. On the other hand, for 3.x, \w in getwords matches unicode chars for suffixes. If possible, getprevword should use \w to determine the prefix. Then 2.7 and 3.x should work on ascii and unicode respectively, with the same code. In a second stage, we should fix this and add tests with non-ascii chars (using \unnnn in literals).
--------

self.bell() makes no sound for me. How about you?

A different non-critical nuisance. I start Idle in the debug interpreter with 'import idlelib.idle'. When root is destroyed, then the following is printed in the console interpreter after unittest is done.

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"

I determined the timing by changing to 'exit=False' in unittest.main and adding 'time.sleep(2); print('done') after the u.main() call. Do you see anything like this? (Deleting the class attributes does not fix this.) tkinter.ttk in not in sys.modules, but perhaps something in one of the other modules directly accesses ttk at the tcl level when shutting down.
msg219684 - (view) Author: Saimadhav Heblikar (Saimadhav.Heblikar) * Date: 2014-06-03 11:56
Attached a patch incorporating changes from msg219661 and test-autoexp-18292.diff

>>"I would like to cover those because ...."
Done

>>Point 2
Done

>>"self.bell() makes no sound for me. How about you?"
No sound for me as well.

>>Do you see anything like this?
Yes. Reproducing your action prints this message. I had previously seen it(when writing htest), it was random. I was unable to reproduce it then.
msg219777 - (view) Author: Roundup Robot (python-dev) Date: 2014-06-05 01:21
New changeset bdbcd0ae0bde by Terry Jan Reedy in branch '2.7':
Issue #18292: Idle - test AutoExpand. Patch by Saihadhav Heblikar.
http://hg.python.org/cpython/rev/bdbcd0ae0bde

New changeset bbdcf09e3097 by Terry Jan Reedy in branch '3.4':
Issue #18292: Idle - test AutoExpand. Patch by Saihadhav Heblikar.
http://hg.python.org/cpython/rev/bbdcf09e3097
msg219780 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-06-05 05:44
Coverage 100%. 2.7 version was easy, so committed after a few minor changes. We can revisit using the mock later.
msg219846 - (view) Author: Roundup Robot (python-dev) Date: 2014-06-05 20:58
New changeset 2567c68fb300 by Zachary Ware in branch '2.7':
Issue #18292: s/tkinter/Tkinter/
http://hg.python.org/cpython/rev/2567c68fb300
msg219853 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-06-05 22:33
Whoops. Zach, did you catch that by reading the checkin, running the test, or seeing a buildbot problem. Is not the first, what symptom on what system revealed the omission?
msg219857 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2014-06-05 23:44
Terry J. Reedy added the comment:

>
> Whoops. Zach, did you catch that by reading the checkin, running the test,
> or seeing a buildbot problem. Is not the first, what symptom on what system
> revealed the omission?
>

Buildbots; there were several red 2.7 bots and I got curious :).  See
http://buildbot.python.org/all/builders/AMD64%20Ubuntu%20LTS%202.7/builds/1091/steps/test/logs/stdio
for example, it basically looked like there was no resource guard.
History
Date User Action Args
2014-06-05 23:44:01zach.waresetmessages: + msg219857
2014-06-05 22:33:46terry.reedysetnosy: + zach.ware
messages: + msg219853
2014-06-05 22:32:52terry.reedysetmessages: - msg219852
2014-06-05 22:31:37terry.reedysetmessages: + msg219852
2014-06-05 20:58:17python-devsetmessages: + msg219846
2014-06-05 05:44:17terry.reedysetstatus: open -> closed
resolution: fixed
messages: + msg219780

stage: needs patch -> resolved
2014-06-05 01:21:34python-devsetnosy: + python-dev
messages: + msg219777
2014-06-04 06:22:35Saimadhav.Heblikarsetfiles: + test-autoexpand3.diff
2014-06-03 11:56:21Saimadhav.Heblikarsetfiles: + test-autoexpand2.diff

messages: + msg219684
2014-06-03 05:47:57terry.reedysetfiles: + test-autoexp-18292.diff

messages: + msg219661
2014-06-02 14:16:48Saimadhav.Heblikarsetfiles: + test-autoexpand1.diff
2014-06-02 05:51:53terry.reedysetmessages: + msg219549
stage: patch review -> needs patch
2014-06-02 02:56:09terry.reedysettitle: IDLE Improvements: Unit test for AutoExpand.py -> Idle: test AutoExpand.py
stage: patch review
versions: + Python 3.5, - Python 3.3
2014-06-01 15:07:29Saimadhav.Heblikarsetfiles: + test-autoexpand.diff

nosy: + Saimadhav.Heblikar, jesstess
messages: + msg219504

keywords: + patch
2013-07-19 13:08:39JayKrishsetmessages: + msg193359
2013-07-01 16:09:42Todd.Rovitosetnosy: + Todd.Rovito
2013-06-29 19:27:01philwebstersetnosy: + philwebster
2013-06-28 18:19:24terry.reedysetnosy: + terry.reedy
2013-06-24 18:29:14JayKrishcreate