Merge lp:~jibel/britney/fix_missing_results into lp:~ubuntu-release/britney/britney2-ubuntu

[Jean-Baptiste Lallement]

This branch fix a case of "vanishing" results in excuses.

Only the last result in history for a given package was considered, results being sorted in alphabetic order, a failed test could be hidden by another test that passed, and the package promoted while it should have been blocked.

For example if we had the following result in history:
udisk2 1 PASS udisks2 1

An upload of systemd will trigger a test of udisk2, if we suppose that this test fails, it will generate the following list of results:
udisk2 1 FAIL systemd 2
udisk2 1 PASS udisk2 1

Which means that udisk2 can be promoted because it pass, and systemd must be blocked because it introduced a regression in udisk2.
With the bug only the last cause in the list is considered and the result for udisk2/systemd is ignored.

This MP also:
- Adds tests to cover the bug mentioned above, and update existing tests for the new states "always fails/regression"
- makes the distinction between tests that are always failing and consider them as valid candidates and tests that regress, in which case the package is block.
- Add colouring to make excuses more readable for long list of autopkgtests
- Points jenkins URL directly lastBuild to save some clicks

[Martin Pitt (pitti)]

Many thanks for this, Jean-Baptiste! Some comments:

 * My original tests (from https://code.launchpad.net/~canonical-platform-qa/britney/tests/+merge/207982) are not merged into britney2-ubuntu yet. I don't know whether this was due to lack of time, or the release team would prefer the tests to have a different form. In the latter case, I suggest merging this without the changes to tests/autopkgtest.py, and we apply the latter on top of my MP above. We shouldn't block this urgent fix on desinging a new test suite, and the existing one at least shows that it's working now.

 * Thanks for introducing the "always fails" state. This introduces the additional requirement that when opening a release we need to copy the last successful result (if any) in Jenkins to the new release, so that we don't consider packages which succeeded in trsuty and now fail in utopic as "always failed". That's a reasonable thing to do, I just want to point it out explicitly.

Code review for the new tests:

  * Checking the precise HTML formatting and colors in the tests seems a bit excessive to me, but I don't mind it much. But if you want to keep it, please don't duplicate the definition of ADT_EXCUSES_LABELS in the test but import it from autopkgtest (note, this might require renaming tests/autopkgtest.py to tests/test_autopkgtest.py to avoid a namespace clash).

  * __merge_records() could do with a docstring, as it's not quite clear what this does and why it's needed in a test. That's the sort of logic I'd expect in the actual code, but in the test suite the fake results should be pre-formatted correctly? Also, is it actually justified to assume any particular ordering in history.txt, or should autopkgtest.py not assume any and just order it by itself?

    def test_request_for_installable_fail_always(self):
        '''Requests a test for an installable package, test fail'''

I think it would make sense to split this in two: One should be the code as it is right now (i. e. with empty history), and be called "test_request_for_installable_first_fail()"; and another one with a history of just one (or perhaps two) FAILs, which is called "test_request_for_installable_fail_always" (with adjusted docstring). I think they both should count as ALWAYSFAIL, but I think it's still important to see that both cases behave as expected.

+ def test_request_for_installable_fail_regression_promoted(self):
+ '''Requests a test for an installable package, test fail, is a
+ regression.

Please no multi-line short docstrings, this causes truncated descriptions when running with -v and is also against PEP-8/looks ugly.

Some really small nitpicks:

+ ''' % {'py': sys.executable, 'path': self.data.path, 'rq':

  Please move the 'rq': to the next line so that key and value are together, for better readability.

+ '<li>autopkgtest for green 1.1~beta: {}'.format(ADT_EXCUSES_LABELS['RUNNING'])])

  We use %s macros (or similar) and the % operator instead of {} and .format() in other places, so maybe this could be made consistent?

+ In this case results for A 1didn't appear in the list of results

missing space after '1'. Also, excess ...

review: Needs Fixing

[Martin Pitt (pitti)]

autopkgtest.py
==============

- logging.warning(
- "Invalid line format: '%s', skipped" % line)
+ print("W: Invalid line format: '%s', skipped" % line)

This indeed should be fixed. However, for consistency this should use self.__log(..., type='W') instead of print.

+ if not trigsrc in self.pkglist[src][ver]['causes']:
+ self.pkglist[src][ver]['causes'][trigsrc] = []
+ self.pkglist[src][ver]['causes'][trigsrc].append((trigver,
+ status))

This is a common pattern in Python which is much more succinct with a dict's setdefault() method. I. e.

                self.pkglist[src][ver]['causes'].setdefault(trigsrc, []).append(
                          (trigver, status))

