Merge ~cjwatson/launchpad:improve-config-fixture into launchpad:master

[Colin Watson]

Allow removing sections from ConfigFixture

This makes it possible to tear down some configuration that couldn't previously be torn down properly.

[Ioana Lasc (ilasc)]

review: Approve

diff --git a/lib/lp/services/config/fixture.py b/lib/lp/services/config/fixture.py
index d79fe37..10426e4 100644
--- a/lib/lp/services/config/fixture.py
+++ b/lib/lp/services/config/fixture.py
@@ -1,15 +1,13 @@
 # Copyright 2010-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-"""Fixtures related to configs.
-
-XXX: Robert Collins 2010-10-20 bug=663454 this is in the wrong namespace.
-"""
+"""Fixtures related to configs."""
 
 __metaclass__ = type
 
 __all__ = [
     'ConfigFixture',
+    'ConfigMismatchError',
     'ConfigUseFixture',
     ]
 
@@ -19,10 +17,16 @@ from textwrap import dedent
 
 from fixtures import Fixture
 import scandir
+import six
+from six.moves.configparser import RawConfigParser
 
 from lp.services.config import config
 
 
+class ConfigMismatchError(Exception):
+    """Removing configuration failed due to a mismatch in expected contents."""
+
+
 class ConfigFixture(Fixture):
     """Create a unique launchpad config."""
 
@@ -40,15 +44,84 @@ class ConfigFixture(Fixture):
         self.instance_name = instance_name
         self.copy_from_instance = copy_from_instance
 
-    def add_section(self, sectioncontent):
-        """Add sectioncontent to the lazy config."""
-        with open(self.absroot + '/launchpad-lazr.conf', 'a') as out:
-            out.write(sectioncontent)
-        # Trigger a refresh if and only if the config is in use at the moment
-        # in order to make these new values available.
+    def _parseConfigData(self, conf_data, conf_filename):
+        """Parse a single chunk of config data, with no inheritance."""
+        # Compare https://bugs.launchpad.net/lazr.config/+bug/1397779.
+        parser_kws = {}
+        if six.PY3:
+            parser_kws['strict'] = False
+        parser = RawConfigParser(**parser_kws)
+        parser.readfp(six.StringIO(conf_data), conf_filename)
+        return parser
+
+    def _parseConfigFile(self, conf_filename):
+        """Parse a single config file, with no inheritance."""
+        if os.path.exists(conf_filename):
+            with open(conf_filename) as conf_file:
+                conf_data = conf_file.read()
+        else:
+            conf_data = ''
+        return self._parseConfigData(conf_data, conf_filename)
+
+    def _writeConfigFile(self, parser, conf_filename):
+        """Write a parsed config to a file."""
+        with open(conf_filename, 'w') as conf_file:
+            for i, section in enumerate(parser.sections()):
+                if i:
+                    conf_file.write('\n')
+                conf_file.write('[%s]\n' % section)
+                for key, value in parser.items(section):
+                    conf_file.write(
+                        '%s: %s\n' % (key, str(value).replace('\n', '\n\t')))
+
+    def _refresh(self):
+        """Trigger a config refresh if necessary.
+
+        If and only if the config is in use at the moment, we need to
+        refresh in order to make changes available.
+        """
         if config.instance_name == self.instance_name:
             config._invalidateConfig()
 
+    def add_section(self, sectioncontent):
+        """Add sectioncontent to the lazr config."""
+        conf_filename = os.path.join(self.absroot, 'launchpad-lazr.conf')
+        parser = self._parseConfigFile(conf_filename)
+        add_parser = self._parseConfigData(
+            sectioncontent, '<configuration to add>')
+        for section in add_parser.sections():
+            if not parser.has_section(section):
+                parser.add_section(section)
+            for name, value in add_parser.items(section):
+                parser.set(section, name, value)
+        self._writeConfigFile(parser, conf_filename)
+        self._refresh()
+
+    def remove_section(self, sectioncontent):
+        """Remove sectioncontent from the lazr config."""
+        conf_filename = os.path.join(self.absroot, 'launchpad-lazr.conf')
+        parser = self._parseConfigFile(conf_filename)
+        remove_parser = self._parseConfigData(
+            sectioncontent, '<configuration to remove>')
+        for section in remove_parser.sections():
+            if not parser.has_section(section):
+                continue
+            for name, value in remove_parser.items(section):
+                if not parser.has_option(section, name):
+                    continue
+                current_value = parser.get(section, name)
+                if value != current_value:
+                    raise ConfigMismatchError(
+                        "Can't remove %s.%s option from %s: "
+                        "expected value '%s', current value '%s'" % (
+                            section, name, conf_filename,
+                            value, current_value))
+                parser.remove_option(section, name)
+            if not parser.options(section):
+                parser.remove_section(section)
+        self._writeConfigFile(parser, conf_filename)
+        self._refresh()
+
     def _setUp(self):
         root = os.path.join(config.root, 'configs', self.instance_name)
         os.mkdir(root)
diff --git a/lib/lp/services/config/tests/test_fixture.py b/lib/lp/services/config/tests/test_fixture.py
index 7db44b4..639d292 100644
--- a/lib/lp/services/config/tests/test_fixture.py
+++ b/lib/lp/services/config/tests/test_fixture.py
@@ -5,13 +5,16 @@
 
 __metaclass__ = type
 
-from testtools import TestCase
+import os.path
+from textwrap import dedent
 
 from lp.services.config import config
 from lp.services.config.fixture import (
     ConfigFixture,
+    ConfigMismatchError,
     ConfigUseFixture,
     )
+from lp.testing import TestCase
 
 
 class TestConfigUseFixture(TestCase):
@@ -54,3 +57,86 @@ class TestConfigFixture(TestCase):
                 lazr_config.strip())
         finally:
             fixture.cleanUp()
+
+    def test_add_and_remove_section(self):
+        fixture = ConfigFixture('testtestconfig', 'testrunner')
+        fixture.setUp()
+        try:
+            confpath = 'configs/testtestconfig/launchpad-lazr.conf'
+            with open(confpath) as f:
+                lazr_config = f.read()
+            self.assertEqual(dedent("""\
+                [meta]
+                extends: ../testrunner/launchpad-lazr.conf
+                """), lazr_config)
+
+            fixture.add_section(dedent("""\
+                [test1]
+                key: false
+                """))
+            with open(confpath) as f:
+                lazr_config = f.read()
+            self.assertEqual(dedent("""\
+                [meta]
+                extends: ../testrunner/launchpad-lazr.conf
+
+                [test1]
+                key: false
+                """), lazr_config)
+
+            fixture.add_section(dedent("""\
+                [test2]
+                key: true
+                """))
+            with open(confpath) as f:
+                lazr_config = f.read()
+            self.assertEqual(dedent("""\
+                [meta]
+                extends: ../testrunner/launchpad-lazr.conf
+
+                [test1]
+                key: false
+
+                [test2]
+                key: true
+                """), lazr_config)
+
+            fixture.remove_section(dedent("""\
+                [test1]
+                key: false
+                """))
+            with open(confpath) as f:
+                lazr_config = f.read()
+            self.assertEqual(dedent("""\
+                [meta]
+                extends: ../testrunner/launchpad-lazr.conf
+
+                [test2]
+                key: true
+                """), lazr_config)
+        finally:
+            fixture.cleanUp()
+
+    def test_remove_section_unexpected_value(self):
+        fixture = ConfigFixture('testtestconfig', 'testrunner')
+        fixture.setUp()
+        try:
+            confpath = os.path.abspath(
+                'configs/testtestconfig/launchpad-lazr.conf')
+
+            fixture.add_section(dedent("""\
+                [test1]
+                key: false
+                """))
+
+            self.assertRaisesWithContent(
+                ConfigMismatchError,
+                "Can't remove test1.key option from %s: "
+                "expected value 'true', current value 'false'" % confpath,
+                fixture.remove_section,
+                dedent("""\
+                    [test1]
+                    key: true
+                    """))
+        finally:
+            fixture.cleanUp()
diff --git a/lib/lp/services/librarianserver/testing/server.py b/lib/lp/services/librarianserver/testing/server.py
index 12c50d6..20bfdfd 100644
--- a/lib/lp/services/librarianserver/testing/server.py
+++ b/lib/lp/services/librarianserver/testing/server.py
@@ -92,6 +92,8 @@ class LibrarianServerFixture(TacTestSetup):
             warnings.warn("Attempt to tearDown inactive fixture.",
                 DeprecationWarning, stacklevel=3)
             return
+        self.config_fixture.remove_section(self.service_config)
+        config.reloadConfig()
         TacTestSetup.cleanUp(self)
 
     def clear(self):
diff --git a/lib/lp/testing/layers.py b/lib/lp/testing/layers.py
index ebd1f96..3b72c73 100644
--- a/lib/lp/testing/layers.py
+++ b/lib/lp/testing/layers.py
@@ -663,11 +663,12 @@ class RabbitMQLayer(BaseLayer):
     def tearDown(cls):
         if not cls._is_setup:
             return
+        cls.appserver_config_fixture.remove_section(
+            cls.rabbit.config.service_config)
+        cls.config_fixture.remove_section(
+            cls.rabbit.config.service_config)
         cls.rabbit.cleanUp()
         cls._is_setup = False
-        # Can't pop the config above, so bail here and let the test runner
-        # start a sub-process.
-        raise NotImplementedError
 
     @classmethod
     @profiled
@@ -795,8 +796,8 @@ class LibrarianLayer(DatabaseLayer):
 
         # Make sure things using the appserver config know the
         # correct Librarian port numbers.
-        cls.appserver_config_fixture.add_section(
-            cls.librarian_fixture.service_config)
+        cls.appserver_service_config = cls.librarian_fixture.service_config
+        cls.appserver_config_fixture.add_section(cls.appserver_service_config)
 
     @classmethod
     @profiled
@@ -805,6 +806,8 @@ class LibrarianLayer(DatabaseLayer):
         # responsibilities : not desirable though.
         if cls.librarian_fixture is None:
             return
+        cls.appserver_config_fixture.remove_section(
+            cls.appserver_service_config)
         try:
             cls._check_and_reset()
         finally:
diff --git a/lib/lp/testing/swift/fixture.py b/lib/lp/testing/swift/fixture.py
index 3359380..2158b29 100644
--- a/lib/lp/testing/swift/fixture.py
+++ b/lib/lp/testing/swift/fixture.py
@@ -67,6 +67,9 @@ class SwiftFixture(TacTestFixture):
                 fakeswift.DEFAULT_PASSWORD, fakeswift.DEFAULT_TENANT_NAME))
         BaseLayer.config_fixture.add_section(service_config)
         config.reloadConfig()
+        self.addCleanup(config.reloadConfig)
+        self.addCleanup(
+            BaseLayer.config_fixture.remove_section, service_config)
         assert config.librarian_server.os_tenant_name == 'test'
 
     def setUpRoot(self):