This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: IDLE Unit test for IdleHistory.py
Type: enhancement Stage: resolved
Components: IDLE Versions: Python 3.3, Python 3.4, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder: IdleHistory.History: eliminate unused parameter; other cleanup.
View: 18732
Assigned To: terry.reedy Nosy List: JayKrish, Todd.Rovito, ezio.melotti, philwebster, python-dev, terry.reedy
Priority: normal Keywords: patch

Created on 2013-07-10 22:03 by JayKrish, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
test_idlehistory.patch JayKrish, 2013-07-10 22:03 idlehistoryTest using mock Text review
test_idlehistory1.patch JayKrish, 2013-07-14 06:50 review
test_idlehistory2.patch JayKrish, 2013-07-18 10:24 review
test_idlehistory3.diff terry.reedy, 2013-08-14 02:43 tjr version review
test_idlehistory4.diff terry.reedy, 2013-08-14 23:40 review
Messages (11)
msg192840 - (view) Author: R. Jayakrishnan (JayKrish) * Date: 2013-07-10 22:03
Continuing the IDLE unittest framework #15392, unit test for IdleHistory.py. This test depends on mockText and uses the patch by Terry at issue #18365.
Because the mockText is still about to commit, first patch http://bugs.python.org/file30865/mock_text2.diff and then use test_idlehistory.patch to run test_idlehistory. 
Once the mock Text get committed I will improve the patch according to.
msg193030 - (view) Author: R. Jayakrishnan (JayKrish) * Date: 2013-07-14 06:50
Now we have the mockText, So I updated my test for Idlehistory.
The submitted patch2 contains tests for two methods in Idlehistory (history_store and History_do).
I hope the tests for history_store are done. But the history_do method tests fails.
I explored a bit on the failure trace,
History_do method is called by PyShell.py
before it calls, pyShell does self.text.mark_set("iomark", "insert") and passes the Text to IdleHistory.
Now Idlehistory uses 'iomark' as an index.
Mock Text's _decode only handles specific input indexes (as it mentions), that means this 'iomark' index failed to all the logic of _decode method and finally crosses   line, char = index.split('.') line which eventually raises a ValueError: need more than 1 value to unpack.

One option is to create an issue to futher develop mock Text and implement mark_set also the _decode and so on,
or Is it sounds like making stuffs more complex?, so we may decide the implemented tests are adequate enough for now.
msg193153 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-07-16 05:40
As I said on one of the other issues, which I hope you read, we have the option of gui tests when mocking becomes too complicated. So at least for now, put the test that requires marks in a separate test case with requires('gui') in setUPClass.

