Merge ~juliank/britney/+git/britney2-ubuntu:lp1688516 into ~ubuntu-release/britney/+git/britney2-ubuntu:master

[Julian Andres Klode]

needs some name bikeshedding

[Julian Andres Klode (juliank)]

Might want to do something other than the revert of the excuses removal.

[Julian Andres Klode (juliank)]

We can make ALWAYSFAIL an IGNORE-FAIL in such a case, or add a new state?

[Iain Lane (laney)]

Basic approach is good, thanks!

As discussed, we should handle the multiple hint case differently: find the highest version that is at most the current trigger and look at that one. If there are multiple versions in play - a version in testing and a version (possibly this is one of many uploads!) in unstable, we want to be able to hint them independently.

I think an 'excuse' should be added so that it's clear on update_excuses.html that a test is only ALWAYSFAIL because it's been hinted - for force-skiptest we say "Should wait for tests relating to %s %s, but forced by %s". Our string could be very similar.

Does /arch work and can we have a test for that?

[Iain Lane (laney)]

review: Needs Fixing

[Julian Andres Klode (juliank)]

We don't correctly handle multiple hints, need to fix that.

[Paul Gevers (paul-climbing)]

Are you aware of the changes I made in Debian to reset the base line?

In Debian I have a migration-reference/0 trigger that tests purely in testing. When there is a new result with this trigger, the baseline gets reset to that. Otherwise any pass in testing, except for the package itself when taken from unstable put the baseline to passing.

[Iain Lane (laney)]

Paul - I think you could use a hint like this to effect a 'reset the baseline to testing' check like you & Steve are proposing? I'm not sure that I've considered all the implications, but it sounds like you want consider that the baseline is always the version that's in testing? Maybe slightly modified to consider only the test runs where the triggering package/version is the one in testing.

@juliank - can you check https://code.launchpad.net/~laney/britney/+git/britney2-ubuntu/+ref/force-reset-test please? I bikeshedded the name, but more importantly made force-reset-test foo/N apply to all versions <= N and ALWAYSFAIL them too. Seems more intuitive, do you think?

[Iain Lane (laney)]

Any comments or should I just merge this?

[Julian Andres Klode (juliank)]

I must say that I prefer the Debian approach somewhat, as it solves two problems:
(a) Testing against the release pocket, and
(b) resetting the baseline
in one operation. That's somewhat easier than manually retrying it with a fake trigger like hello and then if that fails too force-reset-test it.

[Iain Lane (laney)]

1) Why a fake trigger rather than the package/version in release?

2) While systematically forcing to the version in release is one potential use of this, I think that is a *policy* question that is as yet undecided in Ubuntu. In the absence of such a policy, release team members can use this hint when they decide that a regression should not be treated as one - for example if there is a 'fluke' pass, which sometimes happens. In other words, I think the "Debian" approach is the answer to a different question that we're not asking here.

diff --git a/britney2/policies/autopkgtest.py b/britney2/policies/autopkgtest.py
index 806debd..fbf83f8 100644
--- a/britney2/policies/autopkgtest.py
+++ b/britney2/policies/autopkgtest.py
@@ -102,6 +102,7 @@ class AutopkgtestPolicy(BasePolicy):
 
     def register_hints(self, hint_parser):
         hint_parser.register_hint_type('force-badtest', britney2.hints.split_into_one_hint_per_package)
+        hint_parser.register_hint_type('force-badtest-until', britney2.hints.split_into_one_hint_per_package)
         hint_parser.register_hint_type('force-skiptest', britney2.hints.split_into_one_hint_per_package)
 
     def initialise(self, britney):
@@ -658,12 +659,22 @@ class AutopkgtestPolicy(BasePolicy):
             # save pending.json right away, so that we don't re-request if britney crashes
             self.save_pending_json()
 
-    def check_ever_passed(self, src, arch):
-        '''Check if tests for src ever passed on arch'''
+    def check_ever_passed_before(self, src, max_ver, arch, min_ver=None):
+        '''Check if tests for src ever passed on arch for specified range
+
+        If min_ver is specified, it checks that all versions in
+        [min_ver, max_ver) have passed; otherwise it checks that
+        [min_ver, inf) have passed.'''
 
         # FIXME: add caching
         for srcmap in self.test_results.values():
             try:
+                too_high = apt_pkg.version_compare(srcmap[src][arch][1], max_ver) > 0
+                too_low = apt_pkg.version_compare(srcmap[src][arch][1], min_ver) <= 0 if min_ver else False
+
+                if too_high or too_low:
+                    continue
+
                 if srcmap[src][arch][0]:
                     return True
             except KeyError:
