Merge lp:~cjwatson/launchpad/refactor-run-parts-subprocess into lp:launchpad

[Colin Watson]

Refactor publish-ftpmaster to use subprocess.call rather than os.system.

As usual, avoiding unnecessary use of the shell makes things safer and simpler.

[William Grant (wgrant)]

review: Approve (code)

=== added file 'lib/lp/archivepublisher/run_parts.py'
--- lib/lp/archivepublisher/run_parts.py	1970-01-01 00:00:00 +0000
+++ lib/lp/archivepublisher/run_parts.py	2018-01-18 15:33:21 +0000
@@ -0,0 +1,87 @@
+# Copyright 2011-2018 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Publisher support for running programs from a plug-in directory."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+    'execute_subprocess',
+    'find_run_parts_dir',
+    'run_parts',
+    ]
+
+import os
+try:
+    from shlex import quote as shell_quote
+except ImportError:
+    from pipes import quote as shell_quote
+import subprocess
+
+from lp.services.config import config
+from lp.services.scripts.base import LaunchpadScriptFailure
+from lp.services.utils import file_exists
+
+
+def find_run_parts_dir(distribution_name, parts):
+    """Find the requested run-parts directory, if it exists."""
+    run_parts_location = config.archivepublisher.run_parts_location
+    if not run_parts_location:
+        return None
+
+    parts_dir = os.path.join(run_parts_location, distribution_name, parts)
+    if file_exists(parts_dir):
+        return parts_dir
+    else:
+        return None
+
+
+def execute_subprocess(args, log=None, failure=None, **kwargs):
+    """Run `args`, handling logging and failures.
+
+    :param args: Program argument array.
+    :param log: An optional logger.
+    :param failure: Raise `failure` as an exception if the command returns a
+        nonzero value.  If omitted, nonzero return values are ignored.
+    :param kwargs: Other keyword arguments passed on to `subprocess.call`.
+    """
+    if log is not None:
+        log.debug("Executing: %s", " ".join(shell_quote(arg) for arg in args))
+    retval = subprocess.call(args, **kwargs)
+    if retval != 0:
+        if log is not None:
+            log.debug("Command returned %d.", retval)
+        if failure is not None:
+            if log is not None:
+                log.debug("Command failed: %s", failure)
+            raise failure
+
+
+def run_parts(distribution_name, parts, log=None, env=None):
+    """Execute run-parts.
+
+    :param distribution_name: The name of the distribution to execute
+        run-parts scripts for.
+    :param parts: The run-parts directory to execute:
+        "publish-distro.d" or "finalize.d".
+    :param log: An optional logger.
+    :param env: A dict of additional environment variables to pass to the
+        scripts in the run-parts directory, or None.
+    """
+    parts_dir = find_run_parts_dir(distribution_name, parts)
+    if parts_dir is None:
+        if log is not None:
+            log.debug("Skipping run-parts %s: not configured.", parts)
+        return
+    cmd = ["run-parts", "--", parts_dir]
+    failure = LaunchpadScriptFailure(
+        "Failure while executing run-parts %s." % parts_dir)
+    full_env = dict(os.environ)
+    if env is not None:
+        full_env.update(env)
+    scripts_dir = os.path.join(config.root, "cronscripts", "publishing")
+    path_elements = full_env.get("PATH", "").split(os.pathsep)
+    path_elements.append(scripts_dir)
+    full_env["PATH"] = os.pathsep.join(path_elements)
+    execute_subprocess(cmd, log=None, failure=failure, env=full_env)

=== modified file 'lib/lp/archivepublisher/scripts/publish_ftpmaster.py'
--- lib/lp/archivepublisher/scripts/publish_ftpmaster.py	2016-12-05 22:16:25 +0000
+++ lib/lp/archivepublisher/scripts/publish_ftpmaster.py	2018-01-18 15:33:21 +0000
@@ -1,4 +1,4 @@
-# Copyright 2011-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2011-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Master distro publishing script."""
@@ -22,6 +22,10 @@
     cannot_modify_suite,
     GLOBAL_PUBLISHER_LOCK,
     )
