bpo-31285: fix an assertion failure and a SystemError in warnings.warn_explicit #3219

orenmn · 2017-08-27T18:33:36Z

in _warnings.c: add checks whether the return value of the received loader's get_source() is invalid.
in test_warnings/__init__.py: add tests to verify the assertion failure and the SystemError are no more.

https://bugs.python.org/issue31285

…ource)

serhiy-storchaka · 2017-08-30T06:09:26Z

Python/_warnings.c


        /* Split the source into lines. */
-        source_list = PyObject_CallMethodObjArgs(source,
+        source_list = PyObject_CallMethodObjArgs((PyObject *)&PyUnicode_Type,


Use just PyUnicode_Splitlines().

serhiy-storchaka · 2017-08-30T06:24:11Z

Lib/test/test_warnings/__init__.py

+                            return splitlines_ret_val
+                    return BadSource()
+            return BadLoader()
+        self.assertRaises(IndexError, self.module.warn_explicit,


Since this test already takes multiple lines, I think it would look clearer if use a context manager form:

with self.assertRaises(IndexError): self.module.warn_explicit(...)

But why IndexError is expected? Wouldn't be better to test the normal case that doesn't raise an error? You can use test.support.captured_stderr() for capturing the warning output.

serhiy-storchaka · 2017-08-30T06:38:22Z

Lib/test/test_warnings/__init__.py

        self.assertNotIn(b'Warning!', stderr)
        self.assertNotIn(b'Error', stderr)

+    def test_warn_explicit(self):


This name is too general, but the test isn't a general test of warn_explicit(). Use something more specific like test_issue31285.

orenmn · 2017-08-30T12:38:49Z

Lib/test/test_warnings/__init__.py

+                'foo', UserWarning, 'bar', 1,
+                module_globals={'__loader__': _get_bad_loader(42),
+                                '__name__': 'foobar'})
+        self.assertIn('bar:1: UserWarning: foo', stderr.getvalue())


I am not sure whether this and the other call to assertIn() are necessary, as we want to make sure a SystemError and an assertion failure don't happen...

serhiy-storchaka · 2017-09-24T09:12:11Z

Lib/test/test_warnings/__init__.py

+        # warn_explicit() should neither raise a SystemError nor cause an
+        # assertion failure, in case the return value of get_source() has a
+        # bad splitlines() method.
+        def _get_bad_loader(splitlines_ret_val):


Starting underscore is not needed.

serhiy-storchaka · 2017-09-24T09:16:18Z

Lib/test/test_warnings/__init__.py

+            del self.module._showwarnmsg
+            with support.captured_stderr() as stderr:
+                self.module.warn_explicit(
+                    'foo', ArithmeticError, 'bar', 1,


Is it valid to use ArithmeticError which is not a warning category?

It is valid (IIUC, only warning.warns() requires the category to be a subclass of Warning). I used ArithmeticError because it caused get_filter() to return 'default'. But now it seems quite obscure to me, so I would change it.

Also, my test fails with -Werror, so I would fix that too.

miss-islington · 2017-09-24T18:27:14Z

Thanks @orenmn for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 2.7, 3.6.
🐍🍒⛏🤖

miss-islington · 2017-09-24T18:27:18Z

Sorry, @orenmn and @serhiy-storchaka, I could not cleanly backport this to 2.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 91fb0afe181986b48abfc6092dcca912b39de51d 2.7

miss-islington · 2017-09-26T23:29:05Z

Thanks @orenmn for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 2.7, 3.6.
🐍🍒⛏🤖

miss-islington · 2017-09-26T23:29:06Z

Sorry @orenmn and @serhiy-storchaka, I had trouble checking out the 3.6 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 91fb0afe181986b48abfc6092dcca912b39de51d 3.6

miss-islington · 2017-09-26T23:29:07Z

Sorry, @orenmn and @serhiy-storchaka, I could not cleanly backport this to 2.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 91fb0afe181986b48abfc6092dcca912b39de51d 2.7

…gs.warn_explicit. (pythonGH-3219) (cherry picked from commit 91fb0af)

bedevere-bot · 2017-09-26T23:30:15Z

GH-3775 is a backport of this pull request to the 3.6 branch.

…gs.warn_explicit. (GH-3219) (#3775) (cherry picked from commit 91fb0af)

bedevere-bot · 2017-09-28T15:44:00Z

GH-3805 is a backport of this pull request to the 2.7 branch.

…pythonGH-3803) (forgot to remove it in pythonGH-3219) (cherry picked from commit 8b4ff53)

(forgot to remove it in #3219)

…GH-3803) (#3829) (forgot to remove it in GH-3219) (cherry picked from commit 8b4ff53)

…) in case __loader__.get_source() has a bad splitlines() method. (pythonGH-3219)

…) in case __loader__.get_source() has a bad splitlines() method. (GH-3219) (#3823) (cherry picked from commit 91fb0af)

orenmn added 2 commits August 27, 2017 20:18

init commit

ff52f6b

add a NEWS.d item

252647d

the-knights-who-say-ni added the CLA signed label Aug 27, 2017

bedevere-bot added the awaiting review label Aug 27, 2017

orenmn added 2 commits August 28, 2017 13:06

replace the C equivalent of source.splitlines() with str.splitlines(s…

d4dbe6d

…ource)

fix NEWS.d item

9eb55b5

serhiy-storchaka reviewed Aug 30, 2017

View reviewed changes

improve test method and its name, and change to PyUnicode_Splitlines()

5674174

orenmn commented Aug 30, 2017

View reviewed changes

bedevere-bot added awaiting core review and removed awaiting review labels Aug 30, 2017

resolve conflict in test_warnings/__init__.py

b8fe794

serhiy-storchaka reviewed Sep 24, 2017

View reviewed changes

orenmn added 3 commits September 24, 2017 14:52

merge master

64f31fc

remove redundant underscore

6fe73a4

fix the test, and make it clearer

71da7df

serhiy-storchaka added needs backport to 2.7 type-bug An unexpected behavior, bug, or error labels Sep 24, 2017

serhiy-storchaka approved these changes Sep 24, 2017

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting core review labels Sep 24, 2017

serhiy-storchaka merged commit 91fb0af into python:master Sep 24, 2017

serhiy-storchaka added needs backport to 3.6 and removed needs backport to 3.6 labels Sep 26, 2017

serhiy-storchaka pushed a commit to serhiy-storchaka/cpython that referenced this pull request Sep 26, 2017

[3.6] bpo-31285: Fix an assertion failure and a SystemError in warnin…

6430003

…gs.warn_explicit. (pythonGH-3219) (cherry picked from commit 91fb0af)

bedevere-bot removed the needs backport to 3.6 label Sep 26, 2017

serhiy-storchaka added a commit that referenced this pull request Sep 27, 2017

[3.6] bpo-31285: Fix an assertion failure and a SystemError in warnin…

90fe25a

…gs.warn_explicit. (GH-3219) (#3775) (cherry picked from commit 91fb0af)

orenmn mentioned this pull request Sep 28, 2017

bpo-31285: Remove splitlines identifier from Python/_warnings.c (forgot to remove it in #3219) #3803

Merged

bedevere-bot removed the needs backport to 2.7 label Sep 28, 2017

orenmn mentioned this pull request Sep 29, 2017

[2.7] bpo-31285: fix an assertion failure and a SystemError in warnings.warn_explicit (GH-3219) #3805

Closed

miss-islington mentioned this pull request Sep 29, 2017

[3.6] bpo-31285: Remove splitlines identifier from Python/_warnings.c (GH-3803) #3829

Merged

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 29, 2017

[3.6] bpo-31285: Remove splitlines identifier from Python/_warnings.c (…

3c3e62c

…pythonGH-3803) (forgot to remove it in pythonGH-3219) (cherry picked from commit 8b4ff53)

serhiy-storchaka pushed a commit that referenced this pull request Sep 29, 2017

bpo-31285: Remove splitlines identifier from Python/_warnings.c (#3803)

8b4ff53

(forgot to remove it in #3219)

serhiy-storchaka pushed a commit that referenced this pull request Sep 29, 2017

[3.6] bpo-31285: Remove splitlines identifier from Python/_warnings.c (…

66c2b9f

…GH-3803) (#3829) (forgot to remove it in GH-3219) (cherry picked from commit 8b4ff53)

orenmn added a commit to orenmn/cpython that referenced this pull request Sep 30, 2017

[2.7] bpo-31285: Don't raise a SystemError in warnings.warn_explicit(…

c15236b

…) in case __loader__.get_source() has a bad splitlines() method. (pythonGH-3219)

serhiy-storchaka pushed a commit that referenced this pull request Sep 30, 2017

[2.7] bpo-31285: Don't raise a SystemError in warnings.warn_explicit(…

40d736b

…) in case __loader__.get_source() has a bad splitlines() method. (GH-3219) (#3823) (cherry picked from commit 91fb0af)

Mariatta removed the awaiting merge label Oct 8, 2017

Uh oh!

bpo-31285: fix an assertion failure and a SystemError in warnings.warn_explicit #3219

bpo-31285: fix an assertion failure and a SystemError in warnings.warn_explicit #3219

Uh oh!

Conversation

orenmn commented Aug 27, 2017 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serhiy-storchaka Aug 30, 2017

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Aug 30, 2017

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Aug 30, 2017

Choose a reason for hiding this comment

Uh oh!

orenmn Aug 30, 2017

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Sep 24, 2017

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Sep 24, 2017

Choose a reason for hiding this comment

Uh oh!

orenmn Sep 24, 2017

Choose a reason for hiding this comment

Uh oh!

miss-islington commented Sep 24, 2017

Uh oh!

miss-islington commented Sep 24, 2017

Uh oh!

miss-islington commented Sep 26, 2017

Uh oh!

miss-islington commented Sep 26, 2017

Uh oh!

miss-islington commented Sep 26, 2017

Uh oh!

bedevere-bot commented Sep 26, 2017

Uh oh!

bedevere-bot commented Sep 28, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

orenmn commented Aug 27, 2017 •

edited by bedevere-bot

Loading