Merge lp:~robru/cupstream2distro/reorg-managers into lp:cupstream2distro

[Robert Bruce Park]

Reorganize BuildManager and MergeManager classes.

[Robert Bruce Park (robru)]

Build job is looking good in staging, need to double check that I haven't broken publish or merge, also I haven't updated the tests to the new classes yet.

[Robert Bruce Park (robru)]

I've found a bug in which the vivid build in a dual silo gets confused and watches the wily build a second time during watch phase.

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1128
http://jenkins.qa.ubuntu.com/job/cu2d-choo-choo-ci/802/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/cu2d-choo-choo-ci/802/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1129
http://jenkins.qa.ubuntu.com/job/cu2d-choo-choo-ci/803/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/cu2d-choo-choo-ci/803/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1131
http://jenkins.qa.ubuntu.com/job/cu2d-choo-choo-ci/804/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/cu2d-choo-choo-ci/804/rebuild

review: Approve (continuous-integration)

[Robert Bruce Park (robru)]

Ok I've tested this in staging and I'm happy with it, also I'm pleased with the tests, will go live first thing during my shift tomorrow.

review: Approve

=== modified file 'chroot-tools/buildsource-chroot'
--- chroot-tools/buildsource-chroot	2015-09-23 07:14:54 +0000
+++ chroot-tools/buildsource-chroot	2015-09-24 07:13:28 +0000
@@ -115,4 +115,3 @@
 
 $SU "./debian/rules clean"
 $SU "bzr bd -S -- -sa -k$KEY -d -v$DISTRO_VERSION"
-$SU "bzr revert"

=== modified file 'citrain/abandon.py'
--- citrain/abandon.py	2015-09-22 00:58:14 +0000
+++ citrain/abandon.py	2015-09-24 07:13:28 +0000
@@ -34,10 +34,10 @@
 
 sys.path.append(os.path.dirname(os.path.dirname(__file__)))
 
+from citrain.recipes.manager import Manager
 from cupstream2distro.errors import CITrainError
 from cupstream2distro.utils import env, run_script
 from cupstream2distro.silomanager import SiloState
-from citrain.merge_clean import MergeManager
 
 
 def main():
@@ -51,7 +51,7 @@
         SiloState.requestid.fget.cache[(silo_state,)] = env.REQUEST_ID
         if silo_state._siloname:
             env.SILONAME = silo_state._siloname
-            MergeManager(silo_state).do('nuke')
+            Manager(silo_state).do('nuke')
     except CITrainError as err:
         logging.error(str(err))
     finally:

=== modified file 'citrain/build.py'
--- citrain/build.py	2015-09-22 18:51:46 +0000
+++ citrain/build.py	2015-09-24 07:13:28 +0000
@@ -50,44 +50,31 @@
 
 sys.path.append(os.path.dirname(os.path.dirname(__file__)))
 
-from citrain.recipes.base import BuildBase
-from citrain.recipes.merge import Merge
-from citrain.recipes.manual import Manual
-from citrain.recipes.secondary import Secondary
-from citrain.recipes.sourcesync import SourceSync
-from citrain.recipes.binarysync import BinarySync
+from glob import glob
+from os.path import basename
+
+from citrain.recipes.manager import Manager
 from cupstream2distro.silomanager import SiloState
 from cupstream2distro.errors import BuildError, CITrainError
-from cupstream2distro.utils import (
-    env,
-    find_all_uploaded,
-    log_value_of,
-    run_script,
-)
-
-
-class BuildManager(object):
-    """Decide which packages to build, and when."""
-
-    def __init__(self, silo_state):
-        self.silo_state = silo_state
-        self.names = set()
-        self.phases = []
-        self.builds = {}
-        self.types = {}
-
-    def do(self, *phases):
-        """Perform all specified phases.
-
-        :param phases: A list of phase name strings.
-        :raises: CITrainError in case anything goes wrong.
-        """
-        if not self.types:
-            self.choose_packages()
-            self.choose_classes()
-            self.instantiate_build_objects()
-        for phase in phases:
-            self.execute_phase(phase)
+from cupstream2distro.utils import SILO_DIR, env, run_script
+
+
+def find_all_uploaded():
+    """Return all source package names and versions uploaded to the ppa.
+
+    :yields: Source package names that have been uploaded.
+    """
+    for uploaded in glob(SILO_DIR('*_source.ppa.upload')):
+        yield basename(uploaded).split('_')[0]
+
+
+class BuildManager(Manager):
+    """Honor PACKAGES_TO_REBUILD during building."""
+
+    def include_all(self):
+        """Ignore PACKAGES_TO_REBUILD and include all."""
+        self.choose_packages = super().choose_packages
+        self.types.clear()
 
     def choose_packages(self):
         """Decide which packages need to be built."""
@@ -104,68 +91,24 @@
         if self.names:
             logging.info('Including {}.'.format(', '.join(self.names)))
         else:
-            # Don't bother raising this a second time during diff phase...
-            self.do = lambda *ignore: None
             raise BuildError(
                 'No packages are being considered. You probably want one of '
                 'PACKAGES_TO_REBUILD, FORCE_REBUILD, or WATCH_ONLY.')
 
-    def choose_classes(self):
-        """Decide which class to use for each individual package build."""
-        include_binaries = env.INCLUDE_BINARIES_IN_SYNC == 'true'
-        Sync = BinarySync if include_binaries else SourceSync
-        Source = Sync if self.silo_state.source_archive else Manual
-        self.types = {
-            Merge: self.names & set(self.silo_state.mps),
-            Source: self.names & set(self.silo_state.sources),
-        }
-        if self.silo_state.source_archive:
-            Sync.from_archive = self.silo_state.source_archive
-            Sync.from_series = self.silo_state.source_series
-        Merge.all_mps = self.silo_state.mps
-
-    def instantiate_build_objects(self):
-        """Instantiate the chosen classes for each chosen package."""
-        series = self.silo_state.series
-        dest = self.silo_state.dest
-        ppa = self.silo_state.ppa
-        for Build, source_names in self.types.items():
-            for name in source_names:
-                self.builds[name] = Build(name, series, dest, ppa)
-                if self.silo_state.dual:
-                    second = Secondary(name, series, dest, ppa)
-                    self.builds['zzz_' + name] = second
-
-    def execute_phase(self, phase):
-        """Execute a single phase for each available build.
-
-        :param phase: The name of the phase to execute.
-        :raises: A CITrainError in case anything goes wrong.
-        """
-        log_value_of.phase('Beginning')
-        nop = lambda *ignore: None
-        getattr(BuildBase, 'pre_{}_phase'.format(phase), nop)(self.silo_state)
-        for build in sorted(self.builds):
-            getattr(self.builds[build], '{}_phase'.format(phase), nop)()
-        if BuildBase.failures:
-            raise BuildError('. '.join(
-                fail.strip('. ') for fail in sorted(BuildBase.failures)))
-        getattr(BuildBase, 'post_{}_phase'.format(phase), nop)(self.silo_state)
-
-
-def main(manager=BuildManager):
+
+def main(buildmanager=BuildManager):
     """Execute the build, logging & saving any errors.
 
     :returns: 0 on success, 1 on any failure.
     """
