Skip to content

Commit 7deb26b

Browse files
Assign predator bug component id even when missing suspected components (#5065)
Recently, a possible issue in predator was making testcases miss the `suspected_components` field in predator result, even though the correct `suspected_buganizer_component_id` was set. However, the logic in cleanup was preventing the bugs to be assigned with this component ID. This PR fixes by allowing this case. Context: b/464899796 --------- Co-authored-by: Titouan Rigoudy <[email protected]>
1 parent 919a946 commit 7deb26b

File tree

2 files changed

+59
-6
lines changed

2 files changed

+59
-6
lines changed

src/clusterfuzz/_internal/cron/cleanup.py

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1100,6 +1100,14 @@ def update_fuzz_blocker_label(policy, testcase, issue,
11001100
issue.save(new_comment=update_message, notify=True)
11011101

11021102

1103+
def _should_update_component_id(issue, component_id):
1104+
"""Check whether updating the issue component id is needed."""
1105+
if not hasattr(issue, 'component_id'):
1106+
return False
1107+
1108+
return component_id and issue.component_id != component_id
1109+
1110+
11031111
def update_component_labels_and_id(policy, testcase, issue):
11041112
"""Add components to the issue if needed."""
11051113
if not issue:
@@ -1132,9 +1140,10 @@ def update_component_labels_and_id(policy, testcase, issue):
11321140
if not found_component_in_issue:
11331141
filtered_components.append(component)
11341142

1135-
if not filtered_components:
1136-
# If there are no new components to add, then we shouldn't make any changes
1137-
# to issue.
1143+
if not (filtered_components or
1144+
_should_update_component_id(issue, component_id)):
1145+
# If there are no new components to add and neither a component ID to
1146+
# update, then we shouldn't make any changes to issue.
11381147
return
11391148

11401149
# Don't run on issues we've already applied automatic components to in case

src/clusterfuzz/_internal/tests/appengine/handlers/cron/cleanup_test.py

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1450,22 +1450,66 @@ def test_components_added(self):
14501450
self.assertIn('Test-Predator-Auto-Components', self.issue.labels)
14511451

14521452
def test_component_id_added(self):
1453-
"""Ensure that we a component_id when applicable."""
1453+
"""Ensure that we set a component_id when applicable."""
14541454
self.testcase.set_metadata(
14551455
'predator_result', {
14561456
'result': {
14571457
'suspected_components': ['A', 'B>C'],
1458-
'suspected_buganizer_component_id': 123456,
1458+
'suspected_buganizer_component_id': '123456',
14591459
}
14601460
})
14611461

14621462
cleanup.update_component_labels_and_id(self.policy, self.testcase,
14631463
self.issue)
14641464
self.assertIn('A', self.issue.components)
14651465
self.assertIn('B>C', self.issue.components)
1466-
self.assertEqual(123456, self.issue.component_id)
1466+
self.assertEqual('123456', self.issue.component_id)
14671467
self.assertIn('Test-Predator-Auto-Components', self.issue.labels)
14681468

1469+
def test_component_id_without_suspected_components(self):
1470+
"""Ensure we set a component_id even when missing suspected components."""
1471+
self.testcase.set_metadata(
1472+
'predator_result',
1473+
{'result': {
1474+
'suspected_buganizer_component_id': '123456'
1475+
}})
1476+
1477+
cleanup.update_component_labels_and_id(self.policy, self.testcase,
1478+
self.issue)
1479+
self.assertEqual('123456', self.issue.component_id)
1480+
self.assertIn('Test-Predator-Auto-Components', self.issue.labels)
1481+
1482+
def test_component_id_changed(self):
1483+
"""Ensure we update the component_id if it has changed."""
1484+
self.testcase.set_metadata(
1485+
'predator_result',
1486+
{'result': {
1487+
'suspected_buganizer_component_id': '111111'
1488+
}})
1489+
setattr(self.issue, 'component_id', '123456')
1490+
cleanup.update_component_labels_and_id(self.policy, self.testcase,
1491+
self.issue)
1492+
self.assertEqual('111111', self.issue.component_id)
1493+
self.assertIn('Test-Predator-Auto-Components', self.issue.labels)
1494+
1495+
def test_component_id_already_set(self):
1496+
"""Ensure we do not make an API call if the component ID has not changed."""
1497+
helpers.patch(
1498+
self,
1499+
['clusterfuzz._internal.issue_management.monorail.issue.Issue.save'])
1500+
self.testcase.set_metadata(
1501+
'predator_result',
1502+
{'result': {
1503+
'suspected_buganizer_component_id': '123456'
1504+
}})
1505+
setattr(self.issue, 'component_id', '123456')
1506+
1507+
cleanup.update_component_labels_and_id(self.policy, self.testcase,
1508+
self.issue)
1509+
self.assertNotIn('Test-Predator-Auto-Components', self.issue.labels)
1510+
self.assertEqual('123456', self.issue.component_id)
1511+
self.mock.save.assert_not_called()
1512+
14691513
def test_components_not_reapplied(self):
14701514
"""Ensure that we don't re-add components once applied."""
14711515
self.testcase.set_metadata(

0 commit comments

Comments
 (0)