Merge "Don't set CONF options directly in unit tests"

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)

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))

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),

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)

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)
 

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)
 

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 = {

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')