-    silo_state = SiloState(env.SILONAME, primary=True)
-    buildmanager = manager(silo_state)
-    phases = ['validate']
-    if env.WATCH_ONLY != 'true':
-        phases += ['clean', 'collect', 'build', 'upload']
-    phases += ['watch', 'diff']
     try:
-        buildmanager.do(*phases)
+        silo_state = SiloState(env.SILONAME, primary=True)
+        manager = buildmanager(silo_state)
+        manager.do('validate')
+        if env.WATCH_ONLY != 'true':
+            manager.do('clean', 'collect', 'build', 'upload')
+        manager.include_all()
+        manager.do('watch', 'diff')
         silo_state.status = 'Packages built.'
     except CITrainError as err:
         logging.error(str(err))

=== modified file 'citrain/merge_clean.py'
--- citrain/merge_clean.py	2015-09-21 22:21:12 +0000
+++ citrain/merge_clean.py	2015-09-24 07:13:28 +0000
@@ -37,19 +37,10 @@
 
 sys.path.append(os.path.dirname(os.path.dirname(__file__)))
 
+from citrain.recipes.manager import Manager
 from cupstream2distro.errors import CITrainError
 from cupstream2distro.utils import env, run_script
 from cupstream2distro.silomanager import SiloState
-from citrain.build import BuildManager
-
-
-class MergeManager(BuildManager):
-    """Instantiate Build objects necessary for merging."""
-
-    def choose_packages(self):
-        """Act on all silo packages."""
-        self.names = set(self.silo_state.all_projects)
-        logging.info('Including {}.'.format(', '.join(self.names)))
 
 
 def main():
