From: Darragh O'Reilly Date: Fri, 27 Feb 2015 08:23:24 +0000 (+0000) Subject: Fix dhcp _test_sync_state_helper asserting calls wrong X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=b85cfa96118d73ad87b150e488f295cbf9a2c140;p=openstack-build%2Fneutron-build.git Fix dhcp _test_sync_state_helper asserting calls wrong It was using a non-existing method that did nothing and that masked other problems with the tests that used it. Changed to use assert_has_calls() and fixed the problems that fell out. Change-Id: I4a64c3365f9958b14c2384932a31da2ce191e2e2 Closes-Bug: 1426265 --- diff --git a/neutron/hacking/checks.py b/neutron/hacking/checks.py index 392f09d15..0c925cc39 100644 --- a/neutron/hacking/checks.py +++ b/neutron/hacking/checks.py @@ -107,14 +107,22 @@ def check_assert_called_once_with(logical_line, filename): # Try to detect unintended calls of nonexistent mock methods like: # assert_called_once # assertCalledOnceWith + # assert_has_called if 'neutron/tests/' in filename: if '.assert_called_once_with(' in logical_line: return - if '.assertcalledonce' in logical_line.lower().replace('_', ''): + uncased_line = logical_line.lower().replace('_', '') + + if '.assertcalledonce' in uncased_line: msg = ("N322: Possible use of no-op mock method. " "please use assert_called_once_with.") yield (0, msg) + if '.asserthascalled' in uncased_line: + msg = ("N322: Possible use of no-op mock method. " + "please use assert_has_calls.") + yield (0, msg) + def check_oslo_namespace_imports(logical_line): if re.match(oslo_namespace_imports_from_dot, logical_line): diff --git a/neutron/tests/unit/agent/dhcp/test_agent.py b/neutron/tests/unit/agent/dhcp/test_agent.py index 24c0d1056..7a8c138b0 100644 --- a/neutron/tests/unit/agent/dhcp/test_agent.py +++ b/neutron/tests/unit/agent/dhcp/test_agent.py @@ -330,7 +330,9 @@ class TestDhcpAgent(base.BaseTestCase): trace_level='warning', expected_sync=False) - def _test_sync_state_helper(self, known_networks, active_networks): + def _test_sync_state_helper(self, known_net_ids, active_net_ids): + active_networks = set(mock.Mock(id=netid) for netid in active_net_ids) + with mock.patch(DHCP_PLUGIN) as plug: mock_plugin = mock.Mock() mock_plugin.get_active_networks_info.return_value = active_networks @@ -338,23 +340,18 @@ class TestDhcpAgent(base.BaseTestCase): dhcp = dhcp_agent.DhcpAgent(HOSTNAME) - attrs_to_mock = dict( - [(a, mock.DEFAULT) for a in - ['refresh_dhcp_helper', 'disable_dhcp_helper', 'cache']]) + attrs_to_mock = dict([(a, mock.DEFAULT) + for a in ['disable_dhcp_helper', 'cache', + 'safe_configure_dhcp_for_network']]) with mock.patch.multiple(dhcp, **attrs_to_mock) as mocks: - mocks['cache'].get_network_ids.return_value = known_networks + mocks['cache'].get_network_ids.return_value = known_net_ids dhcp.sync_state() - exp_refresh = [ - mock.call(net_id) for net_id in active_networks] - - diff = set(known_networks) - set(active_networks) + diff = set(known_net_ids) - set(active_net_ids) exp_disable = [mock.call(net_id) for net_id in diff] - mocks['cache'].assert_has_calls([mock.call.get_network_ids()]) - mocks['refresh_dhcp_helper'].assert_has_called(exp_refresh) - mocks['disable_dhcp_helper'].assert_has_called(exp_disable) + mocks['disable_dhcp_helper'].assert_has_calls(exp_disable) def test_sync_state_initial(self): self._test_sync_state_helper([], ['a']) @@ -366,19 +363,10 @@ class TestDhcpAgent(base.BaseTestCase): self._test_sync_state_helper(['b'], ['a']) def test_sync_state_waitall(self): - class mockNetwork(object): - id = '0' - admin_state_up = True - subnets = [] - - def __init__(self, id): - self.id = id with mock.patch.object(dhcp_agent.eventlet.GreenPool, 'waitall') as w: - active_networks = [mockNetwork('1'), mockNetwork('2'), - mockNetwork('3'), mockNetwork('4'), - mockNetwork('5')] - known_networks = ['1', '2', '3', '4', '5'] - self._test_sync_state_helper(known_networks, active_networks) + active_net_ids = ['1', '2', '3', '4', '5'] + known_net_ids = ['1', '2', '3', '4', '5'] + self._test_sync_state_helper(known_net_ids, active_net_ids) w.assert_called_once_with() def test_sync_state_plugin_error(self): diff --git a/neutron/tests/unit/hacking/test_checks.py b/neutron/tests/unit/hacking/test_checks.py index b87ad18bc..4ebb4d815 100644 --- a/neutron/tests/unit/hacking/test_checks.py +++ b/neutron/tests/unit/hacking/test_checks.py @@ -92,11 +92,21 @@ class HackingTestCase(base.BaseTestCase): mock.method(1, 2, 3, test='wow') mock.method.assertCalledOnceWith() """ + fail_code3 = """ + mock = Mock() + mock.method(1, 2, 3, test='wow') + mock.method.assert_has_called() + """ pass_code = """ mock = Mock() mock.method(1, 2, 3, test='wow') mock.method.assert_called_once_with() """ + pass_code2 = """ + mock = Mock() + mock.method(1, 2, 3, test='wow') + mock.method.assert_has_calls() + """ self.assertEqual( 1, len(list(checks.check_assert_called_once_with(fail_code1, "neutron/tests/test_assert.py")))) @@ -106,6 +116,12 @@ class HackingTestCase(base.BaseTestCase): self.assertEqual( 0, len(list(checks.check_assert_called_once_with(pass_code, "neutron/tests/test_assert.py")))) + self.assertEqual( + 1, len(list(checks.check_assert_called_once_with(fail_code3, + "neutron/tests/test_assert.py")))) + self.assertEqual( + 0, len(list(checks.check_assert_called_once_with(pass_code2, + "neutron/tests/test_assert.py")))) def test_check_oslo_namespace_imports(self): def check(s, fail=True):