diff --git a/HACKING.rst b/HACKING.rst index 486020db1d1c..b5f20ffd19e3 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -74,6 +74,7 @@ Nova Specific Commandments not "from nova.privsep import path". This ensures callers know that the method they're calling is using priviledge escalation. - [N363] Disallow ``(not_a_tuple)`` because you meant ``(a_tuple_of_one,)``. +- [N364] Check non-existent mock assertion methods and attributes. Creating Unit Tests ------------------- diff --git a/nova/hacking/checks.py b/nova/hacking/checks.py index 09a689dc5aa2..3d5c5cf47dc7 100644 --- a/nova/hacking/checks.py +++ b/nova/hacking/checks.py @@ -115,6 +115,17 @@ privsep_import_re = re.compile( # Close paren disguised_as_tuple_re = re.compile(r''' in \((['"]?)[a-zA-Z0-9_.]+\1\)''') +# NOTE(takashin): The patterns of nox-existent mock assertion methods and +# attributes do not cover all cases. If you find a new pattern, +# add the pattern in the following regex patterns. +mock_assert_method_re = re.compile( + r"\.((called_once(_with)*|has_calls)|" + r"mock_assert_(called(_(once|with|once_with))?" + r"|any_call|has_calls|not_called)|" + r"(asser|asset|asssert|assset)_(called(_(once|with|once_with))?" + r"|any_call|has_calls|not_called))\(") +mock_attribute_re = re.compile(r"[\.\(](retrun_value)[,=\s]") + class BaseASTChecker(ast.NodeVisitor): """Provides a simple framework for writing AST-based checks. @@ -898,6 +909,39 @@ def did_you_mean_tuple(logical_line): "certainly meant ``in (a_tuple_of_one,)``.") +def nonexistent_assertion_methods_and_attributes(logical_line, filename): + """Check non-existent mock assertion methods and attributes. + + The following assertion methods do not exist. + + - called_once() + - called_once_with() + - has_calls() + - mock_assert_*() + + The following typos were found in the past cases. + + - asser_* + - asset_* + - assset_* + - asssert_* + - retrun_value + + N364 + """ + msg = ("N364: Non existent mock assertion method or attribute (%s) is " + "used. Check a typo or whether the assertion method should begin " + "with 'assert_'.") + if 'nova/tests/' in filename: + match = mock_assert_method_re.search(logical_line) + if match: + yield (0, msg % match.group(1)) + + match = mock_attribute_re.search(logical_line) + if match: + yield (0, msg % match.group(1)) + + def factory(register): register(import_no_db_in_virt) register(no_db_session_in_public_api) @@ -944,3 +988,4 @@ def factory(register): register(assert_regexpmatches) register(privsep_imports_not_aliased) register(did_you_mean_tuple) + register(nonexistent_assertion_methods_and_attributes) diff --git a/nova/tests/unit/test_hacking.py b/nova/tests/unit/test_hacking.py index e1455d7105ba..ab570f4d310c 100644 --- a/nova/tests/unit/test_hacking.py +++ b/nova/tests/unit/test_hacking.py @@ -913,3 +913,81 @@ class HackingTestCase(test.NoDBTestCase): or foo in (method_call_should_this_work()): """ self._assert_has_no_errors(code, checks.did_you_mean_tuple) + + def test_nonexistent_assertion_methods_and_attributes(self): + code = """ + mock_sample.called_once() + mock_sample.called_once_with(a, "TEST") + mock_sample.has_calls([mock.call(x), mock.call(y)]) + mock_sample.mock_assert_called() + mock_sample.mock_assert_called_once() + mock_sample.mock_assert_called_with(a, "xxxx") + mock_sample.mock_assert_called_once_with(a, b) + mock_sample.mock_assert_any_call(1, 2, instance=instance) + sample.mock_assert_has_calls([mock.call(x), mock.call(y)]) + mock_sample.mock_assert_not_called() + mock_sample.asser_called() + mock_sample.asser_called_once() + mock_sample.asser_called_with(a, "xxxx") + mock_sample.asser_called_once_with(a, b) + mock_sample.asser_any_call(1, 2, instance=instance) + mock_sample.asser_has_calls([mock.call(x), mock.call(y)]) + mock_sample.asser_not_called() + mock_sample.asset_called() + mock_sample.asset_called_once() + mock_sample.asset_called_with(a, "xxxx") + mock_sample.asset_called_once_with(a, b) + mock_sample.asset_any_call(1, 2, instance=instance) + mock_sample.asset_has_calls([mock.call(x), mock.call(y)]) + mock_sample.asset_not_called() + mock_sample.asssert_called() + mock_sample.asssert_called_once() + mock_sample.asssert_called_with(a, "xxxx") + mock_sample.asssert_called_once_with(a, b) + mock_sample.asssert_any_call(1, 2, instance=instance) + mock_sample.asssert_has_calls([mock.call(x), mock.call(y)]) + mock_sample.asssert_not_called() + mock_sample.assset_called() + mock_sample.assset_called_once() + mock_sample.assset_called_with(a, "xxxx") + mock_sample.assset_called_once_with(a, b) + mock_sample.assset_any_call(1, 2, instance=instance) + mock_sample.assset_has_calls([mock.call(x), mock.call(y)]) + mock_sample.assset_not_called() + mock_sample.retrun_value = 100 + sample.call_method(mock_sample.retrun_value, 100) + mock.Mock(retrun_value=100) + """ + errors = [(x + 1, 0, 'N364') for x in range(41)] + # Check errors in 'nova/tests' directory. + self._assert_has_errors( + code, checks.nonexistent_assertion_methods_and_attributes, + expected_errors=errors, filename="nova/tests/unit/test_context.py") + # Check no errors in other than 'nova/tests' directory. + self._assert_has_no_errors( + code, checks.nonexistent_assertion_methods_and_attributes, + filename="nova/compute/api.py") + + code = """ + mock_sample.assert_called() + mock_sample.assert_called_once() + mock_sample.assert_called_with(a, "xxxx") + mock_sample.assert_called_once_with(a, b) + mock_sample.assert_any_call(1, 2, instance=instance) + mock_sample.assert_has_calls([mock.call(x), mock.call(y)]) + mock_sample.assert_not_called() + mock_sample.return_value = 100 + sample.call_method(mock_sample.return_value, 100) + mock.Mock(return_value=100) + + sample.has_other_calls([1, 2, 3, 4]) + sample.get_called_once("TEST") + sample.check_called_once_with("TEST") + mock_assert_method.assert_called() + test.asset_has_all(test) + test_retrun_value = 99 + test.retrun_values = 100 + """ + self._assert_has_no_errors( + code, checks.nonexistent_assertion_methods_and_attributes, + filename="nova/tests/unit/test_context.py")