It would be helpful to add a docstring to read() to say what it reads (the results file), what its format is, and what the structure of the generated self.pkgcauses and self.pkglist are.

Otherwise the new logic seems fine to me (thanks to JB for additional explanations on IRC).

britney.py
==========

+ adt_label = status
+ if status in ADT_EXCUSES_LABELS:
+ adt_label = ADT_EXCUSES_LABELS[status]

Do we realistically expect a status which isn't in the map? If not (and we want to get pointed to that error), I'd suggest to simply use the dict lookup. If we want to allow that possibility, I suggest this for simplification:

          adt_label = ADT_EXCUSES_LABELS.get(status, status)

LGTM otherwise.

[lp:~jibel/britney/fix_missing_results]

414. By Jean-Baptiste Lallement on 2014-05-12

* tests/test_autopkgtest.py:
- Added docstrings
- Replaced format by %
- Import ADT_EXCUSES_LABELS from autopkgtest.py instead of redefining it
- Renamed test to avoid name conflict with autopkgtest.py from britney
- Fixed some formatting

415. By Jean-Baptiste Lallement on 2014-05-12

autopkgtest.py: Document method read() and code simplification
britney.py: code simplification

[Jean-Baptiste Lallement (jibel)]

> Code review for the new tests:
>
> * Checking the precise HTML formatting and colors in the tests seems a bit
> excessive to me, but I don't mind it much. But if you want to keep it, please
> don't duplicate the definition of ADT_EXCUSES_LABELS in the test but import it
> from autopkgtest (note, this might require renaming tests/autopkgtest.py to
> tests/test_autopkgtest.py to avoid a namespace clash).

I renamed tests/autopkgtest.py and now import ADT_EXCUSES_LABELS.

>
> * __merge_records() could do with a docstring, as it's not quite clear what
> this does and why it's needed in a test. That's the sort of logic I'd expect
> in the actual code, but in the test suite the fake results should be pre-
> formatted correctly? Also, is it actually justified to assume any particular
> ordering in history.txt, or should autopkgtest.py not assume any and just
> order it by itself?
The sorting logic should indeed be in the fake britney, but it is more convenient to have actual python code in the test and make the fake adt-britney just return static data.
Results are sorted by version to have latest results last in the list. There is no other justification than to the ordering than keeping results for a given package grouped. I should revisit it in adt-britney.

>
> def test_request_for_installable_fail_always(self):
> '''Requests a test for an installable package, test fail'''
>
> I think it would make sense to split this in two: One should be the code as it
> is right now (i. e. with empty history), and be called
> "test_request_for_installable_first_fail()"; and another one with a history of
> just one (or perhaps two) FAILs, which is called
> "test_request_for_installable_fail_always" (with adjusted docstring). I think
> they both should count as ALWAYSFAIL, but I think it's still important to see
> that both cases behave as expected.

I renamed the test to clarify what this test does (first result fail) and the second case (test failed and previous test failed too) is already covered by test_history_always_failed()

>
> + def test_request_for_installable_fail_regression_promoted(self):
> + '''Requests a test for an installable package, test fail, is a
> + regression.
>
> Please no multi-line short docstrings, this causes truncated descriptions when
> running with -v and is also against PEP-8/looks ugly.
Fixed.

>
>
> Some really small nitpicks:
>
> + ''' % {'py': sys.executable, 'path': self.data.path,
> 'rq':
>
> Please move the 'rq': to the next line so that key and value are together,
> for better readability.
>
Fixed

> + '<li>autopkgtest for green 1.1~beta:
> {}'.format(ADT_EXCUSES_LABELS['RUNNING'])])
>
> We use %s macros (or similar) and the % operator instead of {} and .format()
> in other places, so maybe this could be made consistent?
>
Fixed

> + In this case results for A 1didn't appear in the list of results
>
> missing space after '1'. Also, excess empty lines after the docstring.
>
Fixed

> I'll do the code review for the actual britney.py and autopkgtest.py code in a
> separate comment.

Thanks for your review

[Jean-Baptiste Lallement (jibel)]

> autopkgtest.py
> ==============
>
> - logging.warning(
> - "Invalid line format: '%s', skipped" % line)
> + print("W: Invalid line format: '%s', skipped" % line)
>
> This indeed should be fixed. However, for consistency this should use
> self.__log(..., type='W') instead of print.
>
Actually __log() is a method of britney and used in britney.py. In autopkgtest.py only print() is used.

>
> + if not trigsrc in self.pkglist[src][ver]['causes']:
> + self.pkglist[src][ver]['causes'][trigsrc] = []
> + self.pkglist[src][ver]['causes'][trigsrc].append((trigver,
> + status))
>
> This is a common pattern in Python which is much more succinct with a dict's
> setdefault() method. I. e.

Fixed.

>
> self.pkglist[src][ver]['causes'].setdefault(trigsrc,
> []).append(
> (trigver, status))
>
> It would be helpful to add a docstring to read() to say what it reads (the
> results file), what its format is, and what the structure of the generated
> self.pkgcauses and self.pkglist are.

Added documentation.

>
> Otherwise the new logic seems fine to me (thanks to JB for additional
> explanations on IRC).
>
> britney.py
> ==========
>
> + adt_label = status
> + if status in ADT_EXCUSES_LABELS:
> + adt_label = ADT_EXCUSES_LABELS[status]
>

Fixed.

> Do we realistically expect a status which isn't in the map? If not (and we
> want to get pointed to that error), I'd suggest to simply use the dict lookup.
> If we want to allow that possibility, I suggest this for simplification:
>
> adt_label = ADT_EXCUSES_LABELS.get(status, status)
>

