diff --git a/tests/hacking/README.rst b/tests/hacking/README.rst index d6d21a141f..c3bc311d74 100644 --- a/tests/hacking/README.rst +++ b/tests/hacking/README.rst @@ -21,6 +21,7 @@ Rally Specific Commandments * [N322] - Ensure that ``assertEqual(A, None)`` and ``assertEqual(None, A)`` are not used * [N323] - Ensure that ``assertTrue/assertFalse(A in/not in B)`` are not used with collection contents * [N324] - Ensure that ``assertEqual(A in/not in B, True/False)`` and ``assertEqual(True/False, A in/not in B)`` are not used with collection contents + * [N325] - Ensure that ``assertNotEqual(A, None)`` and ``assertNotEqual(None, A)`` are not used * [N340] - Ensure that we are importing always ``from rally import objects`` * [N341] - Ensure that we are importing oslo_xyz packages instead of deprecated oslo.xyz ones * [N350] - Ensure that single quotes are not used diff --git a/tests/hacking/checks.py b/tests/hacking/checks.py index 3d5fc9a00e..8ea2fe3245 100644 --- a/tests/hacking/checks.py +++ b/tests/hacking/checks.py @@ -36,6 +36,9 @@ re_assert_equal_type = re.compile( r"(\w|\.|\'|\"|\[|\])+\)") re_assert_equal_end_with_none = re.compile(r"assertEqual\(.*?,\s+None\)$") re_assert_equal_start_with_none = re.compile(r"assertEqual\(None,") +re_assert_not_equal_end_with_none = re.compile( + r"assertNotEqual\(.*?,\s+None\)$") +re_assert_not_equal_start_with_none = re.compile(r"assertNotEqual\(None,") re_assert_true_false_with_in_or_not_in = re.compile( r"assert(True|False)\(" r"(\w|[][.'\"])+( not)? in (\w|[][.'\",])+(, .*)?\)") @@ -278,6 +281,20 @@ def assert_equal_in(logical_line, physical_line, filename): "collection contents.") +@skip_ignored_lines +def assert_not_equal_none(logical_line, physical_line, filename): + """Check for assertNotEqual(A, None) or assertEqual(None, A) sentences + + N325 + """ + res = (re_assert_not_equal_start_with_none.search(logical_line) or + re_assert_not_equal_end_with_none.search(logical_line)) + if res: + yield (0, "N325 assertNotEqual(A, None) or assertNotEqual(None, A) " + "sentences not allowed, you should use assertIsNotNone(A) " + "instead.") + + @skip_ignored_lines def check_no_direct_rally_objects_import(logical_line, physical_line, filename): diff --git a/tests/unit/plugins/common/scenarios/dummy/test_dummy.py b/tests/unit/plugins/common/scenarios/dummy/test_dummy.py index 1b6dd8fcd4..ee255fc441 100644 --- a/tests/unit/plugins/common/scenarios/dummy/test_dummy.py +++ b/tests/unit/plugins/common/scenarios/dummy/test_dummy.py @@ -161,7 +161,7 @@ class DummyTestCase(test.TestCase): self.assertEqual(result["errors"], "") # Since the data is generated in random, # checking for not None - self.assertNotEqual(result["data"], None) + self.assertIsNotNone(result["data"]) def test_dummy_random_fail_in_atomic(self): scenario = dummy.Dummy(test.get_test_context()) diff --git a/tests/unit/test_hacking.py b/tests/unit/test_hacking.py index 821cda6905..a22fbd4929 100644 --- a/tests/unit/test_hacking.py +++ b/tests/unit/test_hacking.py @@ -170,6 +170,17 @@ class HackingTestCase(test.TestCase): self.assertEqual( len(list(checks.assert_equal_none(line, line, "f"))), result) + @ddt.data( + {"line": "self.assertNotEqual(A, None)", "result": 1}, + {"line": "self.assertNotEqual(None, A)", "result": 1}, + {"line": "self.assertIsNotNone()", "result": 0} + ) + @ddt.unpack + def test_assert_not_equal_none(self, line, result): + + self.assertEqual( + len(list(checks.assert_not_equal_none(line, line, "f"))), result) + def test_assert_true_or_false_with_in_or_not_in(self): good_lines = [ "self.assertTrue(any(A > 5 for A in B))",