[Findit] Cross-platform triage

appengine/findit/handlers/triage_analysis.py

 # Copyright 2014 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """This module is to handle manual triage of analysis result.
 
 This handler will flag the analysis result as correct or incorrect.
 TODO: work on an automatic or semi-automatic way to triage analysis result.
 """
 
 import calendar
 from datetime import datetime
+from datetime import timedelta
 
 from google.appengine.api import users
 from google.appengine.ext import ndb
+import pytz.gae
 
 from common.base_handler import BaseHandler
 from common.base_handler import Permission
+from model import result_status
 from model.wf_analysis import WfAnalysis
-from model import result_status
 from waterfall import buildbot
 
 
-@ndb.transactional
-def _UpdateAnalysisResultStatus(
-    master_name, builder_name, build_number, correct, user_name=None):
-  analysis = WfAnalysis.Get(master_name, builder_name, build_number)
-  if not analysis or not analysis.completed:
+MATCHING_ANALYSIS_HOURS_AGO_START = 24
+MATCHING_ANALYSIS_HOURS_AGO_END = 24
+
+MATCHING_ANALYSIS_END_BOUND_TIME_ZONE = "US/Pacific"
+
+def _GenPotentialCulpritTupleList(analysis):
+  """Generates a list of potential culprit tuples.
+
+  Args:
+    analysis: the analysis from which to generate a potenial culript tuple list.
+
+  Returns:
+    A list of cultprit tuples that each could look like:
+
+        (step_name, test_name, revision)
+
+    or could look like:
+
+        (step_name, revision)
+  """
+  potential_culprit_tuple_list = []
+
+  # Iterates through the failures, tests, and suspected_cls, appending potential
+  # (step_name, test_name, revision) and (step_name, revision) culprit tuples to
+  # the list.
+  for failure in analysis.result['failures']:
+    if failure.get('tests'):
+      for test in failure['tests']:
+        for suspected_cl in test.get('suspected_cls', []):
+          potential_culprit_tuple_list.append((
+              failure['step_name'],
+              test['test_name'],
+              suspected_cl['revision']))
+    else:
+      for suspected_cl in failure['suspected_cls']:
+        potential_culprit_tuple_list.append((
+            failure['step_name'],
+            suspected_cl['revision']))
+
+  return potential_culprit_tuple_list
+
+
+def _DoAnalysesMatch(analysis_1, analysis_2):
+  """Checks if two analyses match.
+
+  Args:
+    analysis_1: The first analysis to compare.
+    analysis_2: The second analysis to compare.
+
+  Returns:
+    True if the two analyses' sorted potential culprit lists match, otherwise
+    False.
+  """
+
+  # Get list of potential culprit tuples.
+  potential_culprit_tuple_list_1 = _GenPotentialCulpritTupleList(analysis_1)
+  potential_culprit_tuple_list_2 = _GenPotentialCulpritTupleList(analysis_2)
+
+  # Both analyses must have non-empty potential culprit lists.
+  if not potential_culprit_tuple_list_1 or not potential_culprit_tuple_list_2:
     return False
 
-  if correct:
+  # Both analyses must have matching potential culprit lists.
+  return (sorted(potential_culprit_tuple_list_1) ==
+          sorted(potential_culprit_tuple_list_2))
+
+
+def _AppendTriageHistoryRecord(analysis, is_correct, user_name):
+  """Appends a triage history record to the given analysis.
+
+  Args:
+    analysis: The analysis to which to append the history record.
+    is_correct: True if the history record should indicate a correct judgement,
+        otherwise False.
+    user_name: The user_name of the person to include in the triage record.
+  """
+  if is_correct:
     if analysis.suspected_cls:
       analysis.result_status = result_status.FOUND_CORRECT
       analysis.culprit_cls = analysis.suspected_cls
     else:
       analysis.result_status = result_status.NOT_FOUND_CORRECT
       analysis.culprit_cls = None
   else:
     analysis.culprit_cls = None
     if analysis.suspected_cls:
       analysis.result_status = result_status.FOUND_INCORRECT
     else:
       analysis.result_status = result_status.NOT_FOUND_INCORRECT
 
   triage_record = {
       'triage_timestamp': calendar.timegm(datetime.utcnow().timetuple()),
       'user_name': user_name,
       'result_status': analysis.result_status,
       'version': analysis.version,
   }
   if not analysis.triage_history:
     analysis.triage_history = []
   analysis.triage_history.append(triage_record)
 
   analysis.put()
-  return True
+
+
+@ndb.transactional

[stgao, 2016/06/24 01:07:23]

This looks good to me.

+def _UpdateAnalysisResultStatus(
+    master_name, builder_name, build_number, is_correct, user_name=None):
+  analysis = WfAnalysis.Get(master_name, builder_name, build_number)
+  if not analysis or not analysis.completed:
+    return False, None
+
+  _AppendTriageHistoryRecord(analysis, is_correct, user_name)
+
+  return True, analysis
+
+
+def _GetDuplicateAnalyses(original_analysis):
+  start_time = (original_analysis.build_start_time -
+                timedelta(hours=MATCHING_ANALYSIS_HOURS_AGO_START))
+  end_time = (original_analysis.build_start_time +
+              timedelta(hours=MATCHING_ANALYSIS_HOURS_AGO_END))
+
+  # Don't count any analyses from today (except for exactly at midnight PST).
+  # Get current time (UTC)
+  current_time_as_utc = pytz.utc.localize(datetime.utcnow())

[stgao, 2016/06/24 01:07:23]

Just curious about the context on making the limit to midnight PST?

[lijeffrey, 2016/06/24 01:30:06]

isn't current_time_as_utc just datetime.utcnow()?

[chanli, 2016/06/24 16:51:56]

This is to copy what we're doing right now when we triage manually.

[josiahk, 2016/06/24 18:12:39]

No, because datetime.utcnow() doesn't contain timezone information:
| >>> dt = datetime.utcnow()
| >>> dt
| datetime.datetime(2016, 6, 24, 16, 16, 35, 416300)
| >>> pytz.utc.localize(dt)
| datetime.datetime(2016, 6, 24, 16, 16, 35, 416300, tzinfo=<UTC>)

+
+  # Convert to pacific time

[lijeffrey, 2016/06/24 01:30:06]

nit: comments end with .

[josiahk, 2016/06/24 18:12:39]

Done.

+  current_time_as_pacific = current_time_as_utc.astimezone(
+      pytz.timezone(MATCHING_ANALYSIS_END_BOUND_TIME_ZONE))

[lijeffrey, 2016/06/24 01:30:06]

is there a reason we're using pst and not utc?

[chanli, 2016/06/24 16:51:56]

This is to copy what we're doing right now when we triage manually.

+
+  # Set hours and minutes to 0 to get midnight
+  pacific_midnight_as_pacific = current_time_as_pacific.replace(
+      hour=0, minute=0, second=0, microsecond=0)
+
+  # Convert back to UTC time
+  pacific_midnight_as_utc = pacific_midnight_as_pacific.astimezone(pytz.utc)
+
+  # Strip timezone
+  pacific_midnight = pacific_midnight_as_utc.replace(tzinfo=None)
+
+  if end_time > pacific_midnight:
+    end_time = pacific_midnight
+
+  # Retrieve potential duplicate build analyses.
+  analysis_results = WfAnalysis.query(ndb.AND(
+      WfAnalysis.build_start_time >= start_time,
+      WfAnalysis.build_start_time <= end_time,
+      WfAnalysis.result_status == result_status.FOUND_UNTRIAGED
+      )).fetch()
+
+  # Further filter potential duplicates and return them.
+  return [analysis for analysis in analysis_results if
+          _DoAnalysesMatch(original_analysis, analysis) and
+          original_analysis.key is not analysis.key and
+          analysis.completed]
+
+
+def _TriageDuplicateResults(original_analysis, is_correct, user_name=None):
+  matching_analyses = _GetDuplicateAnalyses(original_analysis)
+
+  for analysis in matching_analyses:
+    _AppendTriageHistoryRecord(analysis, is_correct, user_name)
 
 
 class TriageAnalysis(BaseHandler):
   PERMISSION_LEVEL = Permission.CORP_USER
 
   def HandleGet(self):  # pragma: no cover
     return self.HandlePost()
 
   def HandlePost(self):
     """Sets the manual triage result for the analysis.
 
     Mark the analysis result as correct/wrong/etc.
     TODO: make it possible to set the real culprit CLs.
     """
     url = self.request.get('url').strip()
     build_info = buildbot.ParseBuildUrl(url)
     if not build_info:
       return {'data': {'success': False}}
     master_name, builder_name, build_number = build_info
 
-    correct = self.request.get('correct').lower() == 'true'
+    is_correct = self.request.get('correct').lower() == 'true'
     # As the permission level is CORP_USER, we could assume the current user
     # already logged in.
     user_name = users.get_current_user().email().split('@')[0]
-    success = _UpdateAnalysisResultStatus(
-        master_name, builder_name, build_number, correct, user_name)
+    success, original_analysis = _UpdateAnalysisResultStatus(
+        master_name, builder_name, build_number, is_correct, user_name)
+    if success:
+      _TriageDuplicateResults(original_analysis, is_correct, user_name)
     return {'data': {'success': success}}