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.

Sign up for GitHub

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

python program starting with unmatched quote spews spaces to stdout #54415

Closed
bitdancer opened this issue Oct 26, 2010 · 16 comments
Closed

python program starting with unmatched quote spews spaces to stdout #54415

bitdancer opened this issue Oct 26, 2010 · 16 comments
Assignees
@ericvsmith
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@bitdancer
Copy link
Member

BPO 10206
Nosy @ericvsmith, @benjaminp, @bitdancer, @skrah, @akheron
Files
  • issues10206_test.patch: Test case
  • issue10206.patch: test case
  • issue10206v2.patch: test case
    		•

    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/ericvsmith'
closed_at = <Date 2011-06-24.17:30:00.597>
created_at = <Date 2010-10-26.20:01:27.589>
labels = ['interpreter-core', 'type-bug']
title = 'python program starting with unmatched quote spews spaces to stdout'
updated_at = <Date 2011-06-24.17:30:00.595>
user = 'https://github.com/bitdancer'

    bugs.python.org fields:

    activity = <Date 2011-06-24.17:30:00.595>
actor = 'r.david.murray'
assignee = 'eric.smith'
closed = True
closed_date = <Date 2011-06-24.17:30:00.597>
closer = 'r.david.murray'
components = ['Interpreter Core']
creation = <Date 2010-10-26.20:01:27.589>
creator = 'r.david.murray'
dependencies = []
files = ['22427', '22441', '22443']
hgrepos = []
issue_num = 10206
keywords = ['patch']
message_count = 16.0
messages = ['119646', '119723', '119772', '119773', '119900', '138852', '138853', '138854', '138859', '138884', '138955', '138957', '138960', '138961', '138967', '138968']
nosy_count = 7.0
nosy_names = ['eric.smith', 'benjamin.peterson', 'r.david.murray', 'skrah', 'python-dev', 'petri.lehtinen', 'francismb']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue10206'
versions = ['Python 3.2', 'Python 3.3']
    @bitdancer
    Copy link
    Member Author

    To reproduce:

    python -c "'"

    (Hint: don't do this on a slow terminal)

    @bitdancer bitdancer added the type-bug An unexpected behavior, bug, or error label Oct 26, 2010
    @ericvsmith
    Copy link
    Member

    This is caused by r85814. I've got a fix, as soon as I get it in a presentable state I'll post it.

    @ericvsmith
    Copy link
    Member

    This patch fixes the issue, and makes print_error_text slightly more understandable (and efficient to boot, not that it matters).

    But I'm not convinced it's the correct solution. I think the real error might be the computation "offset" in the caller. I'm still looking at it.

    @ericvsmith ericvsmith added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Oct 28, 2010
    @ericvsmith ericvsmith self-assigned this Oct 28, 2010
    @ericvsmith
    Copy link
    Member

    Now that I think about it some more, I think my patch (although it fixes this issue and passes all tests) doesn't do what the original code does in the presence of multiple newlines. I'm going to rework it some more. I'll have another patch shortly.

    @ericvsmith
    Copy link
    Member

    I think Benjamin fixed this in r85904. I'm going to verify and add some tests.

    @akheron
    Copy link
    Member

    akheron commented Jun 23, 2011

    I'm unable to reproduce this. I checked out the commit 65614:18989ad44636 (corresponding to r85814, right?), built and ran python -c "'", but didn't get a space flood on my face. Just a normal SyntaxError.

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Jun 23, 2011

    I remember that I could reproduce it at the time. The issue was indeed
    fixed in r85904.

    @akheron
    Copy link
    Member

    akheron commented Jun 23, 2011

    By checking out the parent of r85904 I now can reproduce this.

    @akheron
    Copy link
    Member

    akheron commented Jun 23, 2011

    Attached a test case. The patch is against the current default tip.

    @bitdancer
    Copy link
    Member Author

    Hmm. I don't know that it is really necessary to cater to the particular failure mode, I was more interested in seeing a unit test that checked the correct behavior: that a syntax error is raised (by capturing the output using the tools in script_helper). If we do keep the timeout, comments explaining why would be a good idea, because it is completely un-obvious why the test is doing what it is doing...

    @francismb
    Copy link
    Mannequin

    francismb mannequin commented Jun 24, 2011

    I've attached an alternative test case.

    I'm not sure if there is a more robust way to test:
    self.assert_('SyntaxError' in err.decode('ascii', 'ignore'))

    Due the use of 'SyntaxtError' directly as string. I would prefer something like str(SyntaxtError) (or just the name of the exception
    without extra text)

    Review is welcome

    @bitdancer
    Copy link
    Member Author

    I think that self.assertRegex(err.decode('ascii', 'ignore'), 'SyntaxError') would be fine. We are extremely unlikely to change the string representation of the name of SyntaxError or omit it from the error message, so I think this is a reliable test. If you really want to be paranoid, you could use SyntaxError.__name__ as the argument to assertRegex, but there are a number of other places in the test suite where we hardcode the exception names, so I don't think it is necessary.

    @francismb
    Copy link
    Mannequin

    francismb mannequin commented Jun 24, 2011

    Just attaching the review patch

    @francismb
    Copy link
    Mannequin

    francismb mannequin commented Jun 24, 2011

    On 06/24/2011 06:07 PM, R. David Murray wrote:

    self.assertRegex(err.decode('ascii', 'ignore'), 'SyntaxError')

    I understand that's the standard way to check if a given failure
    happened in the command line or there is also a helper for that case
    (maybe something: assert_python_raises('-c', "'", SyntaxError))

    Thanks !

    Francisco

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jun 24, 2011

    New changeset 2a4764376c51 by R David Murray in branch '3.2':
    bpo-10206: add test for previously fixed bug.
    http://hg.python.org/cpython/rev/2a4764376c51

    New changeset 5ec95f46bac5 by R David Murray in branch 'default':
    Merge bpo-10206: add test for previously fixed bug.
    http://hg.python.org/cpython/rev/5ec95f46bac5

    @bitdancer
    Copy link
    Member Author

    Thanks, Petri and Francisco. Eric, I'm closing this since we now have a minimal test. If you still want to go back and add more tests based on a deeper understanding of what was broken, feel free to reopen the issue.

    @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

    @ericvsmith ericvsmith

    Labels
    interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @akheron @bitdancer @ericvsmith