@@ -59,7 +50,7 @@
     """
     try:
         silo_state = SiloState(env.SILONAME, primary=True)
-        MergeManager(silo_state).do(
+        Manager(silo_state).do(
             'validate', 'enumeration', 'merge', 'push', 'nuke')
         silo_state.status = 'Landed'
     except CITrainError as err:

=== modified file 'citrain/migration.py'
--- citrain/migration.py	2015-09-11 00:25:12 +0000
+++ citrain/migration.py	2015-09-24 07:13:28 +0000
@@ -31,10 +31,11 @@
 
 sys.path.append(os.path.dirname(os.path.dirname(__file__)))
 
+from citrain.recipes.manager import Manager
 from cupstream2distro.errors import CITrainError
 from cupstream2distro.utils import env, run_script
 from cupstream2distro.silomanager import SiloState
-from citrain.merge_clean import MergeManager, main as merge_main
+from citrain.merge_clean import main as merge_main
 
 
 def main():
@@ -46,9 +47,8 @@
         env.SILONAME = silo_state
         if silo_state.is_published:
             logging.info('Inspecting silo {}:'.format(silo_state.ppa.web_link))
-            mergemanager = MergeManager(silo_state)
             try:
-                mergemanager.do('migration')
+                Manager(silo_state).do('migration')
                 merge_main()
             except CITrainError as err:
                 logging.info(str(err) + '\n')

=== modified file 'citrain/publisher.py'
--- citrain/publisher.py	2015-09-22 18:51:46 +0000
+++ citrain/publisher.py	2015-09-24 07:13:28 +0000
@@ -43,7 +43,7 @@
 
 sys.path.append(os.path.dirname(os.path.dirname(__file__)))
 
-from citrain.merge_clean import MergeManager
+from citrain.recipes.manager import Manager
 from cupstream2distro.version import V
 from cupstream2distro.archive import Archive
 from cupstream2distro.branchhandling import Branch
@@ -210,7 +210,7 @@
         silo_state = SiloState(env.SILONAME, primary=True)
         dest = silo_state.dest
         ppa = silo_state.ppa
-        MergeManager(silo_state).do(
+        Manager(silo_state).do(
             'checkstatus', 'validate', 'diff', 'checkupload', 'ackaging')
         get_distro_and_series(silo_state.series)
         silo_state.status = 'Publishing.'

=== added file 'citrain/recipes/manager.py'
--- citrain/recipes/manager.py	1970-01-01 00:00:00 +0000
+++ citrain/recipes/manager.py	2015-09-24 07:13:28 +0000
@@ -0,0 +1,109 @@
+# Copyright (C) 2015 Canonical
+#
+# This program is free software; you can redistribute it and/or modify it under
+# the terms of the GNU General Public License as published by the Free Software
+# Foundation; version 3.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+# FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more
+# details.
+#
+# You should have received a copy of the GNU General Public License along with
+# this program; if not, write to the Free Software Foundation, Inc.,
+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+"""CI Train Manager Classes
+
+These classes manage the instantiation of recipes and execution of phases.
+"""
+
+import logging
+import sys
+import os
+
+sys.path.append(os.path.dirname(os.path.dirname(__file__)))
+
+from citrain.recipes.base import BuildBase
+from citrain.recipes.merge import Merge
+from citrain.recipes.manual import Manual
+from citrain.recipes.secondary import Secondary
+from citrain.recipes.sourcesync import SourceSync
+from citrain.recipes.binarysync import BinarySync
+from cupstream2distro.errors import CITrainError
+from cupstream2distro.utils import env, log_value_of
+
+
+class Manager:
+    """Coordinate instantiation & execution of build recipes."""
+
+    def __init__(self, silo_state):
+        self.silo_state = silo_state
+        self.names = set()
+        self.phases = []
+        self.builds = {}
+        self.types = {}
+
+    def do(self, *phases):
+        """Perform all specified phases.
+
+        :param phases: A list of phase name strings.
+        :raises: CITrainError in case anything goes wrong.
+        """
+        if not self.types:
+            self.choose_packages()
+            self.choose_classes()
+            self.instantiate_build_objects()
+        for phase in phases:
+            self.execute_phase(phase)
+
+    def include_all(self):
+        """Nop, implemented by subclass."""
+        pass
+
+    def choose_packages(self):
+        """Act on all silo packages."""
+        self.names = set(self.silo_state.all_projects)
+        logging.info('Including {}.'.format(', '.join(self.names)))
+
+    def choose_classes(self):
+        """Decide which class to use for each individual package build."""
+        include_binaries = env.INCLUDE_BINARIES_IN_SYNC == 'true'
+        Sync = BinarySync if include_binaries else SourceSync
+        Source = Sync if self.silo_state.source_archive else Manual
+        self.types = {
+            Merge: self.names & set(self.silo_state.mps),
+            Source: self.names & set(self.silo_state.sources),
+        }
+        if self.silo_state.source_archive:
+            Sync.from_archive = self.silo_state.source_archive
+            Sync.from_series = self.silo_state.source_series
+        Merge.all_mps = self.silo_state.mps
+
+    def instantiate_build_objects(self):
+        """Instantiate the chosen classes for each chosen package."""
+        series = self.silo_state.series
+        dest = self.silo_state.dest
+        ppa = self.silo_state.ppa
+        for Build, source_names in self.types.items():
+            for name in source_names:
+                self.builds[name] = Build(name, series, dest, ppa)
+                if self.silo_state.dual:
+                    second = Secondary(name, series, dest, ppa)
+                    self.builds['zzz_' + name] = second
+
+    def execute_phase(self, phase):
+        """Execute a single phase for each available build.
+
+        :param phase: The name of the phase to execute.
+        :raises: A CITrainError in case anything goes wrong.
+        """
+        log_value_of.phase('Beginning')
+        nop = lambda *ignore: None
+        getattr(BuildBase, 'pre_{}_phase'.format(phase), nop)(self.silo_state)
+        for build in sorted(self.builds):
+            getattr(self.builds[build], '{}_phase'.format(phase), nop)()
+        if BuildBase.failures:
+            raise CITrainError('. '.join(
+                fail.strip('. ') for fail in sorted(BuildBase.failures)))
+        getattr(BuildBase, 'post_{}_phase'.format(phase), nop)(self.silo_state)

=== modified file 'citrain/recipes/merge.py'
--- citrain/recipes/merge.py	2015-09-09 00:14:07 +0000
+++ citrain/recipes/merge.py	2015-09-24 07:13:28 +0000
@@ -221,6 +221,7 @@
 
     def merge_phase(self):
         """Merge destination trunk back into local branch, if necessary."""
+        self.revert_branch()
         target = self.target = Merge.all_mps[self.name][0].target_branch
         logging.info('Merging {} to {}.'.format(target.web_link, self.path))
         self.merge_branch(target.web_link, 'Resync trunk.', unchanged=False)

=== modified file 'citrain/revert.py'
--- citrain/revert.py	2015-06-23 20:07:17 +0000
+++ citrain/revert.py	2015-09-24 07:13:28 +0000
@@ -45,11 +45,12 @@
 
 sys.path.append(os.path.dirname(os.path.dirname(__file__)))
 
+from citrain.recipes.manager import Manager
+from citrain.recipes.sourcesync import SourceSync
 from cupstream2distro.version import V
-from citrain.merge_clean import MergeManager
 from cupstream2distro.project import DotProject
 from cupstream2distro.archive import sort_by_date
-from citrain.build import SourceSync, main as build_main
+from citrain.build import main as build_main
 from cupstream2distro.errors import RevertError
 from cupstream2distro.utils import (
     SILO_DIR,
@@ -101,7 +102,7 @@
         DotProject.soft_save(self.name, dest=current, ours=new_version)
 
 
-class RevertManager(MergeManager):
+class RevertManager(Manager):
     """Orchestrate the reverting of packages."""
 
     def choose_classes(self):

=== modified file 'cupstream2distro/branchhandling.py'
--- cupstream2distro/branchhandling.py	2015-09-11 19:50:59 +0000
+++ cupstream2distro/branchhandling.py	2015-09-24 07:13:28 +0000
@@ -129,6 +129,10 @@
         log_call('bzr init .', cwd=self.path)
         log_call('bzr add .', cwd=self.path)
 
+    def revert_branch(self):
+        """Revert any changes in the branch."""
+        log_call('bzr revert', cwd=self.path)
+
     def get_branch(self, url):
         """Branch url to self.path.
 

=== modified file 'cupstream2distro/packagelist.py' (properties changed: +x to -x)
=== modified file 'cupstream2distro/packagemanager.py'
--- cupstream2distro/packagemanager.py	2015-09-14 06:12:33 +0000
+++ cupstream2distro/packagemanager.py	2015-09-24 07:13:28 +0000
@@ -160,8 +160,11 @@
     def get_package_version(self):
         """Get newest version from debian changelog."""
         key = 'Version: '
-        instance, stdout, stderr = call(['dpkg-parsechangelog'], cwd=self.path)
-        if instance.returncode != 0:
+        try:
+            inst, stdout, stderr = call(['dpkg-parsechangelog'], cwd=self.path)
+        except FileNotFoundError:
+            return None
+        if inst.returncode != 0:
             raise PackageError(stderr)
         for line in stdout.splitlines():
             if line.startswith(key):

=== modified file 'cupstream2distro/utils.py'
--- cupstream2distro/utils.py	2015-09-22 06:53:49 +0000
+++ cupstream2distro/utils.py	2015-09-24 07:13:28 +0000
@@ -21,15 +21,14 @@
 import logging
 import subprocess
 
