diff options
Diffstat (limited to 'nova')
-rw-r--r-- | nova/hacking/checks.py | 32 | ||||
-rw-r--r-- | nova/tests/unit/api/ec2/test_api.py | 6 | ||||
-rw-r--r-- | nova/tests/unit/cmd/test_idmapshift.py | 2 | ||||
-rw-r--r-- | nova/tests/unit/compute/test_compute_utils.py | 13 | ||||
-rw-r--r-- | nova/tests/unit/test_hacking.py | 48 | ||||
-rw-r--r-- | nova/tests/unit/test_iptables_network.py | 20 | ||||
-rw-r--r-- | nova/tests/unit/virt/libvirt/test_driver.py | 2 | ||||
-rw-r--r-- | nova/tests/unit/virt/libvirt/test_firewall.py | 24 | ||||
-rw-r--r-- | nova/tests/unit/virt/libvirt/test_imagecache.py | 6 | ||||
-rw-r--r-- | nova/tests/unit/virt/xenapi/test_xenapi.py | 4 |
10 files changed, 116 insertions, 41 deletions
diff --git a/nova/hacking/checks.py b/nova/hacking/checks.py index 1b9e9e38b6..7f7ff2a7e5 100644 --- a/nova/hacking/checks.py +++ b/nova/hacking/checks.py @@ -54,6 +54,22 @@ asse_equal_end_with_none_re = re.compile( r"assertEqual\(.*?,\s+None\)$") asse_equal_start_with_none_re = re.compile( r"assertEqual\(None,") +# NOTE(snikitin): Next two regexes weren't united to one for more readability. +# asse_true_false_with_in_or_not_in regex checks +# assertTrue/False(A in B) cases where B argument has no spaces +# asse_true_false_with_in_or_not_in_spaces regex checks cases +# where B argument has spaces and starts/ends with [, ', ". +# For example: [1, 2, 3], "some string", 'another string'. +# We have to separate these regexes to escape a false positives +# results. B argument should have spaces only if it starts +# with [, ", '. Otherwise checking of string +# "assertFalse(A in B and C in D)" will be false positives. +# In this case B argument is "B and C in D". +asse_true_false_with_in_or_not_in = re.compile(r"assert(True|False)\(" + r"(\w|[][.'\"])+( not)? in (\w|[][.'\",])+(, .*)?\)") +asse_true_false_with_in_or_not_in_spaces = re.compile(r"assert(True|False)" + r"\((\w|[][.'\"])+( not)? in [\[|'|\"](\w|[][.'\", ])+" + r"[\[|'|\"](, .*)?\)") conf_attribute_set_re = re.compile(r"CONF\.[a-z0-9_.]+\s*=\s*\w") log_translation = re.compile( r"(.)*LOG\.(audit|error|critical)\(\s*('|\")") @@ -457,6 +473,21 @@ def check_oslo_namespace_imports(logical_line, blank_before, filename): yield(0, msg) +def assert_true_or_false_with_in(logical_line): + """Check for assertTrue/False(A in B), assertTrue/False(A not in B), + assertTrue/False(A in B, message) or assertTrue/False(A not in B, message) + sentences. + + N334 + """ + res = (asse_true_false_with_in_or_not_in.search(logical_line) or + asse_true_false_with_in_or_not_in_spaces.search(logical_line)) + if res: + yield (0, "N334: Use assertIn/NotIn(A, B) rather than " + "assertTrue/False(A in/not in B) when checking collection " + "contents.") + + def factory(register): register(import_no_db_in_virt) register(no_db_session_in_public_api) @@ -479,3 +510,4 @@ def factory(register): register(CheckForStrUnicodeExc) register(CheckForTransAdd) register(check_oslo_namespace_imports) + register(assert_true_or_false_with_in) diff --git a/nova/tests/unit/api/ec2/test_api.py b/nova/tests/unit/api/ec2/test_api.py index cc4a2adb75..5236eda71c 100644 --- a/nova/tests/unit/api/ec2/test_api.py +++ b/nova/tests/unit/api/ec2/test_api.py @@ -285,9 +285,9 @@ class ApiEc2TestCase(test.TestCase): # Any request should be fine self.ec2.get_all_instances() - self.assertTrue(self.ec2.APIVersion in self.http.getresponsebody(), - 'The version in the xmlns of the response does ' - 'not match the API version given in the request.') + self.assertIn(self.ec2.APIVersion, self.http.getresponsebody(), + 'The version in the xmlns of the response does ' + 'not match the API version given in the request.') def test_describe_instances(self): """Test that, after creating a user and a project, the describe diff --git a/nova/tests/unit/cmd/test_idmapshift.py b/nova/tests/unit/cmd/test_idmapshift.py index 2f0fe06bc0..215a797b28 100644 --- a/nova/tests/unit/cmd/test_idmapshift.py +++ b/nova/tests/unit/cmd/test_idmapshift.py @@ -75,7 +75,7 @@ class FindTargetIDTestCase(BaseTestCase): def test_find_target_id_updates_memo(self): memo = dict() idmapshift.find_target_id(0, self.uid_maps, idmapshift.NOBODY_ID, memo) - self.assertTrue(0 in memo) + self.assertIn(0, memo) self.assertEqual(10000, memo[0]) def test_find_target_guest_id_greater_than_count(self): diff --git a/nova/tests/unit/compute/test_compute_utils.py b/nova/tests/unit/compute/test_compute_utils.py index 805b3e360b..c53254d635 100644 --- a/nova/tests/unit/compute/test_compute_utils.py +++ b/nova/tests/unit/compute/test_compute_utils.py @@ -487,8 +487,8 @@ class UsageInfoTestCase(test.TestCase): 'state', 'state_description', 'bandwidth', 'audit_period_beginning', 'audit_period_ending', 'image_meta'): - self.assertTrue(attr in payload, - msg="Key %s not in payload" % attr) + self.assertIn(attr, payload, + "Key %s not in payload" % attr) self.assertEqual(payload['image_meta'], {'md_key1': 'val1', 'md_key2': 'val2'}) image_ref_url = "%s/images/1" % glance.generate_glance_url() @@ -528,8 +528,7 @@ class UsageInfoTestCase(test.TestCase): 'state', 'state_description', 'bandwidth', 'audit_period_beginning', 'audit_period_ending', 'image_meta'): - self.assertTrue(attr in payload, - msg="Key %s not in payload" % attr) + self.assertIn(attr, payload, "Key %s not in payload" % attr) self.assertEqual(payload['image_meta'], {'md_key1': 'val1', 'md_key2': 'val2'}) image_ref_url = "%s/images/1" % glance.generate_glance_url() @@ -559,8 +558,7 @@ class UsageInfoTestCase(test.TestCase): 'state', 'state_description', 'bandwidth', 'audit_period_beginning', 'audit_period_ending', 'image_meta'): - self.assertTrue(attr in payload, - msg="Key %s not in payload" % attr) + self.assertIn(attr, payload, "Key %s not in payload" % attr) self.assertEqual(payload['image_meta'], {}) image_ref_url = "%s/images/1" % glance.generate_glance_url() self.assertEqual(payload['image_ref_url'], image_ref_url) @@ -595,8 +593,7 @@ class UsageInfoTestCase(test.TestCase): self.assertEqual(str(payload['instance_flavor_id']), str(flavor_id)) for attr in ('display_name', 'created_at', 'launched_at', 'state', 'state_description', 'image_meta'): - self.assertTrue(attr in payload, - msg="Key %s not in payload" % attr) + self.assertIn(attr, payload, "Key %s not in payload" % attr) self.assertEqual(payload['image_meta'], {'md_key1': 'val1', 'md_key2': 'val2'}) self.assertEqual(payload['image_name'], 'fake_name') diff --git a/nova/tests/unit/test_hacking.py b/nova/tests/unit/test_hacking.py index 84ca6362cc..36433f1aa1 100644 --- a/nova/tests/unit/test_hacking.py +++ b/nova/tests/unit/test_hacking.py @@ -143,6 +143,54 @@ class HackingTestCase(test.NoDBTestCase): self.assertEqual( len(list(checks.assert_equal_none("self.assertIsNone()"))), 0) + def test_assert_true_or_false_with_in_or_not_in(self): + self.assertEqual(len(list(checks.assert_equal_none( + "self.assertEqual(A, None)"))), 1) + self.assertEqual(len(list(checks.assert_true_or_false_with_in( + "self.assertTrue(A in B)"))), 1) + + self.assertEqual(len(list(checks.assert_true_or_false_with_in( + "self.assertFalse(A in B)"))), 1) + + self.assertEqual(len(list(checks.assert_true_or_false_with_in( + "self.assertTrue(A not in B)"))), 1) + + self.assertEqual(len(list(checks.assert_true_or_false_with_in( + "self.assertFalse(A not in B)"))), 1) + + self.assertEqual(len(list(checks.assert_true_or_false_with_in( + "self.assertTrue(A in B, 'some message')"))), 1) + + self.assertEqual(len(list(checks.assert_true_or_false_with_in( + "self.assertFalse(A in B, 'some message')"))), 1) + + self.assertEqual(len(list(checks.assert_true_or_false_with_in( + "self.assertTrue(A not in B, 'some message')"))), 1) + + self.assertEqual(len(list(checks.assert_true_or_false_with_in( + "self.assertFalse(A not in B, 'some message')"))), 1) + + self.assertEqual(len(list(checks.assert_true_or_false_with_in( + "self.assertTrue(A in 'some string with spaces')"))), 1) + + self.assertEqual(len(list(checks.assert_true_or_false_with_in( + "self.assertTrue(A in 'some string with spaces')"))), 1) + + self.assertEqual(len(list(checks.assert_true_or_false_with_in( + "self.assertTrue(A in ['1', '2', '3'])"))), 1) + + self.assertEqual(len(list(checks.assert_true_or_false_with_in( + "self.assertTrue(A in [1, 2, 3])"))), 1) + + self.assertEqual(len(list(checks.assert_true_or_false_with_in( + "self.assertTrue(any(A > 5 for A in B))"))), 0) + + self.assertEqual(len(list(checks.assert_true_or_false_with_in( + "self.assertTrue(any(A > 5 for A in B), 'some message')"))), 0) + + self.assertEqual(len(list(checks.assert_true_or_false_with_in( + "self.assertFalse(some in list1 and some2 in list2)"))), 0) + def test_no_translate_debug_logs(self): self.assertEqual(len(list(checks.no_translate_debug_logs( "LOG.debug(_('foo'))", "nova/scheduler/foo.py"))), 1) diff --git a/nova/tests/unit/test_iptables_network.py b/nova/tests/unit/test_iptables_network.py index 870d5a021f..5afdf55c1f 100644 --- a/nova/tests/unit/test_iptables_network.py +++ b/nova/tests/unit/test_iptables_network.py @@ -163,14 +163,13 @@ class IptablesManagerTestCase(test.NoDBTestCase): ':%s-snat - [0:0]' % (self.binary_name), ':%s-PREROUTING - [0:0]' % (self.binary_name), ':%s-POSTROUTING - [0:0]' % (self.binary_name)]: - self.assertTrue(line in new_lines, "One of our chains went" + self.assertIn(line, new_lines, "One of our chains went" " missing.") seen_lines = set() for line in new_lines: line = line.strip() - self.assertTrue(line not in seen_lines, - "Duplicate line: %s" % line) + self.assertNotIn(line, seen_lines, "Duplicate line: %s" % line) seen_lines.add(line) last_postrouting_line = '' @@ -179,7 +178,7 @@ class IptablesManagerTestCase(test.NoDBTestCase): if line.startswith('[0:0] -A POSTROUTING'): last_postrouting_line = line - self.assertTrue('-j nova-postrouting-bottom' in last_postrouting_line, + self.assertIn('-j nova-postrouting-bottom', last_postrouting_line, "Last POSTROUTING rule does not jump to " "nova-postouting-bottom: %s" % last_postrouting_line) @@ -198,22 +197,21 @@ class IptablesManagerTestCase(test.NoDBTestCase): ':%s-INPUT - [0:0]' % (self.binary_name), ':%s-local - [0:0]' % (self.binary_name), ':%s-OUTPUT - [0:0]' % (self.binary_name)]: - self.assertTrue(line in new_lines, "One of our chains went" + self.assertIn(line, new_lines, "One of our chains went" " missing.") seen_lines = set() for line in new_lines: line = line.strip() - self.assertTrue(line not in seen_lines, - "Duplicate line: %s" % line) + self.assertNotIn(line, seen_lines, "Duplicate line: %s" % line) seen_lines.add(line) for chain in ['FORWARD', 'OUTPUT']: for line in new_lines: if line.startswith('[0:0] -A %s' % chain): - self.assertTrue('-j nova-filter-top' in line, - "First %s rule does not " - "jump to nova-filter-top" % chain) + self.assertIn('-j nova-filter-top', line, + "First %s rule does not " + "jump to nova-filter-top" % chain) break self.assertTrue('[0:0] -A nova-filter-top ' @@ -233,7 +231,7 @@ class IptablesManagerTestCase(test.NoDBTestCase): for line in ['*filter', 'COMMIT']: - self.assertTrue(line in new_lines, "One of iptables key lines " + self.assertIn(line, new_lines, "One of iptables key lines " "went missing.") self.assertTrue(len(new_lines) > 4, "No iptables rules added") diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index cfaf3f3a66..00507ee1cd 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -10396,7 +10396,7 @@ Active: 8381604 kB conn.post_live_migration_at_destination( self.context, instance, network_info, True, block_device_info=block_device_info) - self.assertTrue('fake' in self.resultXML) + self.assertIn('fake', self.resultXML) self.assertTrue( block_device_info['block_device_mapping'][0].save.called) diff --git a/nova/tests/unit/virt/libvirt/test_firewall.py b/nova/tests/unit/virt/libvirt/test_firewall.py index 41b0917814..c71d29c434 100644 --- a/nova/tests/unit/virt/libvirt/test_firewall.py +++ b/nova/tests/unit/virt/libvirt/test_firewall.py @@ -286,8 +286,8 @@ class IptablesFirewallTestCase(test.NoDBTestCase): self.in_rules) for rule in in_rules: if 'nova' not in rule: - self.assertTrue(rule in self.out_rules, - 'Rule went missing: %s' % rule) + self.assertIn(rule, self.out_rules, + 'Rule went missing: %s' % rule) instance_chain = None for rule in self.out_rules: @@ -573,9 +573,9 @@ class NWFilterTestCase(test.NoDBTestCase): self.recursive_depends[name] = [] for f in dom.getElementsByTagName('filterref'): ref = f.getAttribute('filter') - self.assertTrue(ref in self.defined_filters, - ('%s referenced filter that does ' + - 'not yet exist: %s') % (name, ref)) + self.assertIn(ref, self.defined_filters, + ('%s referenced filter that does ' + + 'not yet exist: %s') % (name, ref)) dependencies = [ref] + self.recursive_depends[ref] self.recursive_depends[name] += dependencies @@ -598,14 +598,14 @@ class NWFilterTestCase(test.NoDBTestCase): else: required_not_list.append('allow-dhcp-server') for required in requiredlist: - self.assertTrue(required in - self.recursive_depends[instance_filter], - "Instance's filter does not include %s" % - required) + self.assertIn(required, + self.recursive_depends[instance_filter], + "Instance's filter does not include %s" % + required) for required_not in required_not_list: - self.assertFalse(required_not in - self.recursive_depends[instance_filter], - "Instance filter includes %s" % required_not) + self.assertNotIn(required_not, + self.recursive_depends[instance_filter], + "Instance filter includes %s" % required_not) network_info = _fake_network_info(self.stubs, 1) # since there is one (network_info) there is one vif diff --git a/nova/tests/unit/virt/libvirt/test_imagecache.py b/nova/tests/unit/virt/libvirt/test_imagecache.py index f46f7f4fd2..5eb80b5681 100644 --- a/nova/tests/unit/virt/libvirt/test_imagecache.py +++ b/nova/tests/unit/virt/libvirt/test_imagecache.py @@ -180,7 +180,7 @@ class ImageCacheManagerTestCase(test.NoDBTestCase): self.assertNotIn(unexpected, image_cache_manager.originals) self.assertEqual(1, len(image_cache_manager.back_swap_images)) - self.assertTrue('swap_1000' in image_cache_manager.back_swap_images) + self.assertIn('swap_1000', image_cache_manager.back_swap_images) def test_list_backing_images_small(self): self.stubs.Set(os, 'listdir', @@ -875,8 +875,8 @@ class ImageCacheManagerTestCase(test.NoDBTestCase): image_cache_manager._age_and_verify_swap_images(None, '/tmp_age_test') self.assertEqual(1, len(expected_exist)) self.assertEqual(1, len(expected_remove)) - self.assertTrue('swap_128' in expected_exist) - self.assertTrue('swap_256' in expected_remove) + self.assertIn('swap_128', expected_exist) + self.assertIn('swap_256', expected_remove) class VerifyChecksumTestCase(test.NoDBTestCase): diff --git a/nova/tests/unit/virt/xenapi/test_xenapi.py b/nova/tests/unit/virt/xenapi/test_xenapi.py index 4b7edd5c48..a927244f53 100644 --- a/nova/tests/unit/virt/xenapi/test_xenapi.py +++ b/nova/tests/unit/virt/xenapi/test_xenapi.py @@ -2654,8 +2654,8 @@ class XenAPIDom0IptablesFirewallTestCase(stubs.XenAPITestBase): self._in_rules) for rule in in_rules: if 'nova' not in rule: - self.assertTrue(rule in self._out_rules, - 'Rule went missing: %s' % rule) + self.assertIn(rule, self._out_rules, + 'Rule went missing: %s' % rule) instance_chain = None for rule in self._out_rules: |