diff --git a/bot/reviewbot/tools/pyflakes.py b/bot/reviewbot/tools/pyflakes.py index aad25b4ac92d1662f991a47e9cc6b927b27c129c..326942410830e793091c84dfb264dd51d03e9b16 100644 --- a/bot/reviewbot/tools/pyflakes.py +++ b/bot/reviewbot/tools/pyflakes.py @@ -118,8 +118,14 @@ class PyflakesTool(BaseTool): # This should be a syntax error. try: linenum = int(linenum) - column = int(m.group('column')) - except ValueError: + + column_str = m.group('column') + + if column_str: + column = int(column_str) + else: + column = None + except (TypeError, ValueError): # This isn't actually an info line. This is # unexpected, but skip it. continue diff --git a/bot/reviewbot/tools/tests/test_cargotool.py b/bot/reviewbot/tools/tests/test_cargotool.py index d2c58a976c7071294d732dd86317e898d5770c2c..946356906578c47cb8cf42e7e8cfa3a23bbf7c41 100644 --- a/bot/reviewbot/tools/tests/test_cargotool.py +++ b/bot/reviewbot/tools/tests/test_cargotool.py @@ -337,10 +337,10 @@ class CargoToolTests(BaseToolTestCase, metaclass=ToolTestCaseMetaclass): "failures:\n", "\n", "---- tests::test1 stdout ----\n", - "thread 'tests::test1' panicked at 'assertion failed:" - " `(left == right)`\n", - " left: `1`,\n", - " right: `2`', tests/test.rs:4:9\n", + "thread 'tests::test1' panicked at tests/test.rs:4:9:\n", + "assertion `left == right` failed\n", + " left: 1\n", + " right: 2\n", "note: run with `RUST_BACKTRACE=1` environment variable to display" " a backtrace\n", "\n", @@ -368,27 +368,49 @@ class CargoToolTests(BaseToolTestCase, metaclass=ToolTestCaseMetaclass): }) self.assertEqual(review.comments, []) - self.assertEqual(review.general_comments, [ - { - 'issue_opened': True, - 'rich_text': True, - 'text': ( - "1 test failed:\n" - "\n" - "```" - "---- tests::test1 stdout ----\n" - "thread 'tests::test1' panicked at 'assertion failed: " - "`(left == right)`\n" - " left: `1`,\n" - " right: `2`', tests/test.rs:4:9\n" - "\n" - "\n" - "failures:\n" - " tests::test1" - "```" - ) - }, - ]) + + general_comments = review.general_comments + self.assertEqual(len(general_comments), 1) + + comment = general_comments[0] + self.assertIn( + comment.pop('text'), + [ + # Added August 11, 2024 + "1 test failed:\n" + "\n" + "```" + "---- tests::test1 stdout ----\n" + "thread 'tests::test1' panicked at tests/test.rs:4:9:\n" + "assertion `left == right` failed\n" + " left: 1\n" + " right: 2\n" + "\n" + "\n" + "failures:\n" + " tests::test1" + "```", + + # Added August 9, 2021 + "1 test failed:\n" + "\n" + "```" + "---- tests::test1 stdout ----\n" + "thread 'tests::test1' panicked at 'assertion failed: " + "`(left == right)`\n" + " left: `1`,\n" + " right: `2`', tests/test.rs:4:9\n" + "\n" + "\n" + "failures:\n" + " tests::test1" + "```", + ]) + + self.assertEqual(comment, { + 'issue_opened': True, + 'rich_text': True, + }) self.assertSpyCalledWith( execute, @@ -419,18 +441,18 @@ class CargoToolTests(BaseToolTestCase, metaclass=ToolTestCaseMetaclass): "failures:\n", "\n", "---- tests::test1 stdout ----\n", - "thread 'tests::test1' panicked at 'assertion failed:" - " `(left == right)`\n", - " left: `1`,\n", - " right: `2`', tests/test.rs:4:9\n", + "thread 'tests::test1' panicked at tests/test.rs:4:9:\n", + "assertion `left == right` failed\n", + " left: 1\n", + " right: 2\n", "note: run with `RUST_BACKTRACE=1` environment variable to display" " a backtrace\n", "\n", "---- tests::test2 stdout ----\n", - "thread 'tests::test2' panicked at 'assertion failed:" - " `(left == right)`\n", - " left: `3`,\n", - " right: `4`', tests/test.rs:9:9\n", + "thread 'tests::test2' panicked at tests/test.rs:9:9:\n", + "assertion `left == right` failed\n", + " left: 3\n", + " right: 4\n", "note: run with `RUST_BACKTRACE=1` environment variable to display" " a backtrace\n", "\n", @@ -459,34 +481,63 @@ class CargoToolTests(BaseToolTestCase, metaclass=ToolTestCaseMetaclass): }) self.assertEqual(review.comments, []) - self.assertEqual(review.general_comments, [ - { - 'issue_opened': True, - 'rich_text': True, - 'text': ( - "2 tests failed:\n" - "\n" - "```" - "---- tests::test1 stdout ----\n" - "thread 'tests::test1' panicked at 'assertion failed: " - "`(left == right)`\n" - " left: `1`,\n" - " right: `2`', tests/test.rs:4:9\n" - "\n" - "---- tests::test2 stdout ----\n" - "thread 'tests::test2' panicked at 'assertion failed: " - "`(left == right)`\n" - " left: `3`,\n" - " right: `4`', tests/test.rs:9:9\n" - "\n" - "\n" - "failures:\n" - " tests::test1\n" - " tests::test2" - "```" - ) - }, - ]) + + general_comments = review.general_comments + self.assertEqual(len(general_comments), 1) + + comment = general_comments[0] + self.assertIn( + comment.pop('text'), + [ + # Added August 11, 2024 + "2 tests failed:\n" + "\n" + "```" + "---- tests::test1 stdout ----\n" + "thread 'tests::test1' panicked at tests/test.rs:4:9:\n" + "assertion `left == right` failed\n" + " left: 1\n" + " right: 2\n" + "\n" + "---- tests::test2 stdout ----\n" + "thread 'tests::test2' panicked at tests/test.rs:9:9:\n" + "assertion `left == right` failed\n" + " left: 3\n" + " right: 4\n" + "\n" + "\n" + "failures:\n" + " tests::test1\n" + " tests::test2" + "```", + + # Added August 9, 2021 + "2 tests failed:\n" + "\n" + "```" + "---- tests::test1 stdout ----\n" + "thread 'tests::test1' panicked at 'assertion failed: " + "`(left == right)`\n" + " left: `1`,\n" + " right: `2`', tests/test.rs:4:9\n" + "\n" + "---- tests::test2 stdout ----\n" + "thread 'tests::test2' panicked at 'assertion failed: " + "`(left == right)`\n" + " left: `3`,\n" + " right: `4`', tests/test.rs:9:9\n" + "\n" + "\n" + "failures:\n" + " tests::test1\n" + " tests::test2" + "```", + ]) + + self.assertEqual(comment, { + 'issue_opened': True, + 'rich_text': True, + }) self.assertSpyCalledWith( execute, diff --git a/bot/reviewbot/tools/tests/test_pyflakes.py b/bot/reviewbot/tools/tests/test_pyflakes.py index dfb3a24660a49f72d52e9df28a8df18192eee532..c821b47c26464bc352a8706d66a5251834126a0b 100644 --- a/bot/reviewbot/tools/tests/test_pyflakes.py +++ b/bot/reviewbot/tools/tests/test_pyflakes.py @@ -108,7 +108,7 @@ class PyflakesToolTests(BaseToolTestCase, metaclass=ToolTestCaseMetaclass): @integration_test() @simulation_test(stderr=[ - 'test.py: problem decoding source', + 'test.py:1: source code string cannot contain null bytes' ]) def test_execute_with_unexpected_error(self): """Testing PyflakesTool.execute with unexpected errors""" @@ -118,15 +118,31 @@ class PyflakesToolTests(BaseToolTestCase, metaclass=ToolTestCaseMetaclass): b'\00\11\22' )) - self.assertEqual(review.comments, []) - self.assertEqual(review.general_comments, [ - { - 'text': ('pyflakes could not process test.py: ' - 'problem decoding source'), - 'issue_opened': True, - 'rich_text': False, - }, - ]) + # PyFlakes on Python 3.11.4+ will have a line number. On earlier + # versions, it won't. Check for both. + if len(review.comments) > 0: + # This is PyFlakes on Python 3.11.4+. + self.assertEqual(review.comments, [ + { + 'filediff_id': 42, + 'first_line': 1, + 'issue_opened': True, + 'num_lines': 1, + 'rich_text': False, + 'text': 'source code string cannot contain null bytes', + } + ]) + self.assertEqual(review.general_comments, []) + else: + self.assertEqual(review.comments, []) + self.assertEqual(review.general_comments, [ + { + 'text': ('pyflakes could not process test.py: ' + 'problem decoding source'), + 'issue_opened': True, + 'rich_text': False, + }, + ]) @integration_test() @simulation_test()