classification
Title: IDLE: Add tests for pyparse
Type: enhancement Stage: resolved
Components: IDLE Versions: Python 3.8, Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: terry.reedy Nosy List: cheryl.sabella, miss-islington, terry.reedy
Priority: normal Keywords: patch

Created on 2018-02-19 15:56 by cheryl.sabella, last changed 2018-03-04 05:36 by terry.reedy. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 5755 merged cheryl.sabella, 2018-02-19 16:02
PR 5803 merged miss-islington, 2018-02-22 03:48
PR 5804 merged miss-islington, 2018-02-22 03:49
Messages (9)
msg312352 - (view) Author: Cheryl Sabella (cheryl.sabella) * (Python committer) Date: 2018-02-19 15:56
Add unit tests for pyparse.py in IDLE.
msg312353 - (view) Author: Cheryl Sabella (cheryl.sabella) * (Python committer) Date: 2018-02-19 16:01
I also moved existing comments in pyparse.py to be docstrings.

Adding the tests revealed a bug in the initialization of self.lastopenbracketpos, but I didn't make any changes to fix it.  With the bug, the tests weren't repeatable, so I modified the tests to work with the bug in place.
msg312375 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2018-02-19 21:52
My only contact with pyparse has been #21765, which modified hyperparser and pyparse to support unicode identifiers.  It also added tests for hyperparser, but not pyparse (msg223150: "it seems to be working as expected").  Thanks for filling in this hole.
msg312428 - (view) Author: Cheryl Sabella (cheryl.sabella) * (Python committer) Date: 2018-02-20 18:06
Thanks for pointing out #21765 - very interesting reading.  :-)

Would the new str.isascii() be helpful or would it be too early to use something only available in 3.7?  It would seem that and combinations of `if isascii() and isidentifier()` might be interesting to benchmark against some of the current code.
msg312438 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2018-02-20 21:46
Respone to msg312353: Yes, let us restrict this to testing pyparse code as is.  I opened issue #32880 for changing the code.  My followup post discusses parse variable initialization.

Putting instance variable defaults in class attributes is a known practice.  But this is usually done when the 'default' is the most common value, not when the class attribute is always masked, before access, by an instance attribute of the same name.

One could claim that it is buggy to not create a new Parser instance for each subtest.  But the class appears to be designed for instance reuse, by having a separate set string method and by never looking at string-specific attributes until set from the string.

We could instead say that the bug is the test checking an undefined value. I rejected the option of not looking when test.lastopenbracketpos is None and instead suggest on the new issue that this instance attribute always be freshly set, like all the others.
msg312440 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2018-02-20 22:11
Response to msg312428: I would generally prefer to put off using 3.x feature in module m until after we think we are done patching m for 3.(x-1), but do so before 3.x.0 release.  When 3.x-1 went to security status a week after the 3.x release, this was not much an issue.

In this case, we could use 'isascii' freely after
3.7+: isascii = str.isascii
3.6:  def isascii(s): return all(ord(c) < 128 for c in s)

Concrete code change proposals, including in hyperparser, should go on #32880.

In #21765, I mentioned looking at ColorDelegator and UndoDelegator.  I never did that, but added this to my list of possible issues.
msg312529 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2018-02-22 03:48
New changeset c84cf6c03fce1fb73bfaf91d7909f1c2708f14a2 by Terry Jan Reedy (Cheryl Sabella) in branch 'master':
bpo-32874: IDLE: add tests for pyparse (GH-5755)
https://github.com/python/cpython/commit/c84cf6c03fce1fb73bfaf91d7909f1c2708f14a2
msg312531 - (view) Author: miss-islington (miss-islington) Date: 2018-02-22 04:09
New changeset c59bc98fb26ff1a2361f168a97da4a5f6c1e5b43 by Miss Islington (bot) in branch '3.7':
bpo-32874: IDLE: add tests for pyparse (GH-5755)
https://github.com/python/cpython/commit/c59bc98fb26ff1a2361f168a97da4a5f6c1e5b43
msg312532 - (view) Author: miss-islington (miss-islington) Date: 2018-02-22 04:34
New changeset 52064c3d8a62c8c14967ec1e004927e9297bb62c by Miss Islington (bot) in branch '3.6':
bpo-32874: IDLE: add tests for pyparse (GH-5755)
https://github.com/python/cpython/commit/52064c3d8a62c8c14967ec1e004927e9297bb62c
History
Date User Action Args
2018-03-04 05:36:10terry.reedysetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2018-02-22 04:34:45miss-islingtonsetmessages: + msg312532
2018-02-22 04:09:42miss-islingtonsetnosy: + miss-islington
messages: + msg312531
2018-02-22 03:49:47miss-islingtonsetpull_requests: + pull_request5582
2018-02-22 03:48:49miss-islingtonsetpull_requests: + pull_request5581
2018-02-22 03:48:44terry.reedysetmessages: + msg312529
2018-02-20 22:11:59terry.reedysetmessages: + msg312440
2018-02-20 21:46:41terry.reedysetmessages: + msg312438
2018-02-20 18:06:30cheryl.sabellasetmessages: + msg312428
2018-02-19 21:52:06terry.reedysetmessages: + msg312375
versions: + Python 3.6
2018-02-19 16:02:12cheryl.sabellasetkeywords: + patch
stage: patch review
pull_requests: + pull_request5533
2018-02-19 16:01:02cheryl.sabellasetmessages: + msg312353
2018-02-19 15:56:06cheryl.sabellacreate