+from lp.archivepublisher.run_parts import (
+    execute_subprocess,
+    run_parts,
+    )
 from lp.archivepublisher.scripts.processaccepted import ProcessAccepted
 from lp.archivepublisher.scripts.publishdistro import PublishDistro
 from lp.registry.interfaces.distribution import IDistributionSet
@@ -30,7 +34,6 @@
     pocketsuffix,
     )
 from lp.registry.interfaces.series import SeriesStatus
-from lp.services.config import config
 from lp.services.database.bulk import load_related
 from lp.services.osutils import ensure_directory_exists
 from lp.services.scripts.base import (
@@ -61,43 +64,6 @@
             if archive.purpose in ARCHIVES_TO_PUBLISH]
 
 
-def compose_shell_boolean(boolean_value):
-    """Represent a boolean value as "yes" or "no"."""
-    boolean_text = {
-        True: "yes",
-        False: "no",
-    }
-    return boolean_text[boolean_value]
-
-
-def shell_quote(literal):
-    """Escape `literal` for use in a double-quoted shell string.
-
-    This is a pretty naive substitution: it doesn't deal well with
-    non-ASCII characters or special characters.
-    """
-    # Characters that need backslash-escaping.  Do the backslash itself
-    # first; any escapes we introduced *before* the backslash would have
-    # their own backslashes escaped afterwards when we got to the
-    # backslash itself.
-    special_characters = '\\"$`\n'
-    for escapee in special_characters:
-        literal = literal.replace(escapee, '\\' + escapee)
-    return '"%s"' % literal
-
-
-def compose_env_string(*env_dicts):
-    """Turn dict(s) into a series of shell parameter assignments.
-
-    Values in later dicts override any values with the same respective
-    keys in earlier dicts.
-    """
-    env = {}
-    for env_dict in env_dicts:
-        env.update(env_dict)
-    return ' '.join(['='.join(pair) for pair in env.iteritems()])
-
-
 def get_backup_dists(archive_config):
     """Return the path of an archive's backup dists directory."""
     return os.path.join(archive_config.archiveroot + "-distscopy", "dists")
@@ -122,31 +88,6 @@
     return get_dists(archive_config) + ".in-progress"
 
 
-def extend_PATH():
-    """Produce env dict for extending $PATH.
-
-    Adds the Launchpad source tree's cronscripts/publishing to the
-    existing $PATH.
-
-    :return: a dict suitable for passing to compose_env_string.
-    """
-    scripts_dir = os.path.join(config.root, "cronscripts", "publishing")
-    return {"PATH": '"$PATH":%s' % shell_quote(scripts_dir)}
-
-
-def find_run_parts_dir(distro, parts):
-    """Find the requested run-parts directory, if it exists."""
-    run_parts_location = config.archivepublisher.run_parts_location
-    if not run_parts_location:
-        return
-
-    parts_dir = os.path.join(run_parts_location, distro.name, parts)
-    if file_exists(parts_dir):
-        return parts_dir
-    else:
-        return None
-
-
 def map_distro_pubconfigs(distro):
     """Return dict mapping archive purpose for distro's pub configs.
 
@@ -238,26 +179,6 @@
                     "Distribution %s not found." % self.options.distribution)
             self.distributions = [distro]
 
-    def executeShell(self, command_line, failure=None):
-        """Run `command_line` through a shell.
-
-        This won't just load an external program and run it; the command
-        line goes through the full shell treatment including variable
-        substitutions, output redirections, and so on.
-
-        :param command_line: Shell command.
-        :param failure: Raise `failure` as an exception if the shell
-            command returns a nonzero value.  If omitted, nonzero return
-            values are ignored.
-        """
-        self.logger.debug("Executing: %s" % command_line)
-        retval = os.system(command_line)
-        if retval != 0:
-            self.logger.debug("Command returned %d.", retval)
-            if failure is not None:
-                self.logger.debug("Command failed: %s", failure)
-                raise failure
-
     def getConfigs(self):
         """Set up configuration objects for archives to be published.
 
@@ -370,8 +291,9 @@
         for purpose, archive_config in self.configs[distribution].iteritems():
             dists = get_dists(archive_config)
             backup_dists = get_backup_dists(archive_config)
