IDLE - Test hyperparser

[SaimadhavHeblikar]

BPO	21686
Nosy	@terryjreedy, @taleinat
Files	test-hyperparser.diff test-hyperparser-v1.diff hyperparser_update.diff test-hp-34-push.diff

^{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 = None
closed_at = <Date 2014-06-16.23:17:46.141>
created_at = <Date 2014-06-07.15:01:54.405>
labels = ['expert-IDLE']
title = 'IDLE - Test hyperparser'
updated_at = <Date 2014-06-17.20:04:40.406>
user = 'https://bugs.python.org/SaimadhavHeblikar'

bugs.python.org fields:

activity = <Date 2014-06-17.20:04:40.406>
actor = 'terry.reedy'
assignee = 'none'
closed = True
closed_date = <Date 2014-06-16.23:17:46.141>
closer = 'terry.reedy'
components = ['IDLE']
creation = <Date 2014-06-07.15:01:54.405>
creator = 'Saimadhav.Heblikar'
dependencies = []
files = ['35512', '35630', '35640', '35661']
hgrepos = []
issue_num = 21686
keywords = ['patch']
message_count = 13.0
messages = ['219942', '220510', '220515', '220548', '220549', '220595', '220700', '220755', '220780', '220781', '220798', '220801', '220870']
nosy_count = 5.0
nosy_names = ['terry.reedy', 'taleinat', 'jesstess', 'python-dev', 'Saimadhav.Heblikar']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue21686'
versions = ['Python 2.7', 'Python 3.4', 'Python 3.5']

[SaimadhavHeblikar]

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.

[taleinat]

Which lines are not hit?

[terryjreedy]

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.

[taleinat]

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().

[taleinat]

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.

[terryjreedy]

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 bpo-21756 (finding closers) and new bpo-21765 -- making HyperParser work with non-ascii identifiers, will be added as part of any code fix patches.

[python-dev]

New changeset 5063df721985 by Terry Jan Reedy in branch '2.7':
Issue bpo-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 bpo-21686: idlelib/HyperParser.py - Update docstrings and comments and
http://hg.python.org/cpython/rev/ddf15cf0db72

[terryjreedy]

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.

[python-dev]

New changeset 59730aecce9b by Terry Jan Reedy in branch '2.7':
Issue bpo-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 bpo-21686: add unittest for idlelib.HyperParser. Original patch by Saimadhav
http://hg.python.org/cpython/rev/2fc89d2230a2

[terryjreedy]

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.

[taleinat]

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.

[terryjreedy]

bpo-21787 Idle: make 3.x Hyperparser.get_expression recognize ...

[terryjreedy]

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.