Merge lp:~robru/cupstream2distro/prevent-publishing-failures into lp:cupstream2distro

[Robert Bruce Park]

Stop publication if silo has bad status.

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[lp:~robru/cupstream2distro/prevent-publishing-failures]

1109. By Robert Bruce Park on 2015-09-22

Preserve status since overwriting failed status is self-defeating.

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Robert Bruce Park (robru)]

Looking good in staging.

review: Approve

=== modified file 'citrain/build.py'
--- citrain/build.py	2015-09-14 06:52:53 +0000
+++ citrain/build.py	2015-09-22 18:51:59 +0000
@@ -166,6 +166,7 @@
     phases += ['watch', 'diff']
     try:
         buildmanager.do(*phases)
+        silo_state.status = 'Packages built.'
     except CITrainError as err:
         logging.error(str(err))
         silo_state.status = 'Build failed: ' + str(err)

=== modified file 'citrain/publisher.py'
--- citrain/publisher.py	2015-09-11 00:25:12 +0000
+++ citrain/publisher.py	2015-09-22 18:51:59 +0000
@@ -48,7 +48,7 @@
 from cupstream2distro.archive import Archive
 from cupstream2distro.branchhandling import Branch
 from cupstream2distro.packagelist import PackageList
-from cupstream2distro.errors import CITrainError, PublishError
+from cupstream2distro.errors import CITrainError, PublishError, NoStatusError
 from cupstream2distro.silomanager import SiloState
 from cupstream2distro.launchpadmanager import lp
 from cupstream2distro.project import DotProject
@@ -206,14 +206,12 @@
 
 def main():
     """Conduct a beautiful orchestra of publication."""
-    silo_state = SiloState(env.SILONAME, primary=True)
-    mergemanager = MergeManager(silo_state)
     try:
+        silo_state = SiloState(env.SILONAME, primary=True)
         dest = silo_state.dest
         ppa = silo_state.ppa
-        silo_state.status = 'Attempting publication.'
-        silo_state.save_config()
-        mergemanager.do('validate', 'diff', 'checkupload', 'ackaging')
+        MergeManager(silo_state).do(
+            'checkstatus', 'validate', 'diff', 'checkupload', 'ackaging')
         get_distro_and_series(silo_state.series)
         silo_state.status = 'Publishing.'
         silo_state.save_config()
@@ -226,7 +224,8 @@
         silo_state.mark_others_dirty()
     except CITrainError as err:
         logging.error(str(err))
-        silo_state.status = "Can't publish: " + str(err)
+        if type(err) is not NoStatusError:
+            silo_state.status = "Can't publish: " + str(err)
         return 1
     finally:
         silo_state.save_config()

=== modified file 'citrain/recipes/base.py'
--- citrain/recipes/base.py	2015-09-22 00:58:14 +0000
+++ citrain/recipes/base.py	2015-09-22 18:51:59 +0000
@@ -41,6 +41,7 @@
     BuildError,
     PublishError,
     MigrationError,