@@ -677,7 +688,8 @@ class AutopkgtestPolicy(BasePolicy):
         EXCUSES_LABELS. log_url is None if the test is still running.
         '''
         # determine current test result status
-        ever_passed = self.check_ever_passed(src, arch)
+        until = self.find_max_force_badtest_until(src, ver, arch)
+        ever_passed = self.check_ever_passed_before(src, ver, arch, until)
         url = None
         try:
             r = self.test_results[trigger][src][arch]
@@ -724,6 +736,22 @@ class AutopkgtestPolicy(BasePolicy):
 
         return (result, ver, url)
 
+    def find_max_force_badtest_until(self, src, ver, arch):
+        '''Find the maximum force-badtest-until hint before/including ver'''
+        hints = self.britney.hints.search('force-badtest-until', package=src)
+        found_ver = None
+
+        if hints:
+            for hint in hints:
+                for mi in hint.packages:
+                    if (mi.architecture in ['source', arch] and
+                            mi.version != 'all' and
+                            apt_pkg.version_compare(mi.version, ver) <= 0 and
+                            (found_ver is None or apt_pkg.version_compare(found_ver, mi.version) < 0)):
+                        found_ver = mi.version
+
+        return found_ver
+
     def has_force_badtest(self, src, ver, arch):
         '''Check if src/ver/arch has a force-badtest hint'''
 
@@ -736,4 +764,3 @@ class AutopkgtestPolicy(BasePolicy):
                     return True
 
         return False
-
diff --git a/britney2/utils.py b/britney2/utils.py
index 837dad6..25223c8 100644
--- a/britney2/utils.py
+++ b/britney2/utils.py
@@ -368,8 +368,6 @@ def write_excuses(excuselist, dest_file, output_format="yaml"):
         ensuredir(os.path.dirname(dest_file))
         with open(dest_file, 'w', encoding='utf-8') as f:
             edatalist = [e.excusedata() for e in excuselist]
-            for e in edatalist:
-                del(e['excuses'])
             excusesdata = {
                 'sources': edatalist,
                 'generated-date': datetime.utcnow(),
diff --git a/tests/test_autopkgtest.py b/tests/test_autopkgtest.py
index bc736ae..a780845 100755
--- a/tests/test_autopkgtest.py
+++ b/tests/test_autopkgtest.py
@@ -1555,6 +1555,137 @@ class T(TestBase):
             {'green': [('old-version', '1'), ('new-version', '2')]
             })
 
+    def test_hint_force_badtest_until_goodbad_alwaysfail(self):
+        '''force-badtest-until hint marks as alwaysfail'''
+
+        self.swift.set_results({'autopkgtest-series': {
+            'series/amd64/l/lightgreen/20150101_100100@': (0, 'lightgreen 1', tr('lightgreen/1')),
+            'series/amd64/l/lightgreen/20150101_100101@': (4, 'lightgreen 2', tr('lightgreen/2')),
+        }})
+
+        self.create_hint('pitti', 'force-badtest-until lightgreen/1')
+
+        self.do_test(
+            [('lightgreen', {'Version': '2', 'Source': 'lightgreen', 'Depends': 'libc6'}, 'autopkgtest')],
+            {'lightgreen': (True, {
+                              'lightgreen/2': {'amd64': 'ALWAYSFAIL'},
+                             }),
+            },
+            {'lightgreen': [('old-version', '1'), ('new-version', '2')]
+            })
+
+    def test_hint_force_badtest_until_goodbad_alwaysfail_arch(self):
+        '''force-badtest-until hint marks as alwaysfail per arch'''
+
+        self.swift.set_results({'autopkgtest-series': {
+            'series/amd64/l/lightgreen/20150101_100100@': (0, 'lightgreen 1', tr('lightgreen/1')),
+            'series/amd64/l/lightgreen/20150101_100101@': (4, 'lightgreen 2', tr('lightgreen/2')),
+            'series/i386/l/lightgreen/20150101_100100@': (0, 'lightgreen 1', tr('lightgreen/1')),
+            'series/i386/l/lightgreen/20150101_100101@': (4, 'lightgreen 2', tr('lightgreen/2')),
+        }})
+
+        self.create_hint('pitti', 'force-badtest-until lightgreen/1/amd64')
+
+        self.do_test(
+            [('lightgreen', {'Version': '2', 'Source': 'lightgreen', 'Depends': 'libc6'}, 'autopkgtest')],
+            {'lightgreen': (False, {
+                             'lightgreen/2': {'amd64': 'ALWAYSFAIL', 'i386': 'REGRESSION'},
+                             }),
+            },
+            {'lightgreen': [('old-version', '1'), ('new-version', '2')]
+            })
+
+    def test_hint_force_badtest_until_bad_good_pass(self):
+        '''force-badtest-until hint followed by pass is pass'''
+
+        self.swift.set_results({'autopkgtest-series': {
+            'series/amd64/l/lightgreen/20150101_100100@': (4, 'lightgreen 1', tr('lightgreen/1')),
+            'series/amd64/l/lightgreen/20150102_100101@': (0, 'lightgreen 2', tr('lightgreen/2')),
+        }})
+
+        self.create_hint('pitti', 'force-badtest-until lightgreen/1')
+
+        self.do_test(
+            [('lightgreen', {'Version': '2', 'Source': 'lightgreen', 'Depends': 'libc6'}, 'autopkgtest')],
+            {'lightgreen': (True, {
+                              'lightgreen/2': {'amd64': 'PASS'},
+                             }),
+            },
+            {'lightgreen': [('old-version', '1'), ('new-version', '2')]
+            })
+
+    def test_hint_force_badtest_until_bad_good_bad_regression(self):
+        '''force-badtest-until hint followed by good, bad is regression'''
+
+        self.swift.set_results({'autopkgtest-series': {
+            'series/amd64/l/lightgreen/20150101_100100@': (4, 'lightgreen 1', tr('lightgreen/1')),
+            'series/amd64/l/lightgreen/20150102_100101@': (0, 'lightgreen 2', tr('lightgreen/2')),
+            'series/amd64/l/lightgreen/20150103_100101@': (4, 'lightgreen 3', tr('lightgreen/3')),
+        }})
+
+        self.create_hint('pitti', 'force-badtest-until lightgreen/1')
+
+        self.do_test(
+            [('lightgreen', {'Version': '3', 'Source': 'lightgreen', 'Depends': 'libc6'}, 'autopkgtest')],
+            {'lightgreen': (False, {
+                              'lightgreen/3': {'amd64': 'REGRESSION'},
+                             }),
+            },
+            {'lightgreen': [('old-version', '1'), ('new-version', '3')]
+            })
+
+    def test_hint_force_badtest_until_multiple_hints(self):
+        '''force-badtest-until multiple hints check ranges'''
+
+        self.swift.set_results({'autopkgtest-series': {
+            'series/amd64/l/lightgreen/20150100_100100@': (0, 'lightgreen 1', tr('lightgreen/1')),
+            'series/amd64/l/lightgreen/20150101_100100@': (4, 'lightgreen 1', tr('green/2')),
+            'series/amd64/l/lightgreen/20150102_100101@': (0, 'lightgreen 2', tr('lightgreen/2')),
+            'series/amd64/l/lightgreen/20150103_100101@': (4, 'lightgreen 3', tr('lightgreen/3')),
+        }})
+
+
+        self.do_test(
+            [('libgreen1', {'Version': '2', 'Source': 'green', 'Depends': 'libc6'}, 'autopkgtest'),
+             ('lightgreen', {'Version': '3', 'Source': 'lightgreen', 'Depends': 'libc6'}, 'autopkgtest')],
+            {'green': (False, {
+                              'lightgreen/1': {'amd64': 'REGRESSION'},
+                             }),
+            },
+            {'green': [('old-version', '1'), ('new-version', '2')],
+             'lightgreen': [('old-version', '1'), ('new-version', '3')],
+            })
+
+        self.create_hint('pitti', 'force-badtest-until lightgreen/1')
+        self.do_test(
+            [],
+            {'green': (True, {
+                              'lightgreen/1': {'amd64': 'ALWAYSFAIL'},
+                             }),
+             'lightgreen': (False, {
+                              'lightgreen/3': {'amd64': 'REGRESSION'},
+                             }),
+
+            },
+            {'green': [('old-version', '1'), ('new-version', '2')],
+             'lightgreen': [('old-version', '1'), ('new-version', '3')],
+            })
+        self.create_hint('pitti', 'force-badtest-until lightgreen/3')
+        self.do_test(
+            [],
+            {'green': (True, {
+                              'lightgreen/1': {'amd64': 'ALWAYSFAIL'},
+                             }),
+             'lightgreen': (True, {
+                              'lightgreen/3': {'amd64': 'ALWAYSFAIL'},
+                             }),
+
+            },
+            {'green': [('old-version', '1'), ('new-version', '2')],
+             'lightgreen': [('old-version', '1'), ('new-version', '3')],
+            })
+
+
     def test_hint_force_badtest_multi_version(self):
         '''force-badtest hint'''