Merge lp:~cprov/britney/boottest-double-blocking into lp:~ubuntu-release/britney/britney2-ubuntu

[Celso Providelo]

Fixing BootTest criteria to stop doubling-blocking package promotion, as reported in:

https://trello.com/c/SxBZD8kN/37-1-boottest-get-a-review-for-britney-boottest-testing-criteria-shared-branch

If an source was already blocked by any previous criteria (autopkgtest) it does not need to be blocked again by BootTest (although boottest job and status will continue to be updated).

[lp:~cprov/britney/boottest-double-blocking]

432. By Celso Providelo on 2015-02-20

Test ADT & BootTest criteria running simultaneously. Fixed minor issue related to doubling-blocking promotions.

433. By Celso Providelo on 2015-02-20

Only enable BOOTTEST for testing, so britney.conf is safe on trunk.

[Vincent Ladeuil (vila)]

One more test with adt passing and autopkgtest running should be even closer to the issue we encounter in production iiuc.

[Vincent Ladeuil (vila)]

Bah, hit the button before voting.

I would be more comfortable with the added test but I think the existing one is already providing us with a way to reproduce issues in that area anyway.

review: Approve

[Celso Providelo (cprov)]

Vincent,

Thanks for the review. We can add more tests exercising the production environment (adt + boottest), perhaps operating only on the happy-path. However in order to reproduce the double-blocking error adt and boottest have to block the promotion in question.

[Vincent Ladeuil (vila)]

> However in order to reproduce the double-blocking error adt and boottest have
> to block the promotion in question.

OIC ! Thanks for the clarification I misunderstood the meaning of the tests indeed.

[Colin Watson (cjwatson)]

I think you should still add the boottest excuse; so move excuse.addreason("boottest") outside the excuse.is_valid test, but still within the status not in BootTest.VALID_STATUSES test.

review: Needs Fixing

[lp:~cprov/britney/boottest-double-blocking]

434. By Celso Providelo on 2015-02-20

Add 'boottest' excuse failure reason when it has failed even if the previous tests had already failed.

[Celso Providelo (cprov)]

Colin,

It make sense, fix added in r434. Thanks.

[Colin Watson (cjwatson)]

review: Approve

=== modified file 'britney.py'
--- britney.py	2015-02-10 20:56:35 +0000
+++ britney.py	2015-02-20 16:02:19 +0000
@@ -1973,14 +1973,16 @@
                         "%s" % (excuse.name, excuse.ver[1],
                                 forces[0].user))
                     continue
-                # Block promotion if any boottests attempt has failed or
-                # still in progress.
+                # Block promotion if the excuse is still valid (adt tests
+                # passed) but the boottests attempt has failed or still in
+                # progress.
                 if status not in BootTest.VALID_STATUSES:
-                    excuse.addhtml("Not considered")
                     excuse.addreason("boottest")
-                    excuse.is_valid = False
-                    upgrade_me.remove(excuse.name)
-                    unconsidered.append(excuse.name)
+                    if excuse.is_valid:
+                        excuse.is_valid = False
+                        excuse.addhtml("Not considered")
+                        upgrade_me.remove(excuse.name)
+                        unconsidered.append(excuse.name)
 
         # invalidate impossible excuses
         for e in self.excuses:
 
=== modified file 'tests/test_boottest.py'
--- tests/test_boottest.py	2015-02-11 15:41:43 +0000
+++ tests/test_boottest.py	2015-02-20 16:02:19 +0000
@@ -129,6 +129,9 @@
         # Disable autopkgtests.
         new_config = original_config.replace(
             'ADT_ENABLE        = yes', 'ADT_ENABLE        = no')
+        # Enable boottest.
+        new_config = new_config.replace(
+            'BOOTTEST_ENABLE   = no', 'BOOTTEST_ENABLE   = yes')
         # Disable TouchManifest auto-fetching.
         new_config = new_config.replace(
             'BOOTTEST_FETCH    = yes', 'BOOTTEST_FETCH    = no')
@@ -151,7 +154,8 @@
         self.create_manifest([
             'green 1.0',
             'pyqt5:armhf 1.0',
-            'signon 1.0'
+            'signon 1.0',
+            'purple 1.1',
         ])
 
     def create_manifest(self, lines):
@@ -165,7 +169,8 @@
         """Create a stub version of boottest-britney script."""
         script_path = os.path.expanduser(
             "~/auto-package-testing/jenkins/boottest-britney")
-        os.makedirs(os.path.dirname(script_path))
+        if not os.path.exists(os.path.dirname(script_path)):
+            os.makedirs(os.path.dirname(script_path))
         with open(script_path, 'w') as f:
             f.write('''#!%(py)s
 import argparse
@@ -178,6 +183,7 @@
 pyqt5-src 1.1~beta PASS
 pyqt5-src 1.1 FAIL
 signon 1.1 PASS
+purple 1.1 RUNNING
 """
 
 def request():
@@ -405,6 +411,43 @@
              r'<li>missing build on .*>armhf</a>: pyqt5 \(from .*>1</a>\)',
              '<li>Not considered'])
 
+    def test_with_adt(self):
+        # Boottest can run simultaneously with autopkgtest (adt).
+
+        # Enable ADT in britney configuration.
+        with open(self.britney_conf, 'r') as fp:
+            original_config = fp.read()
+        new_config = original_config.replace(
+            'ADT_ENABLE        = no', 'ADT_ENABLE        = yes')
+        with open(self.britney_conf, 'w') as fp:
+            fp.write(new_config)
+
+        # Create a fake 'adt-britney' that reports a RUNNING job for
+        # the testing source ('purple_1.1').
+        script_path = os.path.expanduser(
+            "~/auto-package-testing/jenkins/adt-britney")
+        os.makedirs(os.path.dirname(script_path))
+        with open(script_path, 'w') as f:
+            f.write('''#!/bin/sh -e
+mkdir -p ~/proposed-migration/autopkgtest/work
+touch ~/proposed-migration/autopkgtest/work/adt.request.series
+echo "purple 1.1 RUNNING purple 1.1" >> ~/proposed-migration/autopkgtest/work/adt.result.series''')
+        os.chmod(script_path, 0o755)
+
+        # Britney blocks testing source promotion while ADT and boottest
+        # are running.
+        self.data.add('purple', False, {'Version': '1.0'})
+        context = [
+            ('purple', {'Version': '1.1'}),
+        ]
+        self.do_test(
+            context,
+            [r'\bpurple\b.*>1.0<.* to .*>1.1<',
+             '<li>autopkgtest for purple 1.1: {}'.format(
+                 boottest.BootTest.EXCUSE_LABELS['RUNNING']),
+             '<li>Boottest result: {}'.format(
+                 boottest.BootTest.EXCUSE_LABELS['RUNNING']),
+             '<li>Not considered'])
 
 
 if __name__ == '__main__':