+    NoStatusError,
 )
 from cupstream2distro.settings import (
     DEPWAIT_TIMEOUT,
@@ -88,6 +89,7 @@
 TWINS.update({v: k for k, v in TWINS.items()})
 
 
+DO_NOT_PUBLISH = {'failed', 'error', 'dirty'}
 BUILD_FAILURES = (
     'Build for superseded Source',
     'Cancelled build',
@@ -295,10 +297,13 @@
         """Check that no builds failed.
 
         :param silo_state: The SiloState instance representing the silo.
-        :raises BuildError: If any builds in the PPA fail.
         """
         silo_state.set_built_checked()
-        silo_state.status = 'Packages built.'
+
+    @staticmethod
+    def pre_diff_phase(silo_state):
+        """Announce that we are generating diffs since that's a bit slow."""
+        silo_state.status = 'Generating diffs.'
         silo_state.save_config()
 
     def diff_phase(self):
@@ -314,6 +319,12 @@
                 for new in glob(SILO_DIR(dsc)):
                     self.generate_package_diffs(old, new)
 
+    @staticmethod
+    def post_checkstatus_phase(silo_state):
+        """Prevent publication if silo in bad state."""
+        if DO_NOT_PUBLISH & silo_state.tokenize():
+            raise NoStatusError('Silo has bad status.')
+
     def checkupload_phase(self):
         """Confirm that the silo publisher has upload rights."""
         sourcepub = self.main_archive.getPublishedSources(

=== modified file 'cupstream2distro/errors.py'
--- cupstream2distro/errors.py	2015-06-23 20:07:17 +0000
+++ cupstream2distro/errors.py	2015-09-22 18:51:59 +0000
@@ -65,3 +65,8 @@
 class RevertError(CITrainError):
     """Exception raised when there's a problem with reverting."""
     pass
+
+
+class NoStatusError(PublishError):
+    """Special Exception that when raised won't set status."""
+    pass

=== modified file 'cupstream2distro/silomanager.py'
--- cupstream2distro/silomanager.py	2015-09-21 23:19:20 +0000
+++ cupstream2distro/silomanager.py	2015-09-22 18:51:59 +0000
@@ -61,6 +61,7 @@
 
 
 TOKEN_SCRUB = re.compile('token=[a-z0-9-]+', flags=re.IGNORECASE)
+WORD = re.compile('\W+')
 
 
 PPA_DESCRIPTION = """\
@@ -318,6 +319,11 @@
                 raise PrepError(
                     'Sources and syncs unsupported for dual landings.')
 
+    def tokenize(self):
+        """Return a set of words in status & signoff fields."""
+        return set(
+            WORD.split('{} {}'.format(self.status, self.qa_signoff).lower()))
+
     def summarize(self):
         """Return a handy summary."""
         joburl = '{}job/{}-{{}}/build'.format(

=== modified file 'tests/unit/test_recipe_base.py'
--- tests/unit/test_recipe_base.py	2015-09-22 06:53:49 +0000
+++ tests/unit/test_recipe_base.py	2015-09-22 18:51:59 +0000
@@ -369,7 +369,12 @@
         silo_state = Mock()
         BuildBase.post_watch_phase(silo_state)
         silo_state.set_built_checked.assert_called_once_with()
-        self.assertEqual(silo_state.status, 'Packages built.')
+
+    def test_buildbase_pre_diff_phase(self):
+        """Announce that diffing is beginning."""
+        silo_state = Mock()
+        BuildBase.pre_diff_phase(silo_state)
+        self.assertEqual(silo_state.status, 'Generating diffs.')
         silo_state.save_config.assert_called_once_with()
 
     @patch('citrain.recipes.base.glob')
@@ -430,6 +435,20 @@
             status='Published')
         build.generate_package_diffs.assert_called_once_with(None, 'new_dsc')
 
+    def test_buildbase_post_checkstatus_phase(self):
+        """Silently allow publication when no problems found."""
+        silo_state = Mock()
+        silo_state.tokenize.return_value = {'everything', 'is', 'awesome'}
+        BuildBase.post_checkstatus_phase(silo_state)
+
+    def test_buildbase_post_checkstatus_phase_fail(self):
+        """Ensure that publish is halted if silo in bad state."""
+        silo_state = Mock()
+        for problem in ('failed', 'dirty', 'error'):
+            silo_state.tokenize.return_value = {'everything', 'is', problem}
+            with self.assertRaisesRegexp(PublishError, 'Silo has bad status'):
+                BuildBase.post_checkstatus_phase(silo_state)
+
     @patch('citrain.recipes.base.lp')
     def test_buildbase_checkupload_phase(self, lp_mock):
         """Ensure that we call checkupload correctly."""

=== modified file 'tests/unit/test_script_publisher.py'
--- tests/unit/test_script_publisher.py	2015-09-11 00:25:12 +0000
+++ tests/unit/test_script_publisher.py	2015-09-22 18:51:59 +0000
@@ -23,7 +23,7 @@
 from tests.unit import CITrainScriptTestCase
 
 from cupstream2distro.utils import env
-from cupstream2distro.errors import PublishError, BranchError
+from cupstream2distro.errors import PublishError, BranchError, NoStatusError
 
 
 N = '\n'
@@ -432,7 +432,7 @@
         self.assertEqual(self.script.main(), 0)
         self.script.MergeManager.assert_called_once_with(state)
         self.script.MergeManager.return_value.do.assert_called_once_with(
-            'validate', 'diff', 'checkupload', 'ackaging')
+            'checkstatus', 'validate', 'diff', 'checkupload', 'ackaging')
         self.assertEqual(state.status, 'Publishing.')
         self.script.check_versions_at_destination.assert_called_once_with(
             dest, state.all_projects)
@@ -442,8 +442,7 @@
         self.script.initiate_publication.assert_called_once_with(
             dest, ppa, state.set_package_version_list)
         state.set_published.assert_called_once_with()
-        self.assertEqual(state.save_config.mock_calls, [
-            call(), call(), call()])
+        self.assertEqual(state.save_config.mock_calls, [call(), call()])
         state.mark_others_dirty.assert_called_once_with()
 
     def test_main_failure(self):
@@ -461,6 +460,15 @@
         self.script.push_branches.assert_called_once_with(state.mps)
         self.assertEqual(self.script.initiate_publication.call_count, 0)
         self.script.logging.error.assert_called_once_with('Whoops')
-        self.assertEqual(state.save_config.mock_calls, [
-            call(), call(), call()])
+        self.assertEqual(state.save_config.mock_calls, [call(), call()])
         self.assertEqual(state.mark_others_dirty.mock_calls, [])
+
+    def test_main_failure_nostatus(self):
+        """publisher.main() shouldn't set status inappropriately."""
+        self.mock_main()
+        state = self.script.SiloState()
+        state.status = 'Preserved'
+        self.script.MergeManager.return_value.do = Mock(
+            side_effect=NoStatusError('Whoops'))
+        self.assertEqual(self.script.main(), 1)
+        self.assertEqual(state.status, 'Preserved')

=== modified file 'tests/unit/test_silomanager.py'
--- tests/unit/test_silomanager.py	2015-09-21 22:32:57 +0000
+++ tests/unit/test_silomanager.py	2015-09-22 18:51:59 +0000
@@ -698,6 +698,15 @@
         ])
         branch_mock.get_package_name_from_branch.assert_called_with('lp:foo')
 
+    def test_silomanager_tokenize(self):
+        """Convert silo statuses into tokens sensibly."""
+        self.state._bileto.update(
+            status='Hey, you: (job 29) FAILED: broken//#$%^&*',
+            qa_signoff='Really bad')
+        self.assertEqual(self.state.tokenize(), {
+            'hey', 'you', 'job', '29', 'failed', 'broken', 'really', 'bad',
+        })
+
     def test_signal_handler(self):
         """Our signal handler is installed globally."""
         self.assertEqual(