-from glob import glob
 from six import text_type
 from functools import wraps
 from base64 import b64encode
 from pickle import dumps, loads
 from inspect import currentframe
 from contextlib import contextmanager
+from os.path import exists, isfile, join
 from six.moves.urllib.parse import unquote
-from os.path import basename, exists, isfile, join
 
 from cupstream2distro.settings import SILOS_DIR
 
@@ -43,15 +42,6 @@
     return '/'.join([ppa.owner.name, ppa.distribution.name, ppa.name])
 
 
-def find_all_uploaded():
-    """Return all source package names and versions uploaded to the ppa.
-
-    :yields: Source package names that have been uploaded.
-    """
-    for uploaded in glob(SILO_DIR('*_source.ppa.upload')):
-        yield basename(uploaded).split('_')[0]
-
-
 @contextmanager
 def suppress(*exceptions):
     """Silence the given exceptions."""

=== modified file 'tests/unit/test_branchhandling.py'
--- tests/unit/test_branchhandling.py	2015-09-11 20:00:49 +0000
+++ tests/unit/test_branchhandling.py	2015-09-24 07:13:28 +0000
@@ -77,6 +77,12 @@
         ])
 
     @patch('cupstream2distro.branchhandling.log_call')
+    def test_revert_branch(self, call_mock):
+        """Correctly revert branches."""
+        self.branch.revert_branch()
+        call_mock.assert_called_once_with('bzr revert', cwd=self.branch.path)
+
+    @patch('cupstream2distro.branchhandling.log_call')
     def test_branching(self, call_mock):
         """We correcly branch a branch."""
         path = self.branch.path = join(self.tempdir, 'foo')

=== modified file 'tests/unit/test_packagemanager.py'
--- tests/unit/test_packagemanager.py	2015-09-14 06:52:53 +0000
+++ tests/unit/test_packagemanager.py	2015-09-24 07:13:28 +0000
@@ -123,6 +123,11 @@
         with utf8_open(self.package.debian_changelog) as data:
             self.assertEqual(data.read(), expected)
 
+    def test_get_package_version_nonexistant_dir(self):
+        """Get latest packaging version."""
+        self.package.path = '/totally/nonexistant'
+        self.assertIsNone(self.package.get_package_version())
+
     def test_get_package_version(self):
         """Get latest packaging version."""
         self.prep_debian_file('changelog', s.NESTED_CHANGELOG)

=== modified file 'tests/unit/test_recipe_binarysync.py'
--- tests/unit/test_recipe_binarysync.py	2015-08-21 05:56:14 +0000
+++ tests/unit/test_recipe_binarysync.py	2015-09-24 07:13:28 +0000
@@ -16,7 +16,7 @@
 
 """Tests for CI Train Binary Sync Recipe."""
 
-from mock import Mock
+from mock import Mock, patch
 
 from tests.unit import DirectoryAwareTestCase
 
@@ -33,6 +33,7 @@
         self.series = Mock()
         self.ppa = Mock()
 
+    @patch('citrain.recipes.binarysync.BinarySync.get_archive_version', Mock())
     def test_binarysync_upload_phase(self):
         """Copy package binaries from one archive to another."""
         from_archive = BinarySync.from_archive = Mock()

=== added file 'tests/unit/test_recipe_manager.py'
--- tests/unit/test_recipe_manager.py	1970-01-01 00:00:00 +0000
+++ tests/unit/test_recipe_manager.py	2015-09-24 07:13:28 +0000
@@ -0,0 +1,154 @@
+# Copyright: (C) 2015 Canonical
+#
+# This program is free software; you can redistribute it and/or modify it under
+# the terms of the GNU General Public License as published by the Free Software
+# Foundation; version 3.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+# FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more
+# details.
+#
+# You should have received a copy of the GNU General Public License along with
+# this program; if not, write to the Free Software Foundation, Inc.,
+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+"""Tests for CI Train Recipe Manager."""
+
+from mock import Mock, call, patch
+
+from tests.unit import DirectoryAwareTestCase
+
+from citrain.recipes.manager import Manager
+from cupstream2distro.errors import CITrainError
+from cupstream2distro.utils import env
+
+# Unused import necessary for code coverage reporting
+from citrain import build as build_module
+COVERAGE = [build_module]
+
+
+class BuildTestCase(DirectoryAwareTestCase):
+    """Test CI Train Recipe Manager."""
+
+    def test_manager_do(self):
+        """Run the specified phases."""
+        silo_state = Mock()
+        bman = Manager(silo_state)
+        bman.choose_packages = Mock()
+        bman.choose_classes = Mock()
+        bman.instantiate_build_objects = Mock()
+        bman.execute_phase = Mock()
+        bman.do('solid', 'liquid', 'gas')
+        bman.choose_packages.assert_called_once_with()
+        bman.choose_classes.assert_called_once_with()
+        bman.instantiate_build_objects.assert_called_once_with()
+        self.assertEqual(
+            bman.execute_phase.mock_calls, [
+                call('solid'), call('liquid'), call('gas')
+            ])
+
+    def test_manager_include_all(self):
+        """Do nothing."""
+        Manager(Mock()).include_all()
+
+    def test_manager_choose_packages(self):
+        """Manager should choose all packages."""
+        silo_state = Mock(all_projects=list('foobar'))
+        manager = Manager(silo_state)
+        manager.choose_packages()
+        self.assertEqual(manager.names, set(['f', 'o', 'b', 'a', 'r']))
+
+    @patch('citrain.recipes.manager.Merge')
+    @patch('citrain.recipes.manager.SourceSync')
+    def test_manager_choose_classes(self, source, merge):
+        """Determine what classes to build with."""
+        silo_state = Mock(
+            all_projects=['a', 'b', 'c'], mps=['a'], sources=['b', 'c'])
+        env.INCLUDE_BINARIES_IN_SYNC = 'false'
+        bman = Manager(silo_state)
+        bman.names = set(['a', 'b'])
+        bman.choose_classes()
+        self.assertEqual(
+            bman.types, {
+                merge: set(['a']),
+                source: set(['b']),
+            })
+        self.assertEqual(merge.all_mps, silo_state.mps)
+        self.assertEqual(source.from_series, silo_state.source_series)
+
+    def test_manager_instantiate_build_objects(self):
+        """Create all build objects."""
+        silo_state = Mock(
+            all_projects=['a', 'b', 'c'], mps=['a'], sources=['b', 'c'],
+            dual=None)
+        merge = Mock()
+        source = Mock()
+        bman = Manager(silo_state)
+        bman.types = {
+            merge: set('a'),
+            source: set('b'),
+        }
+        bman.instantiate_build_objects()
+        self.assertEqual(
+            bman.builds, {
+                'a': merge.return_value,
+                'b': source.return_value,
+            })
+        merge.assert_called_once_with(
+            'a', silo_state.series, silo_state.dest, silo_state.ppa)
+        source.assert_called_once_with(
+            'b', silo_state.series, silo_state.dest, silo_state.ppa)
+
+    @patch('citrain.recipes.manager.Secondary')
+    def test_manager_instantiate_build_objs_scndry(self, second):
+        """Create Secondary build objects."""
+        silo_state = Mock(
+            all_projects=['a', 'b', 'c'], mps=['a'], sources=['b', 'c'],
+            dual='vivid')
+        merge = Mock()
+        source = Mock()
+        bman = Manager(silo_state)
+        bman.types = {
+            merge: set('a'),
+            source: set('b'),
+        }
+        bman.instantiate_build_objects()
+        self.assertEqual(
+            bman.builds, {
+                'a': merge.return_value,
+                'b': source.return_value,
+                'zzz_a': second.return_value,
+                'zzz_b': second.return_value,
+            })
+
+    @patch('citrain.recipes.manager.BuildBase')
+    def test_manager_execute_phase(self, bbase):
+        """Execute one phase for all builds."""
+        silo_state = Mock()
+        bbase.failures = set()
+        bman = Manager(silo_state)
+        merge = Mock()
+        sync = Mock()
+        bman.builds = {'a': merge, 'b': sync}
+        bman.execute_phase('liquid')
+        bbase.pre_liquid_phase.assert_called_once_with(silo_state)
+        merge.liquid_phase.assert_called_once_with()
+        sync.liquid_phase.assert_called_once_with()
+        bbase.post_liquid_phase.assert_called_once_with(silo_state)
+
+    @patch('citrain.recipes.manager.BuildBase')
+    def test_manager_execute_phase_failures(self, bbase):
+        """Raise error when phase sets a failure."""
+        silo_state = Mock()
+        bbase.failures = {'something broke', 'foo'}
+        bman = Manager(silo_state)
+        merge = Mock()
+        sync = Mock()
+        bman.builds = {'a': merge, 'b': sync}
+        with self.assertRaisesRegexp(CITrainError, 'foo. something broke'):
+            bman.execute_phase('liquid')
+        bbase.pre_liquid_phase.assert_called_once_with(silo_state)
+        merge.liquid_phase.assert_called_once_with()
+        sync.liquid_phase.assert_called_once_with()
+        self.assertEqual(bbase.post_liquid_phase.mock_calls, [])

