From 69e2a3cec12c594127460c0de3980e6fe42fec9f Mon Sep 17 00:00:00 2001 From: Ngo Quoc Cuong Date: Wed, 12 Jul 2017 04:41:44 -0400 Subject: [PATCH] Fix N536 - Use assertIsNone regex self.assertEqual((None, None), A) is valid since (None, None) is not None. However assert_equal_none recognizes it as invalid. Solution: Add '$' (Match the end of the text) with/without comment ( |\t)*#.*)? to the regular expressions which intent to match None at the end of string. Change-Id: Ic5e5c16c9225a80b2765a1d3cde9a5b9e83a06fb --- neutron_lib/hacking/checks.py | 4 ++-- neutron_lib/tests/unit/hacking/test_checks.py | 16 ++++++++++++++++ ...date-hacking-check-n536-2f63898bea693125.yaml | 5 +++++ 3 files changed, 23 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/update-hacking-check-n536-2f63898bea693125.yaml diff --git a/neutron_lib/hacking/checks.py b/neutron_lib/hacking/checks.py index affd51d32..e5af5cef0 100644 --- a/neutron_lib/hacking/checks.py +++ b/neutron_lib/hacking/checks.py @@ -36,9 +36,9 @@ namespace_imports_from_root = re.compile(r"from[\s]+([\w]+)[\s]+import[\s]+") contextlib_nested = re.compile(r"^\s*with (contextlib\.)?nested\(") assert_equal_none_re = re.compile( - r"assertEqual\(.*?,\s+None\)|assertEqual\(None,") + r"assertEqual\(.*?,\s+None\)(( |\t)*#.*)?$|assertEqual\(None,") assert_is_none_re = re.compile( - r"assertIs(Not)?\(.*,\s+None\)|assertIs(Not)?\(None,") + r"assertIs(Not)?\(.*,\s+None\)(( |\t)*#.*)?$|assertIs(Not)?\(None,") def use_jsonutils(logical_line, filename): diff --git a/neutron_lib/tests/unit/hacking/test_checks.py b/neutron_lib/tests/unit/hacking/test_checks.py index 53acfba9f..31fe36e16 100644 --- a/neutron_lib/tests/unit/hacking/test_checks.py +++ b/neutron_lib/tests/unit/hacking/test_checks.py @@ -188,6 +188,14 @@ class HackingTestCase(base.BaseTestCase): "self.assertEqual(None, A)"))), 1) self.assertEqual(len(list(checks.assert_equal_none( "self.assertEqual(None, A) # Comment"))), 1) + self.assertEqual(len(list(checks.assert_equal_none( + "self.assertEqual((None, None), A)"))), 0) + self.assertEqual(len(list(checks.assert_equal_none( + "self.assertEqual((None, None), A) # Comment"))), 0) + self.assertEqual(len(list(checks.assert_equal_none( + "self.assertEqual(A, (None, None))"))), 0) + self.assertEqual(len(list(checks.assert_equal_none( + "self.assertEqual(A, (None, None)) # Comment"))), 0) self.assertEqual(len(list(checks.assert_equal_none( "assertIsNot(A, None)"))), 1) self.assertEqual(len(list(checks.assert_equal_none( @@ -196,6 +204,14 @@ class HackingTestCase(base.BaseTestCase): "assertIsNot(None, A)"))), 1) self.assertEqual(len(list(checks.assert_equal_none( "assertIsNot(None, A) # Comment"))), 1) + self.assertEqual(len(list(checks.assert_equal_none( + "assertIsNot((None, None), A)"))), 0) + self.assertEqual(len(list(checks.assert_equal_none( + "assertIsNot((None, None), A) # Comment"))), 0) + self.assertEqual(len(list(checks.assert_equal_none( + "assertIsNot(A, (None, None))"))), 0) + self.assertEqual(len(list(checks.assert_equal_none( + "assertIsNot(A, (None, None)) # Comment"))), 0) self.assertEqual( len(list(checks.assert_equal_none("self.assertIsNone(A)"))), 0) self.assertEqual( diff --git a/releasenotes/notes/update-hacking-check-n536-2f63898bea693125.yaml b/releasenotes/notes/update-hacking-check-n536-2f63898bea693125.yaml new file mode 100644 index 000000000..f558eed03 --- /dev/null +++ b/releasenotes/notes/update-hacking-check-n536-2f63898bea693125.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - Update hacking check ``N536``. Current implementation recognizes + ``self.assertEqual((None, None), A)`` as invalid incorrectly while + ``(None, None)`` is not ``None``.