classification
Title: IDLE - Test hyperparser
Type: Stage: resolved
Components: IDLE Versions: Python 3.5, Python 3.4, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: jesstess, python-dev, sahutd, taleinat, terry.reedy
Priority: normal Keywords: patch

Created on 2014-06-07 15:01 by sahutd, last changed 2014-06-17 20:04 by terry.reedy. This issue is now closed.

Files
File name Uploaded Description Edit
test-hyperparser.diff sahutd, 2014-06-07 15:01 review
test-hyperparser-v1.diff sahutd, 2014-06-14 14:33 review
hyperparser_update.diff terry.reedy, 2014-06-14 23:20 review
test-hp-34-push.diff terry.reedy, 2014-06-16 23:17 review
Messages (13)
msg219942 - (view) Author: Saimadhav Heblikar (sahutd) * Date: 2014-06-07 15:01
Test for idlelib.HyperParser
5 lines not tested. Any suggestion on how to hit those lines welcome.
Will submit backport 2.7 once the patch for 3.4 is OK.
msg220510 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2014-06-13 22:59
Which lines are not hit?
msg220515 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-06-14 00:01
There are 7 scattered conditionals which only evaluate as one of True or False during tests and 5 lines not hit at all as a result. I am emailing instructions to install and use coverage package.
msg220548 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2014-06-14 12:25
Here are details how to trigger all of the uncovered code lines and branches, listed by line.


Uncovered lines:

Line 159: This catches would-be identifiers (variable names) which are keywords or begin with a character that can't be first (e.g. a digit). We should have at least one test for each of these cases.

Lines 196-198: These strip comments from inside an expression. We should have a test with sample code with a multi-line expression with a comment inside it.

Line 225: Triggered when trying to find an expression including parentheses or brackets when they aren't closed, e.g. "[x for x in ".


Uncovered branches (except those which just cause the above):

Lines 32, 42: To trigger, have a block of code over 50 lines long and create a HyperParser at the last line (not inside any other block).

Line 236: Have a string literal inside brackets or parens, and call get_expression().
msg220549 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2014-06-14 12:28
Regarding lines 32 & 42, a test could also set editwin.num_context_lines to allow triggering with smaller code blocks. It's value needs to be a list of integers, the last of which is incredibly large, perhaps sys.maxint.
msg220595 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-06-14 23:20
Since ParenMatch and fixes* to HyperParser depend on this issue, I would like to do this first. For HyperParser.py, I would like to first commit a clean-up only patch. The test_hyperparser patch should then only add the 'if __name__' clause at the end.

I would like to apply the attached tomorrow, pending comments. It
adds docstrings and properly formats them per PEP 8; reformats comment blocks, and properly wrap lines without '\', eliminating occurences already present.

The v1 tests continue to pass once 'before the analyzed' is changed in the test file to 'precedes'. 'Index' or 'statement' might be better targets. 

*After test_hyperparser.py is added, additional tests for #21756 (finding closers) and new #21765 -- making HyperParser work with non-ascii identifiers, will be added as part of any code fix patches.
msg220700 - (view) Author: Roundup Robot (python-dev) Date: 2014-06-16 06:41
New changeset 5063df721985 by Terry Jan Reedy in branch '2.7':
Issue #21686: idlelib/HyperParser.py - Update docstrings and comments and
http://hg.python.org/cpython/rev/5063df721985

New changeset ddf15cf0db72 by Terry Jan Reedy in branch '3.4':
Issue #21686: idlelib/HyperParser.py - Update docstrings and comments and
http://hg.python.org/cpython/rev/ddf15cf0db72
msg220755 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-06-16 20:31
Coverage is now '99%'. Unless I see problems that are more than trivial, I am going to polish the patch and commit.

I believe partial coverage of "for context in editwin.num_context_lines:" is because the iterable is never empty. But it never will be in practice and I believe the code is not intended to work if it is. So I ignore this.

I believe partial coverage of deeply nested 
                    if rawtext[pos] in "'\"": 
is because this is reached with rawtext[pop] *not* in "'\"". Reading up, to reach this point, rawtext[pos] must also not be in "([":,
   bracketing[brck_index][0] == brck_limit
must never become true inside the while look (because it breaks), and pos == brck_limit must be true. I don't know if this is all possible. Tal, if you find a triggering test case after I commit, it can be added later.
msg220780 - (view) Author: Roundup Robot (python-dev) Date: 2014-06-16 23:01
New changeset 59730aecce9b by Terry Jan Reedy in branch '2.7':
Issue #21686: add unittest for idlelib.HyperParser.  Original patch by Saimadhav
http://hg.python.org/cpython/rev/59730aecce9b

New changeset 2fc89d2230a2 by Terry Jan Reedy in branch '3.4':
Issue #21686: add unittest for idlelib.HyperParser.  Original patch by Saimadhav
http://hg.python.org/cpython/rev/2fc89d2230a2
msg220781 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-06-16 23:17
Attached is the 3.4 patch as pushed. The main 'invisible' change, other than the review comment, was to create the code string just once.

I discovered that the earlier version had a bug in saying 'ok' to the 3.x HyperParser bug of no longer recognizing 'False', etc, as identifiers due to being made keywords. The buggy test properly failed on 2.7. I did the easy fix and split the bad test into two.

Since the 2.7 and 3.x versions of HyperParser were identical before the fix, the 3.x version must also not recognize the new ... Ellipsis literal. Tal, do you think doing so would be sensibly possible? (IE, would it be worth a new issue?)
  ...
  x = ...
  print(...)
are all legal 3.x expressions.
msg220798 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2014-06-17 06:48
Ouch. I hadn't thought about the Ellipsis literal!

That would be sensibly possible, yes, but not simple. I think opening a separate issue for this would be prudent.
msg220801 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-06-17 07:01
#21787 Idle: make 3.x  Hyperparser.get_expression recognize ...
msg220870 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-06-17 20:04
I forgot to mention that with this particular shutdown order
    @classmethod
    def tearDownClass(cls):
        del cls.text, cls.editwin
        cls.root.destroy()
        del cls.root
the shutdown warning message we have seen about not handling an event  went away. Even though editwin is a dummy, it had to be deleted -- I suspect to delete its reference to the gui text widget.
History
Date User Action Args
2014-06-17 20:04:40terry.reedysetmessages: + msg220870
2014-06-17 07:01:11terry.reedysetmessages: + msg220801
2014-06-17 06:48:36taleinatsetmessages: + msg220798
2014-06-16 23:17:46terry.reedysetstatus: open -> closed
files: + test-hp-34-push.diff
messages: + msg220781

resolution: fixed
stage: resolved
2014-06-16 23:01:35python-devsetmessages: + msg220780
2014-06-16 20:31:34terry.reedysetmessages: + msg220755
2014-06-16 06:41:07python-devsetnosy: + python-dev
messages: + msg220700
2014-06-14 23:20:17terry.reedysetfiles: + hyperparser_update.diff

messages: + msg220595
2014-06-14 23:19:34terry.reedylinkissue21765 dependencies
2014-06-14 23:18:28terry.reedylinkissue21756 dependencies
2014-06-14 14:33:00sahutdsetfiles: + test-hyperparser-v1.diff
2014-06-14 12:28:03taleinatsetmessages: + msg220549
2014-06-14 12:25:10taleinatsetmessages: + msg220548
2014-06-14 00:01:20terry.reedysetmessages: + msg220515
2014-06-13 22:59:35taleinatsetnosy: + taleinat
messages: + msg220510
2014-06-07 15:01:54sahutdcreate