Merge lp:~robru/cupstream2distro/drop-merge-checks into lp:cupstream2distro

[Robert Bruce Park]

Drop redundant checks from merge job.

Now that the merge job can no longer be triggered by landers, it is only triggered automatically by the train, or manually by trainguards who want to force it. This means that the non-force option and all safety checks are redundant: migration job only triggers it after checking that merges are in good states and packages are at destination, so there's no point for the merge job to check that stuff a second time before proceeding with the merge.

If a trainguard is triggering it manually, it's *because* they want to force it, so there's no need to check this stuff and prevent them from merging.

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

=== modified file 'citrain/jenkins-templates/merge-clean.xml.tmpl'
--- citrain/jenkins-templates/merge-clean.xml.tmpl	2015-11-04 21:05:27 +0000
+++ citrain/jenkins-templates/merge-clean.xml.tmpl	2015-11-20 06:37:47 +0000
@@ -24,11 +24,6 @@
     <hudson.model.ParametersDefinitionProperty>
       <parameterDefinitions>
         <hudson.model.BooleanParameterDefinition>
-          <name>FORCE</name>
-          <description>Proceed with merging even if versions not at destination.</description>
-          <defaultValue>false</defaultValue>
-        </hudson.model.BooleanParameterDefinition>
-        <hudson.model.BooleanParameterDefinition>
           <name>DEBUG</name>
           <description>Debug mode</description>
           <defaultValue>false</defaultValue>

=== modified file 'citrain/merge_clean.py'
--- citrain/merge_clean.py	2015-11-17 18:30:50 +0000
+++ citrain/merge_clean.py	2015-11-20 06:37:47 +0000
@@ -21,9 +21,6 @@
 
 Environment variables:
 
-FORCE
-    Proceed with merge even if version is not at destination.
-
 SILONAME
     Determines which silo to merge & clean.
 
@@ -42,8 +39,9 @@
 
 def merge(silo_state):
     """Do the steps specific to merging & cleaning."""
-    manager = Manager(silo_state)
-    manager.do('validate', 'enumeration', 'merge', 'push', 'nuke')
+    silo_state.status = 'Merging'
+    silo_state.save_config()
+    Manager(silo_state).do('merge', 'push', 'nuke')
     silo_state.status = 'Landed'
 
 

=== modified file 'citrain/recipes/base.py'
--- citrain/recipes/base.py	2015-11-20 05:43:41 +0000
+++ citrain/recipes/base.py	2015-11-20 06:37:47 +0000
@@ -572,27 +572,6 @@
         if cls.packagelist.lines:
             cls.packagelist.write()
 
-    def enumeration_phase(self):
-        """Halt merge&clean job if packages not in destination.
-
-        :raises MergeError: If PPA versions don't match destination archive.
-        """
-        msg = 'Checking that %s has reached destination archive...'
-        logging.info(msg, self.name)
-        ours = self.get_archive_version(self.silo_ppa)
-        dest = self.get_archive_version(self.dest)
-        log_value_of.ours('Silo PPA version')
-        log_value_of.dest('Archive version')
-        if ours != dest and env.FORCE != 'true':
-            msg = '{} mismatch: PPA has {}, destination archive has {}.'
-            self.failures.add(msg.format(self.name, ours, dest))
-
-    @staticmethod
-    def post_enumeration_phase(silo_state):
-        """Indicate that merge is proceeding in dashboard status. """
-        silo_state.status = 'Merging.'
-        silo_state.save_config()
-
     @staticmethod
     def post_nuke_phase(silo_state):
         """Delete all packages from PPA and remove local silo dir."""

=== modified file 'tests/unit/test_recipe_base.py'
--- tests/unit/test_recipe_base.py	2015-11-20 05:43:41 +0000
+++ tests/unit/test_recipe_base.py	2015-11-20 06:37:47 +0000
@@ -875,39 +875,6 @@
         self.assertEqual(silo_state.mock_calls, [])
         BuildBase.packagelist.write.assert_called_once_with()  # ONCE
 
-    def test_buildbase_enumeration_phase(self):
-        """Don't raise an exception when versions match."""
-        build = BuildBase('foo', self.series, self.dest, self.ppa)
-        build.get_archive_version = Mock(return_value='1.0')
-        build.enumeration_phase()
-        self.assertEqual(
-            build.get_archive_version.mock_calls, [
-                call(self.ppa), call(self.dest),
-            ])
-
-    def test_buildbase_enumeration_phase_failed(self):
-        """Raise MergeError when ppa and dest versions don't match."""
-        versions = ['1+15.04.20150226-0ubuntu1', '1+15.04.20150226-0ubuntu2']
-        build = BuildBase('foo', self.series, self.dest, self.ppa)
-        build.get_archive_version = Mock(
-            side_effect=lambda *a, **kw: versions.pop())
-        build.enumeration_phase()
-        self.assertEqual(build.failures, set([
-            'foo mismatch: PPA has 1+15.04.20150226-0ubuntu2, destination '
-            'archive has 1+15.04.20150226-0ubuntu1.',
-        ]))
-        self.assertEqual(
-            build.get_archive_version.mock_calls, [
-                call(self.ppa), call(self.dest),
-            ])
-
-    def test_buildbase_post_enumeration_phase(self):
-        """Set the silo as merging after enumeration has completed."""
-        silo_state = Mock()
-        BuildBase.post_enumeration_phase(silo_state)
-        self.assertEqual(silo_state.status, 'Merging.')
-        silo_state.save_config.assert_called_once_with()
-
     @patch('citrain.recipes.base.mkdir')
     @patch('citrain.recipes.base.shutil')
     @patch('citrain.recipes.base.SILO_DIR')

=== modified file 'tests/unit/test_script_merge_clean.py'
--- tests/unit/test_script_merge_clean.py	2015-11-17 18:30:50 +0000
+++ tests/unit/test_script_merge_clean.py	2015-11-20 06:37:47 +0000
@@ -16,7 +16,7 @@
 
 """Tests for CI Train Merge & Clean script."""
 
-from mock import Mock
+from mock import Mock, call
 
 from tests.unit import CITrainScriptTestCase
 
@@ -38,9 +38,11 @@
         mergemanager = self.script.Manager.return_value
         self.assertIsNone(self.script.merge(silo_state))
         self.script.Manager.assert_called_once_with(silo_state)
-        mergemanager.do.assert_called_once_with(
-            'validate', 'enumeration', 'merge', 'push', 'nuke')
+        mergemanager.do.assert_called_once_with('merge', 'push', 'nuke')
         self.assertEqual(silo_state.status, 'Landed')
+        self.assertEqual(silo_state.mock_calls, [
+            call.save_config(),
+        ])
 
     def test_merge_failed(self):
         """merge() doesn't catch exceptions."""
@@ -52,6 +54,8 @@
         with self.assertRaisesRegexp(MergeError, 'Merge&Clean failed: whoa'):
             self.script.merge(silo_state)
         self.script.Manager.assert_called_once_with(silo_state)
-        mergemanager.do.assert_called_once_with(
-            'validate', 'enumeration', 'merge', 'push', 'nuke')
+        mergemanager.do.assert_called_once_with('merge', 'push', 'nuke')
         assert silo_state.status != 'Landed'
+        self.assertEqual(silo_state.mock_calls, [
+            call.save_config(),
+        ])