Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

IDLE: Add tests for pyparse #77055

Closed
csabella opened this issue Feb 19, 2018 · 9 comments
Closed

IDLE: Add tests for pyparse #77055

csabella opened this issue Feb 19, 2018 · 9 comments
Assignees
@terryjreedy
Labels
3.7 (EOL) end of life 3.8 only security fixes topic-IDLE type-feature A feature request or enhancement

Comments

@csabella
Copy link
Contributor

BPO 32874
Nosy @terryjreedy, @csabella, @miss-islington
PRs
  • bpo-32874: IDLE: add tests for pyparse #5755
  • [3.7] bpo-32874: IDLE: add tests for pyparse (GH-5755) #5803
  • [3.6] bpo-32874: IDLE: add tests for pyparse (GH-5755) #5804
    		•

    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 = 'https://github.com/terryjreedy'
closed_at = <Date 2018-03-04.05:36:10.734>
created_at = <Date 2018-02-19.15:56:06.060>
labels = ['3.8', 'expert-IDLE', 'type-feature', '3.7']
title = 'IDLE: Add tests for pyparse'
updated_at = <Date 2018-03-04.05:36:10.733>
user = 'https://github.com/csabella'

    bugs.python.org fields:

    activity = <Date 2018-03-04.05:36:10.733>
actor = 'terry.reedy'
assignee = 'terry.reedy'
closed = True
closed_date = <Date 2018-03-04.05:36:10.734>
closer = 'terry.reedy'
components = ['IDLE']
creation = <Date 2018-02-19.15:56:06.060>
creator = 'cheryl.sabella'
dependencies = []
files = []
hgrepos = []
issue_num = 32874
keywords = ['patch']
message_count = 9.0
messages = ['312352', '312353', '312375', '312428', '312438', '312440', '312529', '312531', '312532']
nosy_count = 3.0
nosy_names = ['terry.reedy', 'cheryl.sabella', 'miss-islington']
pr_nums = ['5755', '5803', '5804']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue32874'
versions = ['Python 3.6', 'Python 3.7', 'Python 3.8']
    @csabella
    Copy link
    Contributor Author

    Add unit tests for pyparse.py in IDLE.

    @csabella csabella added 3.7 (EOL) end of life 3.8 only security fixes labels Feb 19, 2018
    @csabella csabella added topic-IDLE type-feature A feature request or enhancement labels Feb 19, 2018
    @csabella
    Copy link
    Contributor Author

    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.

    @terryjreedy
    Copy link
    Member

    My only contact with pyparse has been bpo-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.

    @csabella
    Copy link
    Contributor Author

    Thanks for pointing out bpo-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.

    @terryjreedy
    Copy link
    Member

    Respone to msg312353: Yes, let us restrict this to testing pyparse code as is. I opened issue bpo-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.

    @terryjreedy
    Copy link
    Member

    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 bpo-32880.

    In bpo-21765, I mentioned looking at ColorDelegator and UndoDelegator. I never did that, but added this to my list of possible issues.

    @terryjreedy
    Copy link
    Member

    New changeset c84cf6c by Terry Jan Reedy (Cheryl Sabella) in branch 'master':
    bpo-32874: IDLE: add tests for pyparse (GH-5755)
    c84cf6c

    @miss-islington
    Copy link
    Contributor

    New changeset c59bc98 by Miss Islington (bot) in branch '3.7':
    bpo-32874: IDLE: add tests for pyparse (GH-5755)
    c59bc98

    @miss-islington
    Copy link
    Contributor

    New changeset 52064c3 by Miss Islington (bot) in branch '3.6':
    bpo-32874: IDLE: add tests for pyparse (GH-5755)
    52064c3

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @terryjreedy terryjreedy

    Labels
    3.7 (EOL) end of life 3.8 only security fixes topic-IDLE type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @terryjreedy @csabella @miss-islington