because adt-britney and britney are 2 separate projects, it is possible, while unlikely, that we add new statuses in adt-britney that are not in britney and make britney crash if the status is unknown.
>
> LGTM otherwise.

[Martin Pitt (pitti)]

Thanks Jean-Baptiste! Looks good to me now, passing on to release team / Colin.

review: Approve

[lp:~jibel/britney/fix_missing_results]

416. By Jean-Baptiste Lallement on 2014-05-12

Merged trunk

[Colin Watson (cjwatson)]

review: Approve

=== modified file 'autopkgtest.py'
--- autopkgtest.py	2013-11-15 09:20:54 +0000
+++ autopkgtest.py	2014-05-12 13:01:57 +0000
@@ -19,18 +19,24 @@
 
 from collections import defaultdict
 from contextlib import closing
-import logging
 import os
 import subprocess
 import tempfile
 from textwrap import dedent
 import time
-
 import apt_pkg
 
 
 adt_britney = os.path.expanduser("~/auto-package-testing/jenkins/adt-britney")
 
+ADT_PASS = ["PASS", "ALWAYSFAIL"]
+ADT_EXCUSES_LABELS = {
+    "PASS": '<span style="background:#87d96c">Pass</span>',
+    "ALWAYSFAIL": '<span style="background:#e5c545">Always failed</span>',
+    "REGRESSION": '<span style="background:#ff6666">Regression</span>',
+    "RUNNING": '<span style="background:#99ddff">Test in progress</span>',
+}
+
 
 class AutoPackageTest(object):
     """autopkgtest integration
@@ -62,7 +68,7 @@
                 components: main restricted universe multiverse
                 rsync_host: rsync://tachash.ubuntu-ci/adt/
                 datadir: ~/proposed-migration/autopkgtest/data""" %
-                (self.series, self.series, home)), file=rc_file)
+                         (self.series, self.series, home)), file=rc_file)
 
     @property
     def _request_path(self):
@@ -85,38 +91,39 @@
                         continue
                     linebits = line.split()
                     if len(linebits) < 2:
-                        logging.warning(
-                            "Invalid line format: '%s', skipped" % line)
+                        print("W: Invalid line format: '%s', skipped" % line)
                         continue
                     yield linebits
 
     def read(self):
+        '''Loads a list of results
+
+        This function loads a list of results returned by __parse() and builds
+        2 lists:
+            - a list of source package/version with all the causes that
+            triggered a test and the result of the test for this trigger.
+            - a list of packages/version that triggered a test with the source
+            package/version and result triggered by this package.
+        These lists will be used in result() called from britney.py to generate
+        excuses and now which uploads passed, caused regression or which tests
+        have always been failing
+        '''
         self.pkglist = defaultdict(dict)
         self.pkgcauses = defaultdict(lambda: defaultdict(list))
         for linebits in self._parse(self._result_path):