=== modified file 'tests/unit/test_recipe_merge.py'
--- tests/unit/test_recipe_merge.py	2015-09-23 03:36:24 +0000
+++ tests/unit/test_recipe_merge.py	2015-09-24 07:13:28 +0000
@@ -370,12 +370,14 @@
 
     @patch('citrain.recipes.merge.lp')
     @patch('citrain.recipes.merge.Merge.merge_branch', Mock())
+    @patch('citrain.recipes.merge.Merge.revert_branch', Mock())
     def test_merge_merge_phase(self, lp_mock):
         """Merge target back to branch before pushing."""
         merge1, merge2 = [Mock(target_branch=Mock(web_link='example.biz'))] * 2
         Merge.all_mps = {'foo-pack': [merge1, merge2]}
         merge = Merge('foo-pack', self.series, self.dest, self.ppa)
         merge.merge_phase()
+        merge.revert_branch.assert_called_once_with()
         merge.merge_branch.assert_called_once_with(
             'example.biz', 'Resync trunk.', unchanged=False)
 

=== modified file 'tests/unit/test_recipe_secondary.py'
--- tests/unit/test_recipe_secondary.py	2015-09-22 06:53:49 +0000
+++ tests/unit/test_recipe_secondary.py	2015-09-24 07:13:28 +0000
@@ -66,6 +66,7 @@
     @patch(MOD + 'lp')
     @patch(MOD + 'os')
     @patch(MOD + 'shutil')
+    @patch(MOD + 'Secondary.get_package_version', Mock())
     @patch(MOD + 'Secondary.correct_package_changelog', Mock())
     def test_secondary_collect_phase(self, sh_mock, os_mock, lp_mock):
         """Ensure secondary collection works."""
@@ -86,6 +87,7 @@
     @patch(MOD + 'lp')
     @patch(MOD + 'os')
     @patch(MOD + 'shutil')
+    @patch(MOD + 'Secondary.get_package_version', Mock())
     @patch(MOD + 'Secondary.correct_package_changelog', Mock())
     def test_secondary_collect_phase_skip(self, sh_mock, os_mock, lp_mock):
         """Secondary collection should ignore manual sources, binary syncs."""

=== modified file 'tests/unit/test_script_abandon.py'
--- tests/unit/test_script_abandon.py	2015-09-21 23:16:33 +0000
+++ tests/unit/test_script_abandon.py	2015-09-24 07:13:28 +0000
@@ -36,15 +36,15 @@
         silo_state._siloname = 'ubuntu/landing-xyz'
         self.script.SiloState.requestid.fget.cache = {}
         self.script.env.REQUEST_ID = '500'
-        self.script.MergeManager = Mock()
-        mergemanager = self.script.MergeManager.return_value
+        self.script.Manager = Mock()
+        mergemanager = self.script.Manager.return_value
         self.assertEqual(self.script.main(), 0)
         self.assertEqual(self.script.SiloState.requestid.fget.cache, {
             (silo_state,): '500'
         })
         self.script.SiloState.assert_called_once_with(
             siloname=None, primary=False)
-        self.script.MergeManager.assert_called_once_with(silo_state)
+        self.script.Manager.assert_called_once_with(silo_state)
         self.assertEqual(silo_state.status, 'Abandoned')
         mergemanager.do.assert_called_once_with('nuke')
         silo_state.save_config.assert_called_once_with()
