diff --git a/britney2/policies/sruadtregression.py b/britney2/policies/sruadtregression.py index e4217ef..d08e753 100644 --- a/britney2/policies/sruadtregression.py +++ b/britney2/policies/sruadtregression.py @@ -3,6 +3,7 @@ import json import socket import smtplib +from collections import defaultdict from urllib.request import urlopen, URLError from britney2.policies.rest import Rest @@ -14,9 +15,17 @@ To: {bug_mail} Subject: Autopkgtest regression report ({source_name}/{version}) All autopkgtests for the newly accepted {source_name} ({version}) for {series_name} have finished running. -There have been regressions in tests triggered by the package. Please visit the sru report page and investigate the failures. +The following regressions have been reported in tests triggered by the package: -https://people.canonical.com/~ubuntu-archive/pending-sru.html#{series_name} +{failures} + +Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1]. + +https://people.canonical.com/~ubuntu-archive/proposed-migration/{series_name}/update_excuses.html#{source_name} + +[1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions + +Thank you! """ @@ -60,13 +69,25 @@ class SRUADTRegressionPolicy(BasePolicy, Rest): return bugs def apply_policy_impl(self, policy_info, suite, source_name, source_data_tdist, source_data_srcdist, excuse): - # If all the autopkgtests have finished running. - if (excuse.current_policy_verdict == PolicyVerdict.REJECTED_TEMPORARILY or - excuse.current_policy_verdict == PolicyVerdict.PASS_HINTED): - return PolicyVerdict.PASS # We only care about autopkgtest regressions if 'autopkgtest' not in excuse.reason or not excuse.reason['autopkgtest']: return PolicyVerdict.PASS + # Check the policy_info to see if all tests finished and, if yes, if we + # have any failures to report on. + if not policy_info or 'autopkgtest' not in policy_info: + return PolicyVerdict.PASS + regressions = defaultdict(list) + for pkg in policy_info['autopkgtest']: + for arch in policy_info['autopkgtest'][pkg]: + if policy_info['autopkgtest'][pkg][arch][0] == 'RUNNING': + # If there's at least one test still running, we just stop + # and not proceed any further. + return PolicyVerdict.PASS + elif policy_info['autopkgtest'][pkg][arch][0] == 'REGRESSION': + regressions[pkg].append(arch) + if not regressions: + return PolicyVerdict.PASS + version = source_data_srcdist.version distro_name = self.options.distribution series_name = self.options.series @@ -90,8 +111,7 @@ class SRUADTRegressionPolicy(BasePolicy, Rest): }) try: src = next(iter(data['entries'])) - # IndexError means no packages in -proposed matched this name/version, - # which is expected to happen when bileto runs britney. + # IndexError means no packages in -proposed matched this name/version. except StopIteration: self.log('No packages matching %s/%s the %s/%s main archive, not ' 'informing of ADT regressions' % ( @@ -102,6 +122,10 @@ class SRUADTRegressionPolicy(BasePolicy, Rest): }) if not changes_url: return PolicyVerdict.PASS + # Prepare a helper string that lists all the ADT failures + failures = '' + for pkg, arches in regressions.items(): + failures += '%s (%s)' % (pkg, ', '.join(arches)) bugs = self.bugs_from_changes(changes_url) # Now leave a comment informing about the ADT regressions on each bug diff --git a/tests/test_sruadtregression.py b/tests/test_sruadtregression.py index 254d7d8..82891e6 100755 --- a/tests/test_sruadtregression.py +++ b/tests/test_sruadtregression.py @@ -68,6 +68,22 @@ class FakeExcuse: daysold = 0 reason = {'autopkgtest': 1} current_policy_verdict = PolicyVerdict.REJECTED_PERMANENTLY + policy_info = { + 'autopkgtest': { + 'testpackage/55.0': { + 'amd64': ['PASS', None, None, None, None], + 'i386': ['REGRESSION', None, None, None, None], + }, + 'rdep1/1.1-0ubuntu1': { + 'amd64': ['REGRESSION', None, None, None, None], + 'i386': ['REGRESSION', None, None, None, None], + }, + 'rdep2/0.1-0ubuntu2': { + 'amd64': ['PASS', None, None, None, None], + 'i386': ['PASS', None, None, None, None], + }, + } + } class FakeExcuseRunning: @@ -75,6 +91,22 @@ class FakeExcuseRunning: daysold = 0 reason = {'autopkgtest': 1} current_policy_verdict = PolicyVerdict.REJECTED_TEMPORARILY + policy_info = { + 'autopkgtest': { + 'testpackage/55.0': { + 'amd64': ['PASS', None, None, None, None], + 'i386': ['REGRESSION', None, None, None, None], + }, + 'rdep1/1.1-0ubuntu1': { + 'amd64': ['RUNNING', None, None, None, None], + 'i386': ['PASS', None, None, None, None], + }, + 'rdep2/0.1-0ubuntu2': { + 'amd64': ['PASS', None, None, None, None], + 'i386': ['PASS', None, None, None, None], + }, + } + } class FakeExcusePass: @@ -82,6 +114,22 @@ class FakeExcusePass: daysold = 0 reason = {} current_policy_verdict = PolicyVerdict.PASS + policy_info = { + 'autopkgtest': { + 'testpackage/55.0': { + 'amd64': ['PASS', None, None, None, None], + 'i386': ['PASS', None, None, None, None], + }, + 'rdep1/1.1-0ubuntu1': { + 'amd64': ['PASS', None, None, None, None], + 'i386': ['PASS', None, None, None, None], + }, + 'rdep2/0.1-0ubuntu2': { + 'amd64': ['PASS', None, None, None, None], + 'i386': ['PASS', None, None, None, None], + }, + } + } class FakeExcuseHinted: @@ -89,6 +137,45 @@ class FakeExcuseHinted: daysold = 0 reason = {'skiptest': 1} current_policy_verdict = PolicyVerdict.PASS_HINTED + policy_info = { + 'autopkgtest': { + 'testpackage/55.0': { + 'amd64': ['PASS', None, None, None, None], + 'i386': ['ALWAYSFAIL', None, None, None, None], + }, + 'rdep1/1.1-0ubuntu1': { + 'amd64': ['IGNORE-FAIL', None, None, None, None], + 'i386': ['IGNORE-FAIL', None, None, None, None], + }, + 'rdep2/0.1-0ubuntu2': { + 'amd64': ['PASS', None, None, None, None], + 'i386': ['PASS', None, None, None, None], + }, + } + } + + +class FakeExcuseVerdictOverriden: + is_valid = True + daysold = 0 + reason = {'skiptest': 1} + current_policy_verdict = PolicyVerdict.REJECTED_PERMANENTLY + policy_info = { + 'autopkgtest': { + 'testpackage/55.0': { + 'amd64': ['PASS', None, None, None, None], + 'i386': ['PASS', None, None, None, None], + }, + 'rdep1/1.1-0ubuntu1': { + 'amd64': ['PASS', None, None, None, None], + 'i386': ['PASS', None, None, None, None], + }, + 'rdep2/0.1-0ubuntu2': { + 'amd64': ['PASS', None, None, None, None], + 'i386': ['PASS', None, None, None, None], + }, + } + } class T(unittest.TestCase): @@ -145,10 +232,11 @@ class T(unittest.TestCase): } } } + excuse = FakeExcuse pol = SRUADTRegressionPolicy(options, {}) # Set a base state pol.state = previous_state - status = pol.apply_policy_impl(None, None, 'testpackage', None, FakeSourceData, FakeExcuse) + status = pol.apply_policy_impl(excuse.policy_info, None, 'testpackage', None, FakeSourceData, excuse) self.assertEqual(status, PolicyVerdict.PASS) # Assert that we were looking for the right package as per # FakeSourceData contents @@ -173,6 +261,13 @@ class T(unittest.TestCase): call('localhost:1337') ]) self.assertEqual(smtp().sendmail.call_count, 2) + # call_args is a tuple (args, kwargs), and the email content is + # the third non-named argument of sendmail() - hence [0][2] + message = smtp().sendmail.call_args[0][2] + # Check if the e-mail/comment we were sending includes info about + # which tests have failed + self.assertIn('testpackage/55.0 (i386)', message) + self.assertIn('rdep1/1.1-0ubuntu1 (amd64, i386)', message) # Check if the state has been saved and not overwritten expected_state = { 'testbuntu': { @@ -210,8 +305,9 @@ class T(unittest.TestCase): lp.return_value = {'entries': [pkg_mock]} + excuse = FakeExcuseRunning pol = SRUADTRegressionPolicy(options, {}) - status = pol.apply_policy_impl(None, None, 'testpackage', None, FakeSourceData, FakeExcuseRunning) + status = pol.apply_policy_impl(excuse.policy_info, None, 'testpackage', None, FakeSourceData, excuse) self.assertEqual(status, PolicyVerdict.PASS) bugs_from_changes.assert_not_called() lp.assert_not_called() @@ -230,8 +326,9 @@ class T(unittest.TestCase): bugs_from_changes.return_value = {'entries': [pkg_mock]} + excuse = FakeExcusePass pol = SRUADTRegressionPolicy(options, {}) - status = pol.apply_policy_impl(None, None, 'testpackage', None, FakeSourceData, FakeExcusePass) + status = pol.apply_policy_impl(excuse.policy_info, None, 'testpackage', None, FakeSourceData, excuse) self.assertEqual(status, PolicyVerdict.PASS) bugs_from_changes.assert_not_called() lp.assert_not_called() @@ -249,8 +346,9 @@ class T(unittest.TestCase): pkg_mock.self_link = 'https://api.launchpad.net/1.0/ubuntu/+archive/primary/+sourcepub/9870565' bugs_from_changes.return_value = {'entries': [pkg_mock]} + excuse = FakeExcuseHinted pol = SRUADTRegressionPolicy(options, {}) - status = pol.apply_policy_impl(None, None, 'testpackage', None, FakeSourceData, FakeExcuseHinted) + status = pol.apply_policy_impl(excuse.policy_info, None, 'testpackage', None, FakeSourceData, excuse) self.assertEqual(status, PolicyVerdict.PASS) bugs_from_changes.assert_not_called() lp.assert_not_called() @@ -276,10 +374,38 @@ class T(unittest.TestCase): } }, } + excuse = FakeExcuse pol = SRUADTRegressionPolicy(options, {}) # Set a base state pol.state = previous_state - status = pol.apply_policy_impl(None, None, 'testpackage', None, FakeSourceData, FakeExcuse) + status = pol.apply_policy_impl(excuse.policy_info, None, 'testpackage', None, FakeSourceData, excuse) + self.assertEqual(status, PolicyVerdict.PASS) + bugs_from_changes.assert_not_called() + lp.assert_not_called() + smtp.sendmail.assert_not_called() + + @patch('smtplib.SMTP') + @patch('britney2.policies.sruadtregression.SRUADTRegressionPolicy.bugs_from_changes', return_value={1, 2}) + @patch('britney2.policies.sruadtregression.SRUADTRegressionPolicy.query_lp_rest_api') + def test_no_comment_if_verdict_overriden(self, lp, bugs_from_changes, smtp): + """Make sure the previous 'bug' of commenting on uploads that had no + failing tests (or had them still running) because of the + current_policy_verdict getting reset to REJECTED_PERMANENTLY + by some other policy does not happen anymore.""" + with TemporaryDirectory() as tmpdir: + options = FakeOptions + options.unstable = tmpdir + pkg_mock = Mock() + pkg_mock.self_link = 'https://api.launchpad.net/1.0/ubuntu/+archive/primary/+sourcepub/9870565' + + lp.return_value = {'entries': [pkg_mock]} + + # This should no longer be a valid bug as we switched how we + # actually determine if a regression is present, but still + # - better to have an explicit test case for it. + excuse = FakeExcuseVerdictOverriden + pol = SRUADTRegressionPolicy(options, {}) + status = pol.apply_policy_impl(excuse.policy_info, None, 'testpackage', None, FakeSourceData, excuse) self.assertEqual(status, PolicyVerdict.PASS) bugs_from_changes.assert_not_called() lp.assert_not_called() @@ -363,10 +489,11 @@ class T(unittest.TestCase): } } } + excuse = FakeExcuse pol = SRUADTRegressionPolicy(options, {}, dry_run=True) # Set a base state pol.state = previous_state - status = pol.apply_policy_impl(None, None, 'testpackage', None, FakeSourceData, FakeExcuse) + status = pol.apply_policy_impl(excuse.policy_info, None, 'testpackage', None, FakeSourceData, excuse) self.assertEqual(status, PolicyVerdict.PASS) # Assert that we were looking for the right package as per # FakeSourceData contents