-            src = linebits.pop(0)
-            ver = linebits.pop(0)
-            self.pkglist[src][ver] = {
-                "status": "NEW",
-                "causes": {},
+            (src, ver, status) = linebits[:3]
+
+            if not (src in self.pkglist and ver in self.pkglist[src]):
+                self.pkglist[src][ver] = {
+                    "status": status,
+                    "causes": {}
                 }
-            try:
-                status = linebits.pop(0).upper()
-                self.pkglist[src][ver]["status"] = status
-                while True:
-                    trigsrc = linebits.pop(0)
-                    trigver = linebits.pop(0)
-                    self.pkglist[src][ver]["causes"][trigsrc] = trigver
-            except IndexError:
-                # End of the list
-                pass
-        for src in self.pkglist:
-            all_vers = sorted(self.pkglist[src], cmp=apt_pkg.version_compare)
-            for ver in self.pkglist[src]:
-                status = self.pkglist[src][ver]["status"]
-                for trigsrc, trigver in \
-                        self.pkglist[src][ver]["causes"].items():
-                    self.pkgcauses[trigsrc][trigver].append((status, src, ver))
+
+            i = iter(linebits[3:])
+            for trigsrc, trigver in zip(i, i):
+                self.pkglist[src][ver]['causes'].setdefault(
+                    trigsrc, []).append((trigver, status))
+                self.pkgcauses[trigsrc][trigver].append((status, src, ver))
 
     def _adt_britney(self, *args):
         command = [
@@ -197,12 +204,29 @@
         self.read()
         if self.britney.options.verbose:
             for src in sorted(self.pkglist):
-                for ver in self.pkglist[src]:
-                    print("I: [%s] - Collected autopkgtest status for %s_%s: "
-                          "%s" %
-                          (time.asctime(), src, ver,
-                           self.pkglist[src][ver]["status"]))
+                for ver in sorted(self.pkglist[src],
+                                  cmp=apt_pkg.version_compare):
+                    for trigsrc in sorted(self.pkglist[src][ver]['causes']):
+                        for trigver, status \
+                                in self.pkglist[src][ver]['causes'][trigsrc]:
+                            print("I: [%s] - Collected autopkgtest status "
+                                  "for %s_%s/%s_%s: " "%s" % (
+                                      time.asctime(), src, ver, trigsrc,
+                                      trigver, status))
 
     def results(self, trigsrc, trigver):
         for status, src, ver in self.pkgcauses[trigsrc][trigver]:
+            # Check for regresssion
+            if status == 'FAIL':
+                passed_once = False
+                for ver in self.pkglist[src]:
+                    for trigsrc in self.pkglist[src][ver]['causes']:
+                        for trigver, status \
+                                in self.pkglist[src][ver]['causes'][trigsrc]:
+                            if status == 'PASS':
+                                passed_once = True
+                if not passed_once:
+                    status = 'ALWAYSFAIL'
+                else:
+                    status = 'REGRESSION'
             yield status, src, ver

=== modified file 'britney.py'
--- britney.py	2014-03-05 16:14:48 +0000
+++ britney.py	2014-05-12 13:01:57 +0000
@@ -222,7 +222,7 @@
 from consts import (VERSION, SECTION, BINARIES, MAINTAINER, FAKESRC,
                    SOURCE, SOURCEVER, ARCHITECTURE, DEPENDS, CONFLICTS,
                    PROVIDES, RDEPENDS, RCONFLICTS, MULTIARCH)
-from autopkgtest import AutoPackageTest
+from autopkgtest import AutoPackageTest, ADT_PASS, ADT_EXCUSES_LABELS
 
 __author__ = 'Fabio Tranchitella and the Debian Release Team'
 __version__ = '2.0'
@@ -1756,18 +1756,19 @@
                 adtpass = True
                 for status, adtsrc, adtver in autopkgtest.results(
                         e.name, e.ver[1]):
-                    public_url = "%s/%s-adt-%s/" % (
+                    public_url = "%s/%s-adt-%s/lastBuild" % (
                         jenkins_public, self.options.adt_series,
                         adtsrc.replace("+", "-"))
-                    private_url = "%s/%s-adt-%s/" % (
+                    private_url = "%s/%s-adt-%s/lastBuild" % (
                         jenkins_private, self.options.adt_series,
                         adtsrc.replace("+", "-"))
+                    adt_label = ADT_EXCUSES_LABELS.get(status, status)
                     e.addhtml(
                         "autopkgtest for %s %s: %s (Jenkins: "
                         "<a href=\"%s\">public</a>, "
                         "<a href=\"%s\">private</a>)" %
-                        (adtsrc, adtver, status, public_url, private_url))
-                    if status != "PASS":
+                        (adtsrc, adtver, adt_label, public_url, private_url))
+                    if status not in ADT_PASS:
                         hints = self.hints.search(
                             'force-badtest', package=adtsrc)
                         hints.extend(

=== renamed file 'tests/autopkgtest.py' => 'tests/test_autopkgtest.py'
--- tests/autopkgtest.py	2014-05-12 12:04:55 +0000
+++ tests/test_autopkgtest.py	2014-05-12 13:01:57 +0000
@@ -12,7 +12,10 @@
 import sys
 import subprocess
 import unittest
+import apt_pkg
+import operator
 
+apt_pkg.init()
 architectures = ['amd64', 'arm64', 'armhf', 'i386', 'powerpc', 'ppc64el']
 
 my_dir = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
@@ -20,6 +23,9 @@
 NOT_CONSIDERED = False
 VALID_CANDIDATE = True
 
+sys.path.insert(0, my_dir)
+from autopkgtest import ADT_EXCUSES_LABELS
+
 
 class TestData:
     def __init__(self):
@@ -157,7 +163,29 @@
     def tearDown(self):
         del self.data
 
-    def make_adt_britney(self, request):
+    def __merge_records(self, results, history=""):
+        '''Merges a list of results with records in history.
+
+        This function merges results from a collect with records already in
+        history and sort records by version/name of causes and version/name of
+        source packages with tests. This should be done in the fake
+        adt-britney but it is more convenient to just pass a static list of
+        records and make adt-britney just return this list.
+        '''
+
+        if history is None:
+            history = ""
+        records = [x.split() for x in (results.strip() + '\n' +
+                                       history.strip()).split('\n') if x]
+
+        records.sort(cmp=apt_pkg.version_compare, key=operator.itemgetter(4))
+        records.sort(key=operator.itemgetter(3))
+        records.sort(cmp=apt_pkg.version_compare, key=operator.itemgetter(1))
+        records.sort()
+
+        return "\n".join([' '.join(x) for x in records])
+
+    def make_adt_britney(self, request, history=""):
         with open(self.adt_britney, 'w') as f:
             f.write('''#!%(py)s
 import argparse, shutil,sys
@@ -175,7 +203,7 @@
 
 def collect():
     with open(args.output, 'w') as f:
-        f.write("""%(rq)s""")
+        f.write("""%(res)s""")
 
 p = argparse.ArgumentParser()
 p.add_argument('-c', '--config')
@@ -202,7 +230,9 @@
 
 args = p.parse_args()
 args.func()
-''' % {'py': sys.executable, 'path': self.data.path, 'rq': request})
+                    ''' % {'py': sys.executable, 'path': self.data.path,
+                           'rq': request,
+                           'res': self.__merge_records(request, history)})
 
     def run_britney(self, args=[]):
         '''Run britney.
@@ -245,9 +275,19 @@
             'green 1.1~beta RUNNING green 1.1~beta\n',
             NOT_CONSIDERED,
             [r'\bgreen\b.*>1</a> to .*>1.1~beta<',
-             '<li>autopkgtest for green 1.1~beta: RUNNING'])
-
-    def test_request_for_installable_fail(self):
+             '<li>autopkgtest for green 1.1~beta: %s' % ADT_EXCUSES_LABELS['RUNNING']])
+
+    def test_request_for_installable_first_fail(self):
+        '''Requests a test for an installable package. No history and first result is a failure'''
+
+        self.do_test(
+            [('green', {'Version': '1.1~beta', 'Depends': 'libc6 (>= 0.9), libgreen1'})],
+            'green 1.1~beta FAIL green 1.1~beta\n',
+            VALID_CANDIDATE,
+            [r'\bgreen\b.*>1</a> to .*>1.1~beta<',
+             '<li>autopkgtest for green 1.1~beta: %s' % ADT_EXCUSES_LABELS['ALWAYSFAIL']])
+
+    def test_request_for_installable_fail_regression(self):
         '''Requests a test for an installable package, test fail'''
 
         self.do_test(
@@ -255,7 +295,8 @@
             'green 1.1~beta FAIL green 1.1~beta\n',
             NOT_CONSIDERED,
             [r'\bgreen\b.*>1</a> to .*>1.1~beta<',
-             '<li>autopkgtest for green 1.1~beta: FAIL'])
+             '<li>autopkgtest for green 1.1~beta: %s' % ADT_EXCUSES_LABELS['REGRESSION']],
+            history='green 1.0~beta PASS green 1.0~beta\n')
 
     def test_request_for_installable_pass(self):
         '''Requests a test for an installable package, test pass'''
