From 9400cee06f7f8bfab48582be6bd8735c0749e571 Mon Sep 17 00:00:00 2001 From: Sean McGinnis Date: Mon, 23 May 2016 09:29:43 -0500 Subject: [PATCH] Add hacking check to prevent assert_called_once We've had a few patches now over the last few releases to fix invalid mock assertions for assert_called_once. Rather than keep letting these creep back in to the code, this adds a hacking check to watch for this specific call and block it. Change-Id: I3cf4df523ddba49f57dd1d4d23e95e6d62d03c9e --- HACKING.rst | 1 + cinder/hacking/checks.py | 10 ++++++++++ cinder/tests/unit/test_hacking.py | 6 ++++++ 3 files changed, 17 insertions(+) diff --git a/HACKING.rst b/HACKING.rst index 3e46834bded..0b448988bf4 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -30,6 +30,7 @@ Cinder Specific Commandments - [C311] Check for proper naming and usage in option registration. - [C312] Check that assertIsNone(value) is used and not assertEqual(None, value). - [C313] Check that assertTrue(value) is used and not assertEqual(True, value). +- [C314] Check for use of invalid mock.assert_called_once(). General ------- diff --git a/cinder/hacking/checks.py b/cinder/hacking/checks.py index 176d4e2fd4c..35e242fbd20 100644 --- a/cinder/hacking/checks.py +++ b/cinder/hacking/checks.py @@ -504,6 +504,15 @@ def validate_assertTrue(logical_line): yield(0, msg) +def no_assert_called_once(logical_line, filename): + if "cinder/tests" not in filename or "test_hacking" in filename: + return + msg = ("C314: assert_called_once is not a valid mock assertion, " + "use assert_equal(1, mocked.call_count) instead.") + if 'assert_called_once(' in logical_line: + yield (0, msg) + + def factory(register): register(no_vi_headers) register(no_translate_debug_logs) @@ -526,3 +535,4 @@ def factory(register): register(no_test_log) register(validate_assertIsNone) register(validate_assertTrue) + register(no_assert_called_once) diff --git a/cinder/tests/unit/test_hacking.py b/cinder/tests/unit/test_hacking.py index b4393f1c2c7..68c45facb75 100644 --- a/cinder/tests/unit/test_hacking.py +++ b/cinder/tests/unit/test_hacking.py @@ -448,3 +448,9 @@ class HackingTestCase(test.TestCase): def test_no_test_log(self, first, second, third, fourth): self.assertEqual(first, len(list(checks.no_test_log( "%s('arg')" % second, third, fourth)))) + + def test_no_assert_called_once(self): + self.assertEqual(1, len(list(checks.no_assert_called_once( + "masker.assert_called_once(", "cinder/tests/unit/fake.py")))) + self.assertEqual(0, len(list(checks.no_assert_called_once( + "masker.assert_called_once_with(", "cinder/tests/unit/fake.py"))))