Autopkgtest: Don't re-run passed test results

Test requesting: Don't re-request a test if we already have a result for it for
that trigger (for a relevant version), but there is a new version of the tested
package. First this unnecessarily delays propagation as the test will go back
to "in progress", and second if it fails in the next run this isn't the fault
of the original trigger, but the new version of the tested package.

Result finding: Don't limit acceptable results to the latest version of the
tested package. It's perfectly fine if an earlier version (like the one in
testing, or an earlier upload) was ran and gave a result for the requesting
trigger. If it's PASS, then we are definitively done, and if it's a failure
there is the "Checking for new results for failed test..." logic in collect()
logic which will also accept newer results.

autopkgtest.py

@@ -307,8 +307,14 @@ class AutoPackageTest(object):
         # didn't see in britney yet
         ver_trig_results = self.test_results.get(src, {}).get(arch, [None, {}, None])[1]
         unstable_ver = self.britney.sources['unstable'][src][VERSION]
-        for result_ver in [ver, unstable_ver]:
-            if result_ver not in ver_trig_results or apt_pkg.version_compare(result_ver, ver) < 0:
+        try:
+            testing_ver = self.britney.sources['testing'][src][VERSION]
+        except KeyError:
+            testing_ver = unstable_ver
+        for result_ver in set([testing_ver, ver, unstable_ver]):
+            # result_ver might be < ver here; that's okay, if we already have a
+            # result for trigsrc/trigver we don't need to re-run it again
+            if result_ver not in ver_trig_results:
                 continue
             for trigger in ver_trig_results[result_ver]:
                 (tsrc, tver) = trigger.split('/', 1)
@@ -646,18 +652,25 @@ class AutoPackageTest(object):
                 try:
                     (_, ver_map, ever_passed) = self.test_results[testsrc][arch]
 
-                    # we prefer passing tests for the specified testver; but if
-                    # they fail, we also check for a result > testver, as test
-                    # runs might see built versions which we didn't see in
-                    # britney yet
+                    # check if we have a result for any version of testsrc that
+                    # was triggered for trigsrc/trigver; we prefer PASSes, as
+                    # it could be that an unrelated package upload could break
+                    # testsrc's tests at a later point
+                    status = None
+                    for ver, trigger_results in ver_map.items():
                         try:
-                        trigger_results = ver_map[testver]
-                        if not trigger_results[trigger]:
-                            raise KeyError
+                            status = trigger_results[trigger]
+                            testver = ver
+                            # if we found a PASS, we can stop searching
+                            if status is True:
+                                break
                         except KeyError:
-                        (testver, trigger_results) = latest_item(ver_map, testver)
+                            pass
+
+                    if status is None:
+                        # no result? go to "still running" below
+                        raise KeyError
 
-                    status = trigger_results[trigger]
                     if status:
                         result = 'PASS'
                     else:

tests/test_autopkgtest.py

@@ -1087,6 +1087,44 @@ lightgreen 1 i386 green 3
             })
         self.assertEqual(self.pending_requests, '')
 
+    def test_new_runs_dont_clobber_pass(self):
+        '''passing once is sufficient
+
+        If a test succeeded once for a particular version and trigger,
+        subsequent failures (which might be triggered by other unstable
+        uploads) should not invalidate the PASS, as that new failure is the
+        fault of the new upload, not the original one.
+        '''
+        # new libc6 works fine with green
+        self.swift.set_results({'autopkgtest-series': {
+            'series/i386/g/green/20150101_100000@': (0, 'green 1', {'custom_environment': ['ADT_TEST_TRIGGERS=libc6/2']}),
+            'series/amd64/g/green/20150101_100000@': (0, 'green 1', {'custom_environment': ['ADT_TEST_TRIGGERS=libc6/2']}),
+        }})
+
+        self.do_test(
+            [('libc6', {'Version': '2'}, None)],
+            {'libc6': (True, {'green 1': {'amd64': 'PASS', 'i386': 'PASS'}})})
+        self.assertEqual(self.pending_requests, '')
+
+        # new green fails; that's not libc6's fault though, so it should stay
+        # valid
+        self.swift.set_results({'autopkgtest-series': {
+            'series/i386/g/green/20150101_100100@': (4, 'green 2', {'custom_environment': ['ADT_TEST_TRIGGERS=green/2']}),
+            'series/amd64/g/green/20150101_100100@': (4, 'green 2', {'custom_environment': ['ADT_TEST_TRIGGERS=green/2']}),
+        }})
+        self.do_test(
+            [('libgreen1', {'Version': '2', 'Source': 'green', 'Depends': 'libc6'}, 'autopkgtest')],
+            {'green': (False, {'green 2': {'amd64': 'REGRESSION', 'i386': 'REGRESSION'}}),
+             'libc6': (True, {'green 1': {'amd64': 'PASS', 'i386': 'PASS'}}),
+            })
+        self.assertEqual(
+            self.amqp_requests,
+            set(['debci-series-i386:darkgreen {"triggers": ["green/2"]}',
+                 'debci-series-amd64:darkgreen {"triggers": ["green/2"]}',
+                 'debci-series-i386:lightgreen {"triggers": ["green/2"]}',
+                 'debci-series-amd64:lightgreen {"triggers": ["green/2"]}',
+                ]))
+
     def test_remove_from_unstable(self):
         '''broken package gets removed from unstable'''