@@ -265,7 +306,7 @@
             'green 1.1~beta PASS green 1.1~beta\n',
             VALID_CANDIDATE,
             [r'\bgreen\b.*>1</a> to .*>1.1~beta<',
-             '<li>autopkgtest for green 1.1~beta: PASS'])
+             '<li>autopkgtest for green 1.1~beta: %s' % ADT_EXCUSES_LABELS['PASS']])
 
     def test_multi_rdepends_with_tests_running(self):
         '''Multiple reverse dependencies with tests (still running)'''
@@ -276,10 +317,22 @@
             'darkgreen 1 RUNNING green 2\n',
             NOT_CONSIDERED,
             [r'\bgreen\b.*>1</a> to .*>2<',
-             '<li>autopkgtest for lightgreen 1: PASS',
-             '<li>autopkgtest for darkgreen 1: RUNNING'])
-
-    def test_multi_rdepends_with_tests_fail(self):
+             '<li>autopkgtest for lightgreen 1: %s' % ADT_EXCUSES_LABELS['PASS'],
+             '<li>autopkgtest for darkgreen 1: %s' % ADT_EXCUSES_LABELS['RUNNING']])
+
+    def test_multi_rdepends_with_tests_fail_always(self):
+        '''Multiple reverse dependencies with tests (fail)'''
+
+        self.do_test(
+            [('libgreen1', {'Version': '2', 'Source': 'green', 'Depends': 'libc6'})],
+            'lightgreen 1 PASS green 2\n'
+            'darkgreen 1 FAIL green 2\n',
+            VALID_CANDIDATE,
+            [r'\bgreen\b.*>1</a> to .*>2<',
+             '<li>autopkgtest for lightgreen 1: %s' % ADT_EXCUSES_LABELS['PASS'],
+             '<li>autopkgtest for darkgreen 1: %s' % ADT_EXCUSES_LABELS['ALWAYSFAIL']])
+
+    def test_multi_rdepends_with_tests_fail_regression(self):
         '''Multiple reverse dependencies with tests (fail)'''
 
         self.do_test(
@@ -288,8 +341,9 @@
             'darkgreen 1 FAIL green 2\n',
             NOT_CONSIDERED,
             [r'\bgreen\b.*>1</a> to .*>2<',
-             '<li>autopkgtest for lightgreen 1: PASS',
-             '<li>autopkgtest for darkgreen 1: FAIL'])
+             '<li>autopkgtest for lightgreen 1: %s' % ADT_EXCUSES_LABELS['PASS'],
+             '<li>autopkgtest for darkgreen 1: %s' % ADT_EXCUSES_LABELS['REGRESSION']],
+            history='darkgreen 1 PASS green 1\n')
 
     def test_multi_rdepends_with_tests_pass(self):
         '''Multiple reverse dependencies with tests (pass)'''