@@ -55,11 +55,11 @@
         silo_state._siloname = None
         self.script.SiloState.requestid.fget.cache = {}
         self.script.env.REQUEST_ID = '500'
-        self.script.MergeManager = Mock()
+        self.script.Manager = Mock()
         self.assertEqual(self.script.main(), 0)
         self.script.SiloState.assert_called_once_with(
             siloname=None, primary=False)
-        self.assertEqual(self.script.MergeManager.mock_calls, [])
+        self.assertEqual(self.script.Manager.mock_calls, [])
         self.assertEqual(silo_state.status, 'Abandoned')
         silo_state.save_config.assert_called_once_with()
 
@@ -69,8 +69,8 @@
         silo_state._siloname = 'ubuntu/landing-xyz'
         self.script.SiloState.requestid.fget.cache = {}
         self.script.env.REQUEST_ID = '500'
-        self.script.MergeManager = Mock()
-        mergemanager = self.script.MergeManager.return_value
+        self.script.Manager = Mock()
+        mergemanager = self.script.Manager.return_value
         mergemanager.do.side_effect = CITrainError('failboat.')
         self.assertEqual(self.script.main(), 0)
         self.assertEqual(self.script.SiloState.requestid.fget.cache, {
@@ -78,7 +78,7 @@
         })
         self.script.SiloState.assert_called_once_with(
             siloname=None, primary=False)
-        self.script.MergeManager.assert_called_once_with(silo_state)
+        self.script.Manager.assert_called_once_with(silo_state)
         self.assertEqual(silo_state.status, 'Abandoned')
         mergemanager.do.assert_called_once_with('nuke')
         silo_state.save_config.assert_called_once_with()

=== modified file 'tests/unit/test_script_build.py'
--- tests/unit/test_script_build.py	2015-09-14 17:52:10 +0000
+++ tests/unit/test_script_build.py	2015-09-24 07:13:28 +0000
@@ -50,29 +50,47 @@
 
     def setUp(self):
         super().setUp()
-        self.script.BuildBase.__bases__ = (fake_factory(),)
         self.script.glob = Mock()
         self.source = Mock()
         self.dest = Mock()
         self.series = Mock()
         self.ppa = Mock()
 
-    def test_buildmanager_do(self):
-        """Run the specified phases."""
-        silo_state = Mock()
-        bman = self.script.BuildManager(silo_state)
-        bman.choose_packages = Mock()
-        bman.choose_classes = Mock()
-        bman.instantiate_build_objects = Mock()
-        bman.execute_phase = Mock()
-        bman.do('solid', 'liquid', 'gas')
-        bman.choose_packages.assert_called_once_with()
-        bman.choose_classes.assert_called_once_with()
-        bman.instantiate_build_objects.assert_called_once_with()
-        self.assertEqual(
-            bman.execute_phase.mock_calls, [
-                call('solid'), call('liquid'), call('gas')
-            ])
+    def test_find_all_uploaded(self):
+        """Find everything that has been uploaded from this silo."""
+        self.script.glob.return_value = [
+            '/path/to/foo_0.1_source.ppa.upload',
+            '/path/to/bar_0.2_source.ppa.upload',
+        ]
+        self.assertEqual(
+            sorted(self.script.find_all_uploaded()), ['bar', 'foo'])
+
+    def test_find_all_uploaded_multiple_series(self):
+        """We can handle uploading multiple series from a single silo."""
+        self.script.glob.return_value = [
+            '/path/to/foo_0.1_source.ppa.upload',
+            '/path/to/foo_0.1~utopic_source.ppa.upload',
+            '/path/to/bar_0.2_source.ppa.upload',
+            '/path/to/bar_0.2~utopic_source.ppa.upload',
+        ]
+        self.assertEqual(
+            sorted(self.script.find_all_uploaded()),
+            ['bar', 'bar', 'foo', 'foo'])
+        self.assertEqual(
+            set(self.script.find_all_uploaded()), set(['foo', 'bar']))
+
+    def test_buildmanager_include_all(self):
+        """Correctly switch to watching all packages."""
+        bman = self.script.BuildManager(Mock())
+        bman.types['foo'] = 'bar'
+        self.assertEqual(
+            bman.choose_packages.__doc__,
+            'Decide which packages need to be built.')
+        bman.include_all()
+        self.assertEqual(
+            bman.choose_packages.__doc__,
+            'Act on all silo packages.')
+        self.assertFalse(bman.types)
 
     def test_buildmanager_choose_packages(self):
         """Determine what packages to act upon."""
@@ -96,107 +114,6 @@
         with self.assertRaisesRegexp(BuildError, 'No packages are being'):
             bman.choose_packages()
         self.assertEqual(bman.names, set())