-            self.executeShell(
-                "rsync -aH --delete '%s/' '%s'" % (dists, backup_dists),
+            execute_subprocess(
+                ["rsync", "-aH", "--delete", "%s/" % dists, backup_dists],
+                log=self.logger,
                 failure=LaunchpadScriptFailure(
                     "Failed to rsync new dists for %s." % purpose.title))
 
@@ -469,12 +391,13 @@
         """Execute the publish-distro hooks."""
         archive_config = self.configs[distribution][archive.purpose]
         env = {
-            'ARCHIVEROOT': shell_quote(archive_config.archiveroot),
-            'DISTSROOT': shell_quote(get_backup_dists(archive_config)),
+            'ARCHIVEROOT': archive_config.archiveroot,
+            'DISTSROOT': get_backup_dists(archive_config),
             }
         if archive_config.overrideroot is not None:
-            env["OVERRIDEROOT"] = shell_quote(archive_config.overrideroot)
-        self.runParts(distribution, 'publish-distro.d', env)
+            env["OVERRIDEROOT"] = archive_config.overrideroot
+        run_parts(
+            distribution.name, 'publish-distro.d', log=self.logger, env=env)
 
     def installDists(self, distribution):
         """Put the new dists into place, as near-atomically as possible.
@@ -499,40 +422,22 @@
     def clearEmptyDirs(self, distribution):
         """Clear out any redundant empty directories."""
         for archive_config in self.configs[distribution].itervalues():
-            self.executeShell(
-                "find '%s' -type d -empty | xargs -r rmdir"
-                % archive_config.archiveroot)
-
-    def runParts(self, distribution, parts, env):
-        """Execute run-parts.
-
-        :param distribution: `Distribution` to execute run-parts scripts for.
-        :param parts: The run-parts directory to execute:
-            "publish-distro.d" or "finalize.d".
-        :param env: A dict of environment variables to pass to the
-            scripts in the run-parts directory.
-        """
-        parts_dir = find_run_parts_dir(distribution, parts)
-        if parts_dir is None:
-            self.logger.debug("Skipping run-parts %s: not configured.", parts)
-            return
-        env_string = compose_env_string(env, extend_PATH())
-        self.executeShell(
-            "env %s run-parts -- '%s'" % (env_string, parts_dir),
-            failure=LaunchpadScriptFailure(
-                "Failure while executing run-parts %s." % parts_dir))
+            execute_subprocess(
+                ["find", archive_config.archiveroot, "-type", "d", "-empty",
+                 "-delete"],
+                log=self.logger)
 
     def runFinalizeParts(self, distribution, security_only=False):
         """Run the finalize.d parts to finalize publication."""
-        archive_roots = shell_quote(' '.join([
+        archive_roots = ' '.join([
             archive_config.archiveroot
-            for archive_config in self.configs[distribution].itervalues()]))
+            for archive_config in self.configs[distribution].itervalues()])
 
         env = {
-            'SECURITY_UPLOAD_ONLY': compose_shell_boolean(security_only),
+            'SECURITY_UPLOAD_ONLY': 'yes' if security_only else 'no',
             'ARCHIVEROOTS': archive_roots,
         }
-        self.runParts(distribution, 'finalize.d', env)
+        run_parts(distribution.name, 'finalize.d', log=self.logger, env=env)
 
     def publishSecurityUploads(self, distribution):
         """Quickly process just the pending security uploads.

=== modified file 'lib/lp/archivepublisher/tests/test_publish_ftpmaster.py'
--- lib/lp/archivepublisher/tests/test_publish_ftpmaster.py	2016-12-05 22:16:25 +0000
+++ lib/lp/archivepublisher/tests/test_publish_ftpmaster.py	2018-01-18 15:33:21 +0000
@@ -1,4 +1,4 @@
-# Copyright 2011-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2011-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test publish-ftpmaster cron script."""
@@ -13,6 +13,8 @@
 from apt_pkg import TagFile
 from fixtures import MonkeyPatch
 from testtools.matchers import (
+    ContainsDict,
+    Equals,
     MatchesException,
     MatchesStructure,
     Not,
@@ -26,20 +28,16 @@
 from lp.archivepublisher.config import getPubConfig
 from lp.archivepublisher.interfaces.publisherconfig import IPublisherConfigSet
 from lp.archivepublisher.scripts.publish_ftpmaster import (
-    compose_env_string,
-    compose_shell_boolean,
-    find_run_parts_dir,
     get_working_dists,
     newer_mtime,
     PublishFTPMaster,
-    shell_quote,
     )
+from lp.archivepublisher.tests.test_run_parts import RunPartsMixin
 from lp.registry.interfaces.pocket import (
     PackagePublishingPocket,
     pocketsuffix,
     )
 from lp.registry.interfaces.series import SeriesStatus
-from lp.services.config import config
 from lp.services.database.interfaces import IMasterStore
 from lp.services.log.logger import (
     BufferLogger,
@@ -60,10 +58,7 @@
     TestCaseWithFactory,
     )
 from lp.testing.fakemethod import FakeMethod
-from lp.testing.layers import (
-    LaunchpadZopelessLayer,
-    ZopelessDatabaseLayer,
-    )
+from lp.testing.layers import LaunchpadZopelessLayer
 
 
 def path_exists(*path_components):
@@ -133,26 +128,6 @@
 class HelpersMixin:
     """Helpers for the PublishFTPMaster tests."""
 
-    def enableRunParts(self, parts_directory=None):
-        """Set up for run-parts execution.
-
-        :param parts_directory: Base location for the run-parts directories.
-            If omitted, a temporary directory will be used.
-        """
-        if parts_directory is None:
-            parts_directory = self.makeTemporaryDirectory()
-            os.makedirs(os.path.join(
-                parts_directory, "ubuntu", "publish-distro.d"))
-            os.makedirs(os.path.join(parts_directory, "ubuntu", "finalize.d"))
-        self.parts_directory = parts_directory
-
-        config.push("run-parts", dedent("""\
-            [archivepublisher]
-            run_parts_location: %s
-            """ % parts_directory))
-
-        self.addCleanup(config.pop, "run-parts")
-
     def makeDistroWithPublishDirectory(self):
         """Create a `Distribution` for testing.
 
@@ -178,44 +153,6 @@
             self.makeTemporaryDirectory())
 
 
-class TestPublishFTPMasterHelpers(TestCase):
-
-    def test_compose_env_string_iterates_env_dict(self):
-        env = {
-            "A": "1",
-            "B": "2",
-        }
-        env_string = compose_env_string(env)
-        self.assertIn(env_string, ["A=1 B=2", "B=2 A=1"])
-
-    def test_compose_env_string_combines_env_dicts(self):
-        env1 = {"A": "1"}
-        env2 = {"B": "2"}
-        env_string = compose_env_string(env1, env2)
-        self.assertIn(env_string, ["A=1 B=2", "B=2 A=1"])
-
-    def test_compose_env_string_overrides_repeated_keys(self):
-        self.assertEqual("A=2", compose_env_string({"A": "1"}, {"A": "2"}))
-
-    def test_shell_quote_quotes_string(self):
-        self.assertEqual('"x"', shell_quote("x"))
-
-    def test_shell_quote_escapes_string(self):
-        self.assertEqual('"\\\\"', shell_quote("\\"))
-
-    def test_shell_quote_does_not_escape_its_own_escapes(self):
-        self.assertEqual('"\\$"', shell_quote("$"))
-
-    def test_shell_quote_escapes_entire_string(self):
-        self.assertEqual('"\\$\\$\\$"', shell_quote("$$$"))
-
-    def test_compose_shell_boolean_shows_True_as_yes(self):
-        self.assertEqual("yes", compose_shell_boolean(True))
-
-    def test_compose_shell_boolean_shows_False_as_no(self):
-        self.assertEqual("no", compose_shell_boolean(False))
-
-
 class TestNewerMtime(TestCase):
 
     def setUp(self):
@@ -256,34 +193,8 @@
         self.assertTrue(newer_mtime(self.a, self.b))
 
 
-class TestFindRunPartsDir(TestCaseWithFactory, HelpersMixin):
-    layer = ZopelessDatabaseLayer
-
-    def test_find_run_parts_dir_finds_runparts_directory(self):
-        self.enableRunParts()
-        ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
-        self.assertEqual(
-            os.path.join(
-                config.root, self.parts_directory, "ubuntu", "finalize.d"),
-            find_run_parts_dir(ubuntu, "finalize.d"))
-
-    def test_find_run_parts_dir_ignores_blank_config(self):
-        self.enableRunParts("")
-        ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
-        self.assertIs(None, find_run_parts_dir(ubuntu, "finalize.d"))
-
-    def test_find_run_parts_dir_ignores_none_config(self):
-        self.enableRunParts("none")
-        ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
-        self.assertIs(None, find_run_parts_dir(ubuntu, "finalize.d"))
-
-    def test_find_run_parts_dir_ignores_nonexistent_directory(self):
-        self.enableRunParts()
-        distro = self.factory.makeDistribution()
-        self.assertIs(None, find_run_parts_dir(distro, "finalize.d"))
-
-
-class TestPublishFTPMasterScript(TestCaseWithFactory, HelpersMixin):
+class TestPublishFTPMasterScript(
+        TestCaseWithFactory, RunPartsMixin, HelpersMixin):
     layer = LaunchpadZopelessLayer
 
     # Location of shell script.
@@ -552,12 +463,14 @@
         script = self.makeScript(distro)
         script.setUp()
         script.setUpDirs()
-        script.runParts = FakeMethod()
+        run_parts_fixture = self.useFixture(MonkeyPatch(
+            "lp.archivepublisher.scripts.publish_ftpmaster.run_parts",
+            FakeMethod()))
         script.publishDistroArchive(distro, distro.main_archive)
-        self.assertEqual(1, script.runParts.call_count)
-        args, kwargs = script.runParts.calls[0]
-        run_distro, parts_dir, env = args
-        self.assertEqual(distro, run_distro)
+        self.assertEqual(1, run_parts_fixture.new_value.call_count)
+        args, _ = run_parts_fixture.new_value.calls[0]
+        run_distro_name, parts_dir = args
+        self.assertEqual(distro.name, run_distro_name)
         self.assertEqual("publish-distro.d", parts_dir)
 
     def test_runPublishDistroParts_passes_parameters(self):
@@ -565,14 +478,19 @@
         script = self.makeScript(distro)
         script.setUp()
         script.setUpDirs()
-        script.runParts = FakeMethod()
+        run_parts_fixture = self.useFixture(MonkeyPatch(
+            "lp.archivepublisher.scripts.publish_ftpmaster.run_parts",
+            FakeMethod()))
         script.runPublishDistroParts(distro, distro.main_archive)
-        args, kwargs = script.runParts.calls[0]
-        run_distro, parts_dir, env = args
-        required_parameters = set([
-            "ARCHIVEROOT", "DISTSROOT", "OVERRIDEROOT"])
-        missing_parameters = required_parameters.difference(set(env.keys()))
-        self.assertEqual(set(), missing_parameters)
+        _, kwargs = run_parts_fixture.new_value.calls[0]
+        distro_config = get_pub_config(distro)
+        self.assertThat(kwargs["env"], ContainsDict({
+            "ARCHIVEROOT": Equals(get_archive_root(distro_config)),
+            "DISTSROOT": Equals(
+                os.path.join(get_distscopy_root(distro_config), "dists")),
+            "OVERRIDEROOT": Equals(
+                get_archive_root(distro_config) + "-overrides"),
+            }))
 
     def test_clearEmptyDirs_cleans_up_empty_directories(self):
         distro = self.makeDistroWithPublishDirectory()
@@ -621,67 +539,16 @@
         script.options.distribution = self.factory.getUniqueString()
         self.assertRaises(LaunchpadScriptFailure, script.processOptions)
 
-    def test_runParts_runs_parts(self):
-        self.enableRunParts()
-        script = self.makeScript(self.prepareUbuntu())
-        script.setUp()
-        distro = script.distributions[0]
-        script.executeShell = FakeMethod()
-        script.runParts(distro, "finalize.d", {})
-        self.assertEqual(1, script.executeShell.call_count)
-        args, kwargs = script.executeShell.calls[-1]
-        command_line, = args
-        self.assertIn("run-parts", command_line)
-        self.assertIn(
-            os.path.join(self.parts_directory, "ubuntu/finalize.d"),
-            command_line)
-
-    def test_runParts_passes_parameters(self):
-        self.enableRunParts()
-        script = self.makeScript(self.prepareUbuntu())
-        script.setUp()
-        distro = script.distributions[0]
-        script.executeShell = FakeMethod()
-        key = self.factory.getUniqueString()
-        value = self.factory.getUniqueString()
-        script.runParts(distro, "finalize.d", {key: value})
-        args, kwargs = script.executeShell.calls[-1]
-        command_line, = args
-        self.assertIn("%s=%s" % (key, value), command_line)
-
-    def test_executeShell_executes_shell_command(self):
-        distro = self.makeDistroWithPublishDirectory()
-        script = self.makeScript(distro)
-        marker = os.path.join(
-            get_pub_config(distro).root_dir, "marker")
-        script.executeShell("touch %s" % marker)
-        self.assertTrue(file_exists(marker))
-
-    def test_executeShell_reports_failure_if_requested(self):
-        distro = self.makeDistroWithPublishDirectory()
-        script = self.makeScript(distro)
-
-        class ArbitraryFailure(Exception):
-            """Some exception that's not likely to come from elsewhere."""
-
-        self.assertRaises(
-            ArbitraryFailure,
-            script.executeShell, "/bin/false", failure=ArbitraryFailure())
-
-    def test_executeShell_does_not_report_failure_if_not_requested(self):
-        distro = self.makeDistroWithPublishDirectory()
-        script = self.makeScript(distro)
-        # The test is that this does not fail:
-        script.executeShell("/bin/false")
-
     def test_runFinalizeParts_passes_parameters(self):
         script = self.makeScript(self.prepareUbuntu())
         script.setUp()
         distro = script.distributions[0]
-        script.runParts = FakeMethod()
+        run_parts_fixture = self.useFixture(MonkeyPatch(
+            "lp.archivepublisher.scripts.publish_ftpmaster.run_parts",
+            FakeMethod()))
         script.runFinalizeParts(distro)
-        args, kwargs = script.runParts.calls[0]
-        run_distro, parts_dir, env = args
+        _, kwargs = run_parts_fixture.new_value.calls[0]
+        env = kwargs["env"]
         required_parameters = set(["ARCHIVEROOTS", "SECURITY_UPLOAD_ONLY"])
         missing_parameters = required_parameters.difference(set(env.keys()))
         self.assertEqual(set(), missing_parameters)
@@ -798,15 +665,11 @@
         self.assertContentEqual(
             [distro.main_archive, partner_archive], published_archives)
 
-    def test_runFinalizeParts_quotes_archiveroots(self):
-        # Passing ARCHIVEROOTS to the finalize.d scripts is a bit
-        # difficult because the variable holds multiple values in a
-        # single, double-quoted string.  Escaping and quoting a sequence
-        # of escaped and quoted items won't work.
-        # This test establishes how a script can sanely deal with the
-        # list.  It'll probably go wrong if the configured archive root
-        # contains spaces and such, but should work with Unix-sensible
-        # paths.
+    def test_runFinalizeParts_passes_archiveroots_correctly(self):
+        # The ARCHIVEROOTS environment variable may contain spaces, and
+        # these are passed through correctly.  It'll go wrong if the
+        # configured archive root contains whitespace, but works with
+        # Unix-sensible paths.
         distro = self.makeDistroWithPublishDirectory()
         self.factory.makeArchive(
             distribution=distro, purpose=ArchivePurpose.PARTNER)

=== added file 'lib/lp/archivepublisher/tests/test_run_parts.py'
--- lib/lp/archivepublisher/tests/test_run_parts.py	1970-01-01 00:00:00 +0000
+++ lib/lp/archivepublisher/tests/test_run_parts.py	2018-01-18 15:33:21 +0000
@@ -0,0 +1,113 @@
+# Copyright 2011-2018 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test publisher run-parts handling."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+
+import os.path
+
+from fixtures import MonkeyPatch
+from testtools.matchers import (
+    ContainsDict,
+    Equals,
+    FileExists,
+    )
+
+from lp.archivepublisher.run_parts import (
+    execute_subprocess,
+    find_run_parts_dir,
+    run_parts,
+    )
+from lp.services.config import config
+from lp.services.log.logger import DevNullLogger
+from lp.testing import TestCase
+from lp.testing.fakemethod import FakeMethod
+
+
+class RunPartsMixin:
+    """Helper for run-parts tests."""
+
+    def enableRunParts(self, parts_directory=None):
+        """Set up for run-parts execution.
+
+        :param parts_directory: Base location for the run-parts directories.
+            If omitted, a temporary directory will be used.
+        """
+        if parts_directory is None:
+            parts_directory = self.makeTemporaryDirectory()
+            os.makedirs(os.path.join(
+                parts_directory, "ubuntu", "publish-distro.d"))
+            os.makedirs(os.path.join(parts_directory, "ubuntu", "finalize.d"))
+        self.parts_directory = parts_directory
+        self.pushConfig("archivepublisher", run_parts_location=parts_directory)
+
+
+class TestFindRunPartsDir(TestCase, RunPartsMixin):
+
+    def test_finds_runparts_directory(self):
+        self.enableRunParts()
+        self.assertEqual(
+            os.path.join(
+                config.root, self.parts_directory, "ubuntu", "finalize.d"),
+            find_run_parts_dir("ubuntu", "finalize.d"))
+
+    def test_ignores_blank_config(self):
+        self.enableRunParts("")
+        self.assertIs(None, find_run_parts_dir("ubuntu", "finalize.d"))
+
+    def test_ignores_none_config(self):
+        self.enableRunParts("none")
+        self.assertIs(None, find_run_parts_dir("ubuntu", "finalize.d"))
+
+    def test_ignores_nonexistent_directory(self):
+        self.enableRunParts()
+        self.assertIs(None, find_run_parts_dir("nonexistent", "finalize.d"))
+
+
+class TestExecuteSubprocess(TestCase):
+
+    def test_executes_shell_command(self):
+        marker = os.path.join(self.makeTemporaryDirectory(), "marker")
+        execute_subprocess(["touch", marker])
+        self.assertThat(marker, FileExists())
+
+    def test_reports_failure_if_requested(self):
+        class ArbitraryFailure(Exception):
+            """Some exception that's not likely to come from elsewhere."""
+
+        self.assertRaises(
+            ArbitraryFailure,
+            execute_subprocess, ["/bin/false"], failure=ArbitraryFailure())
+
+    def test_does_not_report_failure_if_not_requested(self):
+        # The test is that this does not fail:
+        execute_subprocess(["/bin/false"])
+
+
+class TestRunParts(TestCase, RunPartsMixin):
+
+    def test_runs_parts(self):
+        self.enableRunParts()
+        execute_subprocess_fixture = self.useFixture(MonkeyPatch(
+            "lp.archivepublisher.run_parts.execute_subprocess", FakeMethod()))
+        run_parts("ubuntu", "finalize.d", log=DevNullLogger(), env={})
+        self.assertEqual(1, execute_subprocess_fixture.new_value.call_count)
+        args, kwargs = execute_subprocess_fixture.new_value.calls[-1]
+        self.assertEqual(
+            (["run-parts", "--",
+              os.path.join(self.parts_directory, "ubuntu/finalize.d")],),
+            args)
+
+    def test_passes_parameters(self):
+        self.enableRunParts()
+        execute_subprocess_fixture = self.useFixture(MonkeyPatch(
+            "lp.archivepublisher.run_parts.execute_subprocess", FakeMethod()))
+        key = self.factory.getUniqueString()
+        value = self.factory.getUniqueString()
+        run_parts(
+            "ubuntu", "finalize.d", log=DevNullLogger(), env={key: value})
+        args, kwargs = execute_subprocess_fixture.new_value.calls[-1]
+        self.assertThat(kwargs["env"], ContainsDict({key: Equals(value)}))