@@ -300,8 +354,8 @@
             'darkgreen 1 PASS green 2\n',
             VALID_CANDIDATE,
             [r'\bgreen\b.*>1</a> to .*>2<',
-             '<li>autopkgtest for lightgreen 1: PASS',
-             '<li>autopkgtest for darkgreen 1: PASS'])
+             '<li>autopkgtest for lightgreen 1: %s' % ADT_EXCUSES_LABELS['PASS'],
+             '<li>autopkgtest for darkgreen 1: %s' % ADT_EXCUSES_LABELS['PASS']])
 
     def test_multi_rdepends_with_some_tests_running(self):
         '''Multiple reverse dependencies with some tests (running)'''
@@ -315,10 +369,25 @@
             'darkgreen 1 RUNNING green 2\n',
             NOT_CONSIDERED,
             [r'\bgreen\b.*>1</a> to .*>2<',
-             '<li>autopkgtest for lightgreen 1: RUNNING',
-             '<li>autopkgtest for darkgreen 1: RUNNING'])
-
-    def test_multi_rdepends_with_some_tests_fail(self):
+             '<li>autopkgtest for lightgreen 1: %s' % ADT_EXCUSES_LABELS['RUNNING'],
+             '<li>autopkgtest for darkgreen 1: %s' % ADT_EXCUSES_LABELS['RUNNING']])
+
+    def test_multi_rdepends_with_some_tests_fail_always(self):
+        '''Multiple reverse dependencies with some tests (fail)'''
+
+        # add a third reverse dependency to libgreen1 which does not have a test
+        self.data.add('mint', False, {'Depends': 'libgreen1'})
+
+        self.do_test(
+            [('libgreen1', {'Version': '2', 'Source': 'green', 'Depends': 'libc6'})],
+            'lightgreen 1 PASS green 2\n'
+            'darkgreen 1 FAIL green 2\n',
+            VALID_CANDIDATE,
+            [r'\bgreen\b.*>1</a> to .*>2<',
+             '<li>autopkgtest for lightgreen 1: %s' % ADT_EXCUSES_LABELS['PASS'],
+             '<li>autopkgtest for darkgreen 1: %s' % ADT_EXCUSES_LABELS['ALWAYSFAIL']])
+
+    def test_multi_rdepends_with_some_tests_fail_regression(self):
         '''Multiple reverse dependencies with some tests (fail)'''
 
         # add a third reverse dependency to libgreen1 which does not have a test