-        self.assertIsNone(bman.do('ignored'))
-
-    def test_buildmanager_choose_classes(self):
-        """Determine what classes to build with."""
-        silo_state = Mock(
-            all_projects=['a', 'b', 'c'], mps=['a'], sources=['b', 'c'])
-        self.script.env.INCLUDE_BINARIES_IN_SYNC = 'false'
-        bman = self.script.BuildManager(silo_state)
-        bman.names = set(['a', 'b'])
-        bman.choose_classes()
-        self.assertEqual(
-            bman.types, {
-                self.script.Merge: set(['a']),
-                self.script.SourceSync: set(['b']),
-            })
-        self.assertEqual(self.script.Merge.all_mps, silo_state.mps)
-        self.assertEqual(
-            self.script.SourceSync.from_series,
-            silo_state.source_series)
-
-    def test_buildmanager_instantiate_build_objects(self):
-        """Create all build objects."""
-        silo_state = Mock(
-            all_projects=['a', 'b', 'c'], mps=['a'], sources=['b', 'c'],
-            dual=None)
-        self.script.lp.load = lambda x: x
-        merge = Mock()
-        source = Mock()
-        bman = self.script.BuildManager(silo_state)
-        bman.types = {
-            merge: set('a'),
-            source: set('b'),
-        }
-        bman.instantiate_build_objects()
-        self.assertEqual(
-            bman.builds, {
-                'a': merge.return_value,
-                'b': source.return_value,
-            })
-        merge.assert_called_once_with(
-            'a', silo_state.series, silo_state.dest, silo_state.ppa)
-        source.assert_called_once_with(
-            'b', silo_state.series, silo_state.dest, silo_state.ppa)
-
-    def test_buildmanager_instantiate_build_objects_secondary(self):
-        """Create Secondary build objects."""
-        silo_state = Mock(
-            all_projects=['a', 'b', 'c'], mps=['a'], sources=['b', 'c'],
-            dual='vivid')
-        self.script.lp.load = lambda x: x
-        merge = Mock()
-        source = Mock()
-        bman = self.script.BuildManager(silo_state)
-        bman.types = {
-            merge: set('a'),
-            source: set('b'),
-        }
-        self.script.Secondary = Mock()
-        bman.instantiate_build_objects()
-        self.assertEqual(
-            bman.builds, {
-                'a': merge.return_value,
-                'b': source.return_value,
-                'zzz_a': self.script.Secondary.return_value,
-                'zzz_b': self.script.Secondary.return_value,
-            })
-
-    def test_buildmanager_execute_phase(self):
-        """Execute one phase for all builds."""
-        silo_state = Mock()
-        self.script.BuildBase = Mock()
-        self.script.BuildBase.failures = set()
-        bman = self.script.BuildManager(silo_state)
-        merge = Mock()
-        sync = Mock()
-        bman.builds = {'a': merge, 'b': sync}
-        bman.execute_phase('liquid')
-        self.script.BuildBase.pre_liquid_phase.assert_called_once_with(
-            silo_state)
-        merge.liquid_phase.assert_called_once_with()
-        sync.liquid_phase.assert_called_once_with()
-        self.script.BuildBase.post_liquid_phase.assert_called_once_with(
-            silo_state)
-
-    def test_buildmanager_execute_phase_failures(self):
-        """Raise error when phase sets a failure."""
-        silo_state = Mock()
-        self.script.BuildBase = Mock()
-        self.script.BuildBase.failures = {'something broke', 'foo'}
-        bman = self.script.BuildManager(silo_state)
-        merge = Mock()
-        sync = Mock()
-        bman.builds = {'a': merge, 'b': sync}
-        with self.assertRaisesRegexp(BuildError, 'foo. something broke'):
-            bman.execute_phase('liquid')
-        self.script.BuildBase.pre_liquid_phase.assert_called_once_with(
-            silo_state)
-        merge.liquid_phase.assert_called_once_with()
-        sync.liquid_phase.assert_called_once_with()
-        self.assertEqual(
-            self.script.BuildBase.post_liquid_phase.mock_calls, [])
 
     def test_main(self):
         """Run things without issue."""
@@ -207,9 +124,12 @@
         bman.assert_called_once_with(silo_state)
         self.assertEqual(
             bman.return_value.mock_calls, [
-                call.do('validate', 'clean', 'collect',
-                        'build', 'upload', 'watch', 'diff'),
+                call.do('validate'),
+                call.do('clean', 'collect', 'build', 'upload'),
+                call.include_all(),
+                call.do('watch', 'diff'),
             ])
+        self.assertEqual(silo_state.status, 'Packages built.')
         silo_state.save_config.assert_called_once_with()
 
     def test_main_watch_only(self):
@@ -221,7 +141,9 @@
         bman.assert_called_once_with(silo_state)
         self.assertEqual(
             bman.return_value.mock_calls, [
-                call.do('validate', 'watch', 'diff'),
+                call.do('validate'),
+                call.include_all(),
+                call.do('watch', 'diff'),
             ])
         silo_state.save_config.assert_called_once_with()
 
@@ -239,8 +161,7 @@
         bman.assert_called_once_with(silo_state)
         self.assertEqual(
             bman.return_value.mock_calls, [
-                call.do('validate', 'clean', 'collect',
-                        'build', 'upload', 'watch', 'diff'),
+                call.do('validate'),
             ])
         self.assertEqual(silo_state.status, 'Build failed: It asploded!')
         silo_state.save_config.assert_called_once_with()

=== modified file 'tests/unit/test_script_merge_clean.py'
--- tests/unit/test_script_merge_clean.py	2015-09-22 07:27:29 +0000
+++ tests/unit/test_script_merge_clean.py	2015-09-24 07:13:28 +0000
@@ -31,21 +31,14 @@
     """Test CI Train Merge & Clean script."""
     scriptname = 'merge_clean.py'
 
-    def test_mergemanager_choose_packages(self):
-        """MergeManager should choose all packages."""
-        silo_state = Mock(all_projects=list('foobar'))
-        mergemanager = self.script.MergeManager(silo_state)
-        mergemanager.choose_packages()
-        self.assertEqual(mergemanager.names, set(['f', 'o', 'b', 'a', 'r']))
-
     def test_main(self):
         """main() succeeds in the primary case."""
         self.script.env.SILONAME = 'fred'
-        self.script.MergeManager = Mock()
-        mergemanager = self.script.MergeManager.return_value
+        self.script.Manager = Mock()
+        mergemanager = self.script.Manager.return_value
         self.assertEqual(self.script.main(), 0)
         self.script.SiloState.assert_called_once_with('fred', primary=True)
