| msg119646 - (view) |
Author: R. David Murray (r.david.murray) *  |
Date: 2010-10-26 20:01 |
| To reproduce: python -c "'" (Hint: don't do this on a slow terminal) |
|
|
| msg119723 - (view) |
Author: Eric V. Smith (eric.smith) * |
Date: 2010-10-27 17:46 |
| This is caused by r85814. I've got a fix, as soon as I get it in a presentable state I'll post it. |
|
|
| msg119772 - (view) |
Author: Eric V. Smith (eric.smith) * |
Date: 2010-10-28 11:43 |
| 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. |
|
|
| msg119773 - (view) |
Author: Eric V. Smith (eric.smith) * |
Date: 2010-10-28 11:46 |
| 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. |
|
|
| msg119900 - (view) |
Author: Eric V. Smith (eric.smith) * |
Date: 2010-10-29 13:15 |
| I think Benjamin fixed this in r85904. I'm going to verify and add some tests. |
|
|
| msg138852 - (view) |
Author: Petri Lehtinen (petri.lehtinen) * |
Date: 2011-06-23 06:28 |
| 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. |
|
|
| msg138853 - (view) |
Author: Stefan Krah (skrah) * |
Date: 2011-06-23 06:55 |
| I remember that I could reproduce it at the time. The issue was indeed fixed in r85904. |
|
|
| msg138854 - (view) |
Author: Petri Lehtinen (petri.lehtinen) * |
Date: 2011-06-23 06:58 |
| By checking out the parent of r85904 I now can reproduce this. |
|
|
| msg138859 - (view) |
Author: Petri Lehtinen (petri.lehtinen) * |
Date: 2011-06-23 07:36 |
| Attached a test case. The patch is against the current default tip. |
|
|
| msg138884 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2011-06-24 01:10 |
| 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... |
|
|
| msg138955 - (view) |
Author: Francis MB (francismb) * |
Date: 2011-06-24 15:49 |
| 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 |
|
|
| msg138957 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2011-06-24 16:07 |
| 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. |
|
|
| msg138960 - (view) |
Author: Francis MB (francismb) * |
Date: 2011-06-24 16:21 |
| Just attaching the review patch |
|
|
| msg138961 - (view) |
Author: Francis MB (francismb) * |
Date: 2011-06-24 16:30 |
| 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 |
|
|
| msg138967 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2011-06-24 17:28 |
| New changeset 2a4764376c51 by R David Murray in branch '3.2': #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 #10206: add test for previously fixed bug. http://hg.python.org/cpython/rev/5ec95f46bac5 |
|
|
| msg138968 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2011-06-24 17:30 |
| 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. |
|
|