@@ -330,8 +399,9 @@
             'darkgreen 1 FAIL green 2\n',
             NOT_CONSIDERED,
             [r'\bgreen\b.*>1</a> to .*>2<',
-             '<li>autopkgtest for lightgreen 1: PASS',
-             '<li>autopkgtest for darkgreen 1: FAIL'])
+             '<li>autopkgtest for lightgreen 1: %s' % ADT_EXCUSES_LABELS['PASS'],
+             '<li>autopkgtest for darkgreen 1: %s' % ADT_EXCUSES_LABELS['REGRESSION']],
+            history='darkgreen 1 PASS green 1\n')
 
     def test_multi_rdepends_with_some_tests_pass(self):
         '''Multiple reverse dependencies with some tests (pass)'''
@@ -345,8 +415,8 @@
             'darkgreen 1 PASS green 2\n',
             VALID_CANDIDATE,
             [r'\bgreen\b.*>1</a> to .*>2<',
-             '<li>autopkgtest for lightgreen 1: PASS',
-             '<li>autopkgtest for darkgreen 1: PASS'])
+             '<li>autopkgtest for lightgreen 1: %s' % ADT_EXCUSES_LABELS['PASS'],
+             '<li>autopkgtest for darkgreen 1: %s' % ADT_EXCUSES_LABELS['PASS']])
 
     def test_binary_from_new_source_package_running(self):
         '''building an existing binary for a new source package (running)'''
@@ -357,10 +427,22 @@
             'darkgreen 1 RUNNING newgreen 2\n',
             NOT_CONSIDERED,
             [r'\bnewgreen\b.*\(- to .*>2<',
-             '<li>autopkgtest for lightgreen 1: PASS',
-             '<li>autopkgtest for darkgreen 1: RUNNING'])
-
-    def test_binary_from_new_source_package_fail(self):
+             '<li>autopkgtest for lightgreen 1: %s' % ADT_EXCUSES_LABELS['PASS'],
+             '<li>autopkgtest for darkgreen 1: %s' % ADT_EXCUSES_LABELS['RUNNING']])
+
+    def test_binary_from_new_source_package_fail_always(self):
+        '''building an existing binary for a new source package (fail)'''
+
+        self.do_test(
+            [('libgreen1', {'Version': '2', 'Source': 'newgreen', 'Depends': 'libc6'})],
+            'lightgreen 1 PASS newgreen 2\n'
+            'darkgreen 1 FAIL newgreen 2\n',
+            VALID_CANDIDATE,
+            [r'\bnewgreen\b.*\(- to .*>2<',
+             '<li>autopkgtest for lightgreen 1: %s' % ADT_EXCUSES_LABELS['PASS'],
+             '<li>autopkgtest for darkgreen 1: %s' % ADT_EXCUSES_LABELS['ALWAYSFAIL']])
+
+    def test_binary_from_new_source_package_fail_regression(self):
         '''building an existing binary for a new source package (fail)'''
 
         self.do_test(
@@ -369,8 +451,9 @@
             'darkgreen 1 FAIL newgreen 2\n',
             NOT_CONSIDERED,
             [r'\bnewgreen\b.*\(- to .*>2<',
-             '<li>autopkgtest for lightgreen 1: PASS',
-             '<li>autopkgtest for darkgreen 1: FAIL'])
+             '<li>autopkgtest for lightgreen 1: %s' % ADT_EXCUSES_LABELS['PASS'],
+             '<li>autopkgtest for darkgreen 1: %s' % ADT_EXCUSES_LABELS['REGRESSION']],
+            history='darkgreen 1 PASS green 1\n')
 
     def test_binary_from_new_source_package_pass(self):
         '''building an existing binary for a new source package (pass)'''
@@ -381,8 +464,8 @@
             'darkgreen 1 PASS newgreen 2\n',
             VALID_CANDIDATE,
             [r'\bnewgreen\b.*\(- to .*>2<',
-             '<li>autopkgtest for lightgreen 1: PASS',
-             '<li>autopkgtest for darkgreen 1: PASS'])
+             '<li>autopkgtest for lightgreen 1: %s' % ADT_EXCUSES_LABELS['PASS'],
+             '<li>autopkgtest for darkgreen 1: %s' % ADT_EXCUSES_LABELS['PASS']])
 
     def test_binary_from_new_source_package_uninst(self):
         '''building an existing binary for a new source package (uninstallable)'''