-        self.script.MergeManager.assert_called_once_with(
+        self.script.Manager.assert_called_once_with(
             self.script.SiloState.return_value)
         mergemanager.do.assert_called_once_with(
             'validate', 'enumeration', 'merge', 'push', 'nuke')
@@ -53,13 +46,13 @@
     def test_main_failed(self):
         """main() returns 1 when a phase raises an exception."""
         self.script.env.SILONAME = 'fred'
-        self.script.MergeManager = Mock()
-        mergemanager = self.script.MergeManager.return_value
+        self.script.Manager = Mock()
+        mergemanager = self.script.Manager.return_value
         mergemanager.do.side_effect = MergeError('whoa buddy!')
         silo_state = self.script.SiloState.return_value
         self.assertEqual(self.script.main(), 1)
         self.script.SiloState.assert_called_once_with('fred', primary=True)
-        self.script.MergeManager.assert_called_once_with(
+        self.script.Manager.assert_called_once_with(
             self.script.SiloState.return_value)
         mergemanager.do.assert_called_once_with(
             'validate', 'enumeration', 'merge', 'push', 'nuke')

=== modified file 'tests/unit/test_script_migration.py'
--- tests/unit/test_script_migration.py	2015-09-11 00:25:12 +0000
+++ tests/unit/test_script_migration.py	2015-09-24 07:13:28 +0000
@@ -35,7 +35,7 @@
 
     def setUp(self):
         super().setUp()
-        self.script.MergeManager = Mock()
+        self.script.Manager = Mock()
         self.script.merge_main = Mock()
         self.script.glob.return_value = [
             join(SILOS_DIR, 'ubuntu', 'landing-00{}'.format(x))
@@ -48,7 +48,7 @@
         self.assertEqual(self.script.main(), 0)
         for state in states:
             self.assertEqual(state.mock_calls, [])
-        self.assertEqual(self.script.MergeManager.mock_calls, [
+        self.assertEqual(self.script.Manager.mock_calls, [
             call(states[0]),
             call().do('migration'),
             call(states[1]),
@@ -62,7 +62,7 @@
         """Don't check silos that are empty."""
         self.script.SiloState.iterate.return_value = []
         self.assertEqual(self.script.main(), 0)
-        self.assertEqual(self.script.MergeManager.mock_calls, [])
+        self.assertEqual(self.script.Manager.mock_calls, [])
         self.assertEqual(self.script.merge_main.mock_calls, [])
 
     def test_main_skip_unpublished_silos(self):
@@ -70,7 +70,7 @@
         states = self.script.SiloState.iterate.return_value = [
             Mock(is_published=False) for rid in range(3)]
         self.assertEqual(self.script.main(), 0)
-        self.assertEqual(self.script.MergeManager.mock_calls, [])
+        self.assertEqual(self.script.Manager.mock_calls, [])
         self.assertEqual(self.script.merge_main.mock_calls, [])
         for state in states:
             self.assertEqual(state.set_migrating.mock_calls, [])
@@ -80,7 +80,7 @@
         """Don't merge silos that are still migrating."""
         states = self.script.SiloState.iterate.return_value = [
             Mock(requestid=str(rid)) for rid in range(3)]
-        self.script.MergeManager.return_value.do.side_effect = MigrationError(
+        self.script.Manager.return_value.do.side_effect = MigrationError(
             'foo is in the Proposed pocket.')
         self.assertEqual(self.script.main(), 0)
         for state in states:
@@ -89,7 +89,7 @@
             self.assertEqual(state.mock_calls, [
                 call.save_config(),
             ])
-        self.assertEqual(self.script.MergeManager.mock_calls, [
+        self.assertEqual(self.script.Manager.mock_calls, [
             call(states[0]),
             call().do('migration'),
             call(states[1]),

=== modified file 'tests/unit/test_script_publisher.py'
--- tests/unit/test_script_publisher.py	2015-09-22 18:51:46 +0000
+++ tests/unit/test_script_publisher.py	2015-09-24 07:13:28 +0000
@@ -413,7 +413,7 @@
 
     def mock_main(self):
         """Ensure enough of the publisher is mocked out for testing main()."""
-        self.script.MergeManager = Mock()
+        self.script.Manager = Mock()
         self.script.get_distro_and_series = Mock()
         self.script.check_versions_at_destination = Mock()
         self.script.check_for_unapproved = Mock()
@@ -430,8 +430,8 @@
         dest = state.dest
         ppa = state.ppa
         self.assertEqual(self.script.main(), 0)
-        self.script.MergeManager.assert_called_once_with(state)
-        self.script.MergeManager.return_value.do.assert_called_once_with(
+        self.script.Manager.assert_called_once_with(state)
+        self.script.Manager.return_value.do.assert_called_once_with(
             'checkstatus', 'validate', 'diff', 'checkupload', 'ackaging')
         self.assertEqual(state.status, 'Publishing.')
         self.script.check_versions_at_destination.assert_called_once_with(
@@ -468,7 +468,7 @@
         self.mock_main()
         state = self.script.SiloState()
         state.status = 'Preserved'
-        self.script.MergeManager.return_value.do = Mock(
+        self.script.Manager.return_value.do = Mock(
             side_effect=NoStatusError('Whoops'))
         self.assertEqual(self.script.main(), 1)
         self.assertEqual(state.status, 'Preserved')

=== modified file 'tests/unit/test_utils.py'
--- tests/unit/test_utils.py	2015-09-22 06:53:49 +0000
+++ tests/unit/test_utils.py	2015-09-24 07:13:28 +0000
@@ -25,7 +25,6 @@
     SILO_DIR,
     call,
     env,
-    find_all_uploaded,
     log_call,
     name_ppa,
     run_script,
@@ -48,28 +47,6 @@
         ppa.name = 'foo-ppa'
         self.assertEqual(name_ppa(ppa), 'foo-team/ubuntu/foo-ppa')
 
-    @patch('cupstream2distro.utils.glob')
-    def test_find_all_uploaded(self, glob_mock):
-        """Find everything that has been uploaded from this silo."""
-        glob_mock.return_value = [
-            '/path/to/foo_0.1_source.ppa.upload',
-            '/path/to/bar_0.2_source.ppa.upload',
-        ]
-        self.assertEqual(sorted(find_all_uploaded()), ['bar', 'foo'])
-
-    @patch('cupstream2distro.utils.glob')
-    def test_find_all_uploaded_multiple_series(self, glob_mock):
-        """We can handle uploading multiple series from a single silo."""
-        glob_mock.return_value = [
-            '/path/to/foo_0.1_source.ppa.upload',
-            '/path/to/foo_0.1~utopic_source.ppa.upload',
-            '/path/to/bar_0.2_source.ppa.upload',
-            '/path/to/bar_0.2~utopic_source.ppa.upload',
-        ]
-        self.assertEqual(sorted(find_all_uploaded()),
-                         ['bar', 'bar', 'foo', 'foo'])
-        self.assertEqual(set(find_all_uploaded()), set(['foo', 'bar']))
-
     @patch('cupstream2distro.utils.logging')
     def test_env_init(self, log_mock):
         """Log environment variables when debugging enabled."""