Switch logic of detecting failures to looking for failed tests in policy_info. Explicitly list all failing tests in the bug comment. Change wording.

sru-messages-fixes
Łukasz 'sil2100' Zemczak 6 years ago
parent 66036553ac
commit e51cc58a2b

@ -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

@ -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

Loading…
Cancel
Save