diff --git a/nova/hacking/checks.py b/nova/hacking/checks.py index ac0c43ee9c..c35b18a76b 100644 --- a/nova/hacking/checks.py +++ b/nova/hacking/checks.py @@ -49,6 +49,7 @@ asse_equal_end_with_none_re = re.compile( r"(.)*assertEqual\((\w|\.|\'|\"|\[|\])+, None\)") asse_equal_start_with_none_re = re.compile( r"(.)*assertEqual\(None, (\w|\.|\'|\"|\[|\])+\)") +conf_attribute_set_re = re.compile(r"CONF\.[a-z0-9_.]+\s*=\s*\w") def import_no_db_in_virt(logical_line, filename): @@ -243,6 +244,22 @@ def no_translate_debug_logs(logical_line, filename): yield(0, "N319 Don't translate debug level logs") +def no_setting_conf_directly_in_tests(logical_line, filename): + """Check for setting CONF.* attributes directly in tests + + The value can leak out of tests affecting how subsequent tests run. + Using self.flags(option=value) is the preferred method to temporarily + set config options in tests. + + N320 + """ + if 'nova/tests/' in filename: + res = conf_attribute_set_re.match(logical_line) + if res: + yield (0, "N320: Setting CONF.* attributes directly in tests is " + "forbidden. Use self.flags(option=value) instead") + + def factory(register): register(import_no_db_in_virt) register(no_db_session_in_public_api) @@ -256,3 +273,4 @@ def factory(register): register(assert_equal_type) register(assert_equal_none) register(no_translate_debug_logs) + register(no_setting_conf_directly_in_tests) diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py index 86c05da04f..6ee4fd5001 100644 --- a/nova/tests/compute/test_compute.py +++ b/nova/tests/compute/test_compute.py @@ -604,7 +604,7 @@ class ComputeVolumeTestCase(BaseTestCase): # None of the mocks should be called. self.mox.ReplayAll() - CONF.bandwidth_poll_interval = 0 + self.flags(bandwidth_poll_interval=0) self.compute._poll_bandwidth_usage(ctxt) self.mox.UnsetStubs() @@ -627,7 +627,7 @@ class ComputeVolumeTestCase(BaseTestCase): NotImplementedError) self.mox.ReplayAll() - CONF.bandwidth_poll_interval = 1 + self.flags(bandwidth_poll_interval=1) self.compute._poll_bandwidth_usage(ctxt) # A second call won't call the stubs again as the bandwidth # poll is now disabled @@ -662,7 +662,7 @@ class ComputeVolumeTestCase(BaseTestCase): # None of the mocks should be called. self.mox.ReplayAll() - CONF.volume_usage_poll_interval = 0 + self.flags(volume_usage_poll_interval=0) self.compute._poll_volume_usage(ctxt) self.mox.UnsetStubs() @@ -676,7 +676,7 @@ class ComputeVolumeTestCase(BaseTestCase): self.compute._get_host_volume_bdms(ctxt).AndReturn([]) self.mox.ReplayAll() - CONF.volume_usage_poll_interval = 10 + self.flags(volume_usage_poll_interval=10) self.compute._poll_volume_usage(ctxt) self.mox.UnsetStubs() @@ -692,7 +692,7 @@ class ComputeVolumeTestCase(BaseTestCase): self.compute._get_host_volume_bdms(ctxt).AndReturn([1, 2]) self.compute._update_volume_usage_cache(ctxt, [3, 4]) self.mox.ReplayAll() - CONF.volume_usage_poll_interval = 10 + self.flags(volume_usage_poll_interval=10) self.compute._poll_volume_usage(ctxt) self.mox.UnsetStubs() @@ -742,7 +742,7 @@ class ComputeVolumeTestCase(BaseTestCase): # Poll volume usage & then detach the volume. This will update the # total fields in the volume usage cache. - CONF.volume_usage_poll_interval = 10 + self.flags(volume_usage_poll_interval=10) self.compute._poll_volume_usage(self.context) # Check that a volume.usage and volume.attach notification was sent self.assertEqual(2, len(fake_notifier.NOTIFICATIONS)) diff --git a/nova/tests/network/test_linux_net.py b/nova/tests/network/test_linux_net.py index 016a9f75ab..380c09c8f4 100644 --- a/nova/tests/network/test_linux_net.py +++ b/nova/tests/network/test_linux_net.py @@ -593,7 +593,7 @@ class LinuxNetworkTestCase(test.NoDBTestCase): default_domain = CONF.dhcp_domain for domain in ('', default_domain): executes = [] - CONF.dhcp_domain = domain + self.flags(dhcp_domain=domain) linux_net.restart_dhcp(self.context, dev, network_ref) expected = ['env', 'CONFIG_FILE=%s' % jsonutils.dumps(CONF.dhcpbridge_flagfile), diff --git a/nova/tests/test_hacking.py b/nova/tests/test_hacking.py index ed7fbbfc6d..d4b689d8ab 100644 --- a/nova/tests/test_hacking.py +++ b/nova/tests/test_hacking.py @@ -123,3 +123,24 @@ class HackingTestCase(test.NoDBTestCase): self.assertEqual(len(list(checks.no_translate_debug_logs( "LOG.info(_('foo'))", "nova/scheduler/foo.py"))), 0) + + def test_no_setting_conf_directly_in_tests(self): + self.assertEqual(len(list(checks.no_setting_conf_directly_in_tests( + "CONF.option = 1", "nova/tests/test_foo.py"))), 1) + + self.assertEqual(len(list(checks.no_setting_conf_directly_in_tests( + "CONF.group.option = 1", "nova/tests/test_foo.py"))), 1) + + self.assertEqual(len(list(checks.no_setting_conf_directly_in_tests( + "CONF.option = foo = 1", "nova/tests/test_foo.py"))), 1) + + # Shouldn't fail with comparisons + self.assertEqual(len(list(checks.no_setting_conf_directly_in_tests( + "CONF.option == 'foo'", "nova/tests/test_foo.py"))), 0) + + self.assertEqual(len(list(checks.no_setting_conf_directly_in_tests( + "CONF.option != 1", "nova/tests/test_foo.py"))), 0) + + # Shouldn't fail since not in nova/tests/ + self.assertEqual(len(list(checks.no_setting_conf_directly_in_tests( + "CONF.option = 1", "nova/compute/foo.py"))), 0) diff --git a/nova/tests/test_utils.py b/nova/tests/test_utils.py index 9efed945f1..6969240cac 100644 --- a/nova/tests/test_utils.py +++ b/nova/tests/test_utils.py @@ -955,7 +955,7 @@ class GetImageFromSystemMetadataTestCase(test.NoDBTestCase): sys_meta = self.get_system_metadata() sys_meta["%soo1" % utils.SM_IMAGE_PROP_PREFIX] = "bar" - CONF.non_inheritable_image_properties = ["foo1"] + self.flags(non_inheritable_image_properties=["foo1"]) image = utils.get_image_from_system_metadata(sys_meta) diff --git a/nova/tests/test_wsgi.py b/nova/tests/test_wsgi.py index f6707be51f..c91c3ee05a 100644 --- a/nova/tests/test_wsgi.py +++ b/nova/tests/test_wsgi.py @@ -104,7 +104,7 @@ class TestWSGIServer(test.NoDBTestCase): self.assertEqual("test_app", server.name) def test_custom_max_header_line(self): - CONF.max_header_line = 4096 # Default value is 16384. + self.flags(max_header_line=4096) # Default value is 16384. nova.wsgi.Server("test_custom_max_header_line", None) self.assertEqual(CONF.max_header_line, eventlet.wsgi.MAX_HEADER_LINE) diff --git a/nova/tests/virt/baremetal/test_pxe.py b/nova/tests/virt/baremetal/test_pxe.py index 185d51e2b8..297bb4d01c 100644 --- a/nova/tests/virt/baremetal/test_pxe.py +++ b/nova/tests/virt/baremetal/test_pxe.py @@ -285,15 +285,12 @@ class PXEClassMethodsTestCase(BareMetalPXETestCase): flavor = utils.get_test_flavor() # Raises an exception when options are neither specified # on the instance nor in configuration file - CONF.baremetal.deploy_kernel = None - CONF.baremetal.deploy_ramdisk = None self.assertRaises(exception.NovaException, pxe.get_tftp_image_info, self.instance, flavor) # Test that other non-true values also raise an exception - CONF.baremetal.deploy_kernel = "" - CONF.baremetal.deploy_ramdisk = "" + self.flags(deploy_kernel='', deploy_ramdisk='', group='baremetal') self.assertRaises(exception.NovaException, pxe.get_tftp_image_info, self.instance, flavor) @@ -313,8 +310,8 @@ class PXEClassMethodsTestCase(BareMetalPXETestCase): # Here, we confirm both that all four values were set # and that the proper paths are getting set for all of them - CONF.baremetal.deploy_kernel = 'cccc' - CONF.baremetal.deploy_ramdisk = 'dddd' + self.flags(deploy_kernel='cccc', deploy_ramdisk='dddd', + group='baremetal') base = os.path.join(CONF.baremetal.tftp_root, self.instance['uuid']) res = pxe.get_tftp_image_info(self.instance, flavor) expected = { diff --git a/nova/tests/virt/libvirt/test_libvirt.py b/nova/tests/virt/libvirt/test_libvirt.py index 98ee581d8a..93b907a185 100644 --- a/nova/tests/virt/libvirt/test_libvirt.py +++ b/nova/tests/virt/libvirt/test_libvirt.py @@ -9069,7 +9069,7 @@ class LibvirtVolumeSnapshotTestCase(test.TestCase): self.conn._volume_api, self.conn) def test_volume_snapshot_create(self, quiesce=True): - CONF.instance_name_template = 'instance-%s' + self.flags(instance_name_template='instance-%s') self.mox.StubOutWithMock(self.conn, '_lookup_by_name') self.mox.StubOutWithMock(self.conn, '_volume_api')