launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #27233
[Merge] ~cjwatson/launchpad:improve-config-fixture into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:improve-config-fixture into launchpad:master.
Commit message:
Allow removing sections from ConfigFixture
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/404849
This makes it possible to tear down some configuration that couldn't previously be torn down properly.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:improve-config-fixture into launchpad:master.
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):