@@ -406,14 +489,76 @@
             NOT_CONSIDERED,
             [r'\bgreen\b.*>1</a> to .*>1.1~beta<',
              # it's not entirely clear what precisely it should say here
-             '<li>autopkgtest for green 1.1~beta: RUNNING'])
+             '<li>autopkgtest for green 1.1~beta: %s' % ADT_EXCUSES_LABELS['RUNNING']])
+
+    def test_request_for_installable_fail_regression_promoted(self):
+        '''Requests a test for an installable package, test fail, is a regression.
+
+        This test verifies a bug in britney where a package was promoted if latest test
+        appeared before previous result in history, only the last result in
+        alphabetic order was taken into account. For example:
+            A 1 FAIL B 1
+            A 1 PASS A 1
+        In this case results for A 1 didn't appear in the list of results
+        triggered by the upload of B 1 and B 1 was promoted
+        '''
+
+        self.do_test(
+            [('green', {'Version': '1.1~beta', 'Depends': 'libc6 (>= 0.9), libgreen1'})],
+            'lightgreen 1 FAIL green 1.1~beta\n',
+            NOT_CONSIDERED,
+            [r'\bgreen\b.*>1</a> to .*>1.1~beta<',
+             '<li>autopkgtest for lightgreen 1: %s' % ADT_EXCUSES_LABELS['REGRESSION']],
+            history="lightgreen 1 PASS lightgreen 1"
+        )
+
+    def test_history_always_passed(self):
+        '''All the results in history are PASS, and test passed
+
+        '''
+
+        self.do_test(
+            [('green', {'Version': '1.1~beta', 'Depends': 'libc6 (>= 0.9), libgreen1'})],
+            'lightgreen 1 PASS green 1.1~beta\n',
+            VALID_CANDIDATE,
+            [r'\bgreen\b.*>1</a> to .*>1.1~beta<',
+             '<li>autopkgtest for lightgreen 1: %s' % ADT_EXCUSES_LABELS['PASS']],
+            history="lightgreen 1 PASS lightgreen 1"
+        )
+
+    def test_history_always_failed(self):
+        '''All the results in history are FAIL, test fails. not a regression.
+
+        '''
+
+        self.do_test(
+            [('green', {'Version': '1.1~beta', 'Depends': 'libc6 (>= 0.9), libgreen1'})],
+            'lightgreen 1 FAIL green 1.1~beta\n',
+            VALID_CANDIDATE,
+            [r'\bgreen\b.*>1</a> to .*>1.1~beta<',
+             '<li>autopkgtest for lightgreen 1: %s' % ADT_EXCUSES_LABELS['ALWAYSFAIL']],
+            history="lightgreen 1 FAIL lightgreen 1"
+        )
+
+    def test_history_regression(self):
+        '''All the results in history are PASS, test fails. Blocked.
+
+        '''
+        self.do_test(
+            [('green', {'Version': '1.1~beta', 'Depends': 'libc6 (>= 0.9), libgreen1'})],
+            'lightgreen 1 FAIL green 1.1~beta\n',
+            NOT_CONSIDERED,
+            [r'\bgreen\b.*>1</a> to .*>1.1~beta<',
+             '<li>autopkgtest for lightgreen 1: %s' % ADT_EXCUSES_LABELS['REGRESSION']],
+            history="lightgreen 1 PASS lightgreen 1"
+        )
 
     def do_test(self, unstable_add, adt_request, considered, expect=None,
-                no_expect=None):
+                no_expect=None, history=""):
         for (pkg, fields) in unstable_add:
             self.data.add(pkg, True, fields)
 
-        self.make_adt_britney(adt_request)
+        self.make_adt_britney(adt_request, history)
 
         (excuses, out) = self.run_britney()
         #print('-------\nexcuses: %s\n-----' % excuses)
@@ -437,7 +582,8 @@
         self.data.add('yellow', True, {'Version': '1.1~beta',
                                        'Depends': 'libc6 (>= 0.9), nosuchpkg'})
 
-        self.make_adt_britney('yellow 1.1~beta RUNNING yellow 1.1~beta\n')
+        self.make_adt_britney('yellow 1.1~beta RUNNING yellow 1.1~beta\n',
+                              'purple 2 FAIL pink 3.0.~britney\n')
 
         print('run:\n%s -c %s\n' % (self.britney, self.britney_conf))
         subprocess.call(['bash', '-i'], cwd=self.data.path)