The problem with a full implementation of marks is that they change position when text is inserted or deleted before their position. Also, some mark functions are easier if they are stored in a dict, while one function and the movement operations are easier if they are stored in a sorted list. A partial implementation that allowed only 1 or 2 marks would be easier that a full implementation allowing any number. Mock marks that do not move (and don't need to because of no inserts or deletes between definition and use) would also be easy. So I want to see how marks need to be used for testing before doing anything.

Even when a mock is used, we can temporarily use the tk version to verify that the mock is working right. I did that in the patch for #18279. See the commented out code in
http://hg.python.org/cpython/rev/22ce68d98345.
msg193278 - (view) Author: R. Jayakrishnan (JayKrish) * Date: 2013-07-18 10:24
Thanks Terry for "we have the option of gui tests when mocking becomes too complicated"

I put two separate classes IdleHistoryMockTest and IdleHistoryTKTest for mock and TK text usages and used TK Text to test IdleHistory.history_do function. This worked for me and hopefully IMHO the logic behind history_do is tested enough for now.
msg193468 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-07-21 21:19
Resolution is for when the issue is closed, and I expect this to be closed as 'fixed'.
msg195109 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-08-13 23:52
New changeset 0bb9346665e9 by Terry Jan Reedy in branch '3.3':
Issue #18425: Add docstrings to IdleHistory.py.  Remove redundant 'history_'
http://hg.python.org/cpython/rev/0bb9346665e9

New changeset 22d7c755163d by Terry Jan Reedy in branch '2.7':
Issue #18425: Add docstrings to IdleHistory.py.  Remove redundant 'history_'
http://hg.python.org/cpython/rev/22d7c755163d
msg195119 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-08-14 02:41
IdleHistory.py is identical in 2.7 and 3.3. It is only imported in PyShell.py (within class PyShell, line 852 in 3.3). History is only initialized once, with the default output_sep. See #18732 for deleting the unneeded parameter and associated cruft.

The pushed patch adds docstrings and a couple of comments and removes most of the redundant prefixes. It does not change any logic.

The attached patch started as JK's patch with names changed and Ezio's changes. All test methods pass. There are three chunks of History.fetch that are not covered. Two require cyclic=False. The other requires special situations with text or cursor changes after one call to fetch but before the next. I will add these soon and push.
msg195228 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-08-14 23:40
Moved most initialization to method setUp to prevent cascading of test failures. Added info about History.fetch and FetchTest.fetch_test methods. Added tests for non-cyclic and special behavior and for fetch event wrappers. Coverage is essentially 100%.
msg195271 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-08-15 18:33
New changeset 0e9d41edb2e4 by Terry Jan Reedy in branch '2.7':
Issue #18425: Unittests for idlelib.IdleHistory. First patch by R. Jayakrishnan.
http://hg.python.org/cpython/rev/0e9d41edb2e4

New changeset c4cac5d73e9d by Terry Jan Reedy in branch '3.3':
Issue #18425: Unittests for idlelib.IdleHistory. First patch by R. Jayakrishnan.
http://hg.python.org/cpython/rev/c4cac5d73e9d
msg195594 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-08-19 02:17
Ezio: grepping test/*.py for (name)'= self.assertEqual' gets 182 hits in 15 different files. If you are generically opposed to name localization, please take your case to pydev. I also found 'unless = self.assertTrue' and am sure I have seen others in the stdlib and test files.

For (name), there is 1 use of 'e' and 'asseq', 5 of 'equal', and 175 of 'eq'. (In one place, 'eq' was only used once after being defined. I would not bother.) If you have a strong preference for 'eq', I will consider it, though it almost seems too light.
msg195595 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-08-19 02:19
> If you are generically opposed to name localization, please take your
> case to pydev.

I'm not strongly opposed to it, I probably even use it sometimes.  In that case I wouldn't have bothered, but YMMV.
History
Date User Action Args
2022-04-11 14:57:47adminsetgithub: 62625
2013-08-19 02:19:21ezio.melottisetmessages: + msg195595
2013-08-19 02:17:03terry.reedysetmessages: + msg195594
2013-08-15 19:12:39terry.reedysetstatus: open -> closed
2013-08-15 19:11:33terry.reedysetsuperseder: IdleHistory.History: eliminate unused parameter; other cleanup.
resolution: fixed
2013-08-15 18:33:09python-devsetmessages: + msg195271
2013-08-14 23:40:20terry.reedysetfiles: + test_idlehistory4.diff

messages: + msg195228
stage: patch review -> resolved
2013-08-14 02:43:08terry.reedysetfiles: + test_idlehistory3.diff
2013-08-14 02:41:10terry.reedysetmessages: + msg195119
2013-08-13 23:52:19python-devsetnosy: + python-dev
messages: + msg195109
2013-08-13 21:20:50terry.reedylinkissue18732 dependencies
2013-08-08 17:29:22ezio.melottisetnosy: + ezio.melotti
2013-07-21 21:19:14terry.reedysetassignee: terry.reedy
resolution: works for me -> (no value)
messages: + msg193468
stage: patch review
2013-07-21 20:24:53JayKrishsetresolution: works for me
2013-07-18 10:24:16JayKrishsetfiles: + test_idlehistory2.patch

messages: + msg193278
2013-07-16 05:40:45terry.reedysetmessages: + msg193153
2013-07-14 06:50:45JayKrishsetfiles: + test_idlehistory1.patch

messages: + msg193030
2013-07-10 23:47:43Todd.Rovitosetnosy: + Todd.Rovito
2013-07-10 23:06:29philwebstersetnosy: + philwebster
2013-07-10 22:03:58JayKrishcreate