Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1618)

Unified Diff: appengine/findit/waterfall/identify_try_job_culprit_pipeline.py

Issue 2302223002: [Findit] Change the criteria of sending notification to code review. (Closed)
Patch Set: . Created 4 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: appengine/findit/waterfall/identify_try_job_culprit_pipeline.py
diff --git a/appengine/findit/waterfall/identify_try_job_culprit_pipeline.py b/appengine/findit/waterfall/identify_try_job_culprit_pipeline.py
index 12be524fe7fe94d697e053b80b743b824dd5c3b3..d2dabf93e5037a569d6eb3d1634b06aff73df4c1 100644
--- a/appengine/findit/waterfall/identify_try_job_culprit_pipeline.py
+++ b/appengine/findit/waterfall/identify_try_job_culprit_pipeline.py
@@ -182,16 +182,63 @@ def _GetCulpritsForTestsFromResultsDict(blame_list, test_results):
return culprit_map, list(failed_revisions)
-def _NotifyCulprits(master_name, builder_name, build_number, culprits):
- """Sends notifications to the identified culprits."""
+def _GetSuspectedCLFoundByHeuristicForCompile(analysis):
+ """For compile failure, gets the suspected revision found by heuristic."""
+ if not analysis or not analysis.result:
+ return None
+
+ for failure in analysis.result.get('failures', []):
+ if (failure['step_name'].lower() == 'compile' and
+ len(failure['suspected_cls']) == 1):
+ # Based on confidence calculation, suspected_cl found by heuristic for
+ # compile is very likely to be the culprit.
+ # Since the current confidence calculation is for results with single
+ # suspected_cl, we might need to have the same regulation here.
+ return failure['suspected_cls'][0]
+ return None
+
+
+def _GetHeuristicSuspectedCLs(analysis):
+ """Gets revisions of suspected cls found by heuristic approach."""
+ if analysis and analysis.suspected_cls:
+ return [(cl['repo_name'], cl['revision']) for cl in analysis.suspected_cls]
+ return []
+
+
+def _StartSendNotificationPipeline(
+ master_name, builder_name, build_number, repo_name, revision,
+ send_notification_right_now):
try:
- for culprit in (culprits or {}).itervalues():
- pipeline = SendNotificationForCulpritPipeline(
- master_name, builder_name, build_number,
- culprit['repo_name'], culprit['revision'])
- pipeline.start()
+ pipeline = SendNotificationForCulpritPipeline(
+ master_name, builder_name, build_number, repo_name, revision,
lijeffrey1 2016/09/06 16:53:56 nit: 4 spaces instead of 2
chanli 2016/09/06 17:12:42 Done.
+ send_notification_right_now)
+ pipeline.start()
except Exception: # pragma: no cover.
- logging.exception('Failed to notify culprits.')
+ logging.exception(
+ 'Failed to notify culprit.')
lijeffrey1 2016/09/06 16:53:56 nit: Keep this on 1 line
chanli 2016/09/06 17:12:42 Done.
+
+
+def _NotifyCulprits(master_name, builder_name, build_number, culprits,
+ heuristic_cls, compile_suspected_cl):
+ """Sends notifications to the identified culprits."""
+
+ if culprits:
+ # There is a try job result, so check if any of the culprits
+ # was also found by heuristic analysis.
+ for culprit in culprits.itervalues():
+ send_notification_right_now = False
+ if (culprit['repo_name'], culprit['revision']) in heuristic_cls:
+ send_notification_right_now = True
+ _StartSendNotificationPipeline(
+ master_name, builder_name, build_number,
+ culprit['repo_name'], culprit['revision'],
+ send_notification_right_now)
+ elif compile_suspected_cl:
+ # Need to check if the failure is compile and heuristic found a culprit.
+ _StartSendNotificationPipeline(
+ master_name, builder_name, build_number,
+ compile_suspected_cl['repo_name'], compile_suspected_cl['revision'],
+ send_notification_right_now=True)
class IdentifyTryJobCulpritPipeline(BasePipeline):
@@ -324,7 +371,6 @@ class IdentifyTryJobCulpritPipeline(BasePipeline):
# Update analysis result and suspected CLs with results of this try job if
# culprits were found.
- analysis = WfAnalysis.Get(master_name, builder_name, build_number)
updated_result_status = _GetResultAnalysisStatus(analysis, result)
updated_suspected_cls = _GetSuspectedCLs(analysis, result)
@@ -336,8 +382,16 @@ class IdentifyTryJobCulpritPipeline(BasePipeline):
# Store try-job results.
UpdateTryJobResult()
+
+ analysis = WfAnalysis.Get(master_name, builder_name, build_number)
+ heuristic_cls = _GetHeuristicSuspectedCLs(analysis)
+ compile_suspected_cl = (
+ _GetSuspectedCLFoundByHeuristicForCompile(analysis)
+ if try_job_type == failure_type.COMPILE else None)
+
# Add try-job results to WfAnalysis.
UpdateWfAnalysisWithTryJobResult()
- _NotifyCulprits(master_name, builder_name, build_number, culprits)
- return result.get('culprit') if result else None
+ _NotifyCulprits(master_name, builder_name, build_number, culprits,
+ heuristic_cls, compile_suspected_cl)
+ return result.get('culprit') if result else None

Powered by Google App Engine
This is Rietveld 408576698