Revert "Add chronological sorting to test results and key=value to --info."

This reverts commit ff94ab47bd69ce811ffdeb90e0d10a5802c7c0a6, which broke
the ability of generate_test_results.py to gather data on crashes.

Example failure:
  http://chromegw/i/chromeos/builders/link%20paladin/builds/9128

Traceback (most recent call last):
  File "/usr/bin/generate_test_report", line 631, in <module>
    main()
  File "/usr/bin/generate_test_report", line 627, in main
    generator.Run()
  File "/usr/bin/generate_test_report", line 568, in Run
    self._GenerateReportText()
  File "/usr/bin/generate_test_report", line 532, in _GenerateReportText
    crashes[crash].add(test)
NameError: global name 'test' is not defined

Original commit message:

Long suites have lengthy result lists and its hard to see how an
early failure can affect subsequent tests.  Add an option for sorting
test results in chronological order. Grabs a human-readable localtime
for grepping and a timestamp for sorting.

Also adds keys to --info csv data so that automated tools may find specific
info keys like bios version and ec version.

BUG=chromium-os:34919
TEST=generate_test_report --strip "/tmp/..." --sort-chron "/tmp/..."
TEST=multiple variations with/without --sort-cron, --csv --info and --no-debug.

Change-Id: Ia09bef1a5853bd70f49988352747268ed8d4c5d2
Reverted-Change-Id: I9839e55526c5871a0e28434e8434224a2223e8c7
Reviewed-on: https://gerrit.chromium.org/gerrit/34375
Reviewed-on: https://gerrit.chromium.org/gerrit/34844
Reviewed-by: David James <davidjames@chromium.org>
Tested-by: David James <davidjames@chromium.org>
diff --git a/utils_py/generate_test_report.py b/utils_py/generate_test_report.py
index 8de8e99..166ba95 100755
--- a/utils_py/generate_test_report.py
+++ b/utils_py/generate_test_report.py
@@ -12,12 +12,10 @@
 
 import datetime
 import glob
-import operator
 import optparse
 import os
 import re
 import sys
-import time
 
 import constants
 sys.path.append(constants.CROSUTILS_LIB_DIR)
@@ -172,36 +170,25 @@
     return info
 
   def _MakeResultKey(self, testdir):
-    """Helper to shorten directory for easier review of long printed lines.
+    """Parses keyval file under testdir.
 
-    WITHOUT --strip [all lines exceed 80 characters]
-
-    /tmp/run_remote_tests.MVyY/tmp.combined-control.cHThH
-                               [  FAILED  ]
-    /tmp/run_remote_tests.MVyY/tmp.combined-control.cHThH/firmware_SoftwareSync.
-    normal                     [  PASSED  ]
-    /tmp/run_remote_tests.MVyY/tmp.combined-control.cHThH/firmware_SoftwareSync.
-    normal/firmware_FAFTClient [  PASSED  ]
-
-    WITH --strip
-
-    /tmp/run_remote_tests.MVyY/tmp.combined-control.cHThH [  FAILED  ]
-    firmware_SoftwareSync.normal                          [  PASSED  ]
-    firmware_SoftwareSync.normal/firmware_FAFTClient      [  PASSED  ]
+    If testdir contains a result folder, process the keyval file and return
+    a dictionary of perf keyval pairs.
 
     Args:
       testdir: The autoserv test result directory.
 
     Returns:
-      If the option for shortening the directory is provided, truncates the
-      long directory path for a cleaner output report.
+      If the perf option is disabled or the there's no keyval file under
+      testdir, returns an empty dictionary. Otherwise, returns a dictionary of
+      parsed keyvals. Duplicate keys are uniquified by their instance number.
     """
     if testdir.startswith(self._strip_text):
       return testdir.replace(self._strip_text, '', 1)
     return testdir
 
   def _CollectResult(self, testdir, results):
-    """Collects results stored under testdir into a dictionary.
+    """Adds results stored under testdir to the self._results dictionary.
 
     The presence/location of status files (status.log, status and
     job_report.html) varies depending on whether the job is a simple
@@ -224,14 +211,13 @@
     files.  Parent and subdirectory 'status.log' files are always expected
     to agree on the outcome of a given test.
 
-    The test results discovered from the 'status*' files are included
-    in the result dictionary.  The test directory name and a test directory
-    timestamp/localtime are saved to be used as sort keys for the results.
+    The test results discovered from the 'status*' files are added/udpated
+    into the self._results dictionary.  The test directory name is used as
+    a key into the results dictionary.
 
     Args:
       testdir: The autoserv test result directory.
-      results: A list to which a populated test-result-dictionary will
-               be appended if a status file is found.
+      results: Results dictionary to store results in.
     """
     status_file = os.path.join(testdir, 'status.log')
     if not os.path.isfile(status_file):
@@ -256,36 +242,22 @@
           reason = re.escape(reason)
         error_msg = ': '.join([failure_type, reason])
 
-    # Grab the timestamp - it can be used for sorting the test runs.
     # Grab the localtime - it may be printed to enable line filtering by date.
-    test_timestamp = ''
     test_localtime = ''
-    # Designed to match a line like this:
-    #   END GOOD  test_name ... timestamp=1347324321  localtime=Sep 10 17:45:21
-    status_re = 'GOOD|%s' % failures
-    localtime_re = '\w+\s+\w+\s+[:\w]+'
-    matches = re.findall(r'^\s*END\s+(%s).*timestamp=(\d*).*localtime=(%s).*$'
-                         % (status_re, localtime_re), status_raw, re.MULTILINE)
-    if matches:
-      # There may be multiple lines with timestamp/localtime info.
-      # The last one found is selected because it will reflect the end time.
-      # Sometimes there is superfluous data following the localtime. Therefore,
-      # the first 3 space-delimited fields are selected to ignore the rest.
-      _, test_timestamp, test_localtime = matches[-1]
+    match = re.search(r'^\s*END\s+(GOOD|%s).*localtime=(.*)$' % failures,
+                      status_raw, re.MULTILINE)
+    if match:
+      test_localtime = str(match.group(2).strip())
 
-    results.append({
-        'testdir': self._MakeResultKey(testdir),
+    results[self._MakeResultKey(testdir)] = {
         'crashes': self._CollectCrashes(status_raw),
         'status': status,
         'error_msg': error_msg,
-        'localtime': test_localtime,
-        'timestamp': test_timestamp,
         'perf': self._CollectPerf(testdir),
-        'info': self._CollectInfo(testdir, {'localtime': test_localtime,
-                                            'timestamp': test_timestamp})})
+        'info': self._CollectInfo(testdir, {'localtime': test_localtime})}
 
-  def RecursivelyCollectResults(self, resdir):
-    """Recursively collect results into a list of dictionaries.
+  def CollectResults(self, resdir):
+    """Recursively collect results into a dictionary.
 
     Only recurses into directories that possess a 'debug' subdirectory
     because anything else is not considered a 'test' directory.
@@ -294,16 +266,16 @@
       resdir: results/test directory to parse results from and recurse into.
 
     Returns:
-      List of dictionaries of results.
+      Dictionary of results.
     """
-    results = []
+    results = {}
     self._CollectResult(resdir, results)
     for testdir in glob.glob(os.path.join(resdir, '*')):
       # Remove false positives that are missing a debug dir.
       if not os.path.exists(os.path.join(testdir, 'debug')):
         continue
 
-      results.extend(self.RecursivelyCollectResults(testdir))
+      results.update(self.CollectResults(testdir))
     return results
 
 
@@ -323,22 +295,20 @@
     self._args = args
     self._color = Color(options.color)
 
-  def _CollectAllResults(self):
-    """Parses results into the self._results list.
+  def _CollectResults(self):
+    """Parses results into the self._results dictionary.
 
-    Builds a list (self._results) where each entry is a dictionary of result
-    data from one test (which may contain other tests). Each dictionary will
-    contain values such as: test folder, status, localtime, crashes, error_msg,
-    perf keyvals [optional], info [optional].
+    Initializes a dictionary (self._results) with test folders as keys and
+    result data (status, perf keyvals) as values.
     """
-    self._results = []
+    self._results = {}
     collector = ResultCollector(self._options.perf, self._options.info,
                                 self._options.escape_error, self._options.strip,
                                 self._options.whitelist_chrome_crashes)
     for resdir in self._args:
       if not os.path.isdir(resdir):
         Die('\'%s\' does not exist' % resdir)
-      self._results.extend(collector.RecursivelyCollectResults(resdir))
+      self._results.update(collector.CollectResults(resdir))
 
     if not self._results:
       Die('no test directories found')
@@ -350,7 +320,6 @@
 
     Args:
       status: True or False, indicating success or failure.
-
     Returns:
       The appropriate string for printing..
     """
@@ -366,16 +335,16 @@
   def _GetTestColumnWidth(self):
     """Returns the test column width based on the test data.
 
-    The test results are aligned by discovering the longest width test
-    directory name or perf key stored in the list of result dictionaries.
+    Aligns the test results by formatting the test directory entry based on
+    the longest test directory or perf key string stored in the self._results
+    dictionary.
 
     Returns:
       The width for the test column.
     """
-    width = 0
-    for result in self._results:
-      width = max(width, len(result['testdir']))
-      perf = result.get('perf')
+    width = len(max(self._results, key=len))
+    for result in self._results.values():
+      perf = result['perf']
       if perf:
         perf_key_width = len(max(perf, key=len))
         width = max(width, perf_key_width + self._KEYVAL_INDENT)
@@ -451,31 +420,7 @@
         key_entry = dict_key.ljust(width - self._KEYVAL_INDENT)
         key_entry = key_entry.rjust(width)
       value_entry = self._color.Color(Color.BOLD, result_dict[dict_key])
-      if self._options.csv:
-        self._PrintEntries([test_entry, '%s=%s' % (key_entry, value_entry)])
-      else:
-        self._PrintEntries([test_entry, key_entry, value_entry])
-
-  def _GetSortedTests(self):
-    """Sort the test result dictionaries in preparation for results printing.
-
-    By default sorts the results directionaries by their test names.
-    However, when running long suites, it is useful to see if an early test
-    has wedged the system and caused the remaining tests to abort/fail. The
-    datetime-based chronological sorting allows this view.
-
-    Uses the --sort-chron command line option to control.
-    """
-    if self._options.sort_chron:
-      # Need to reverse sort the test dirs to ensure the suite folder shows
-      # at the bottom. Because the suite folder shares its datetime with the
-      # last test it shows second-to-last without the reverse sort first.
-      tests = sorted(self._results, key=operator.itemgetter('testdir'),
-                     reverse=True)
-      tests = sorted(tests, key=operator.itemgetter('timestamp'))
-    else:
-      tests = sorted(self._results, key=operator.itemgetter('testdir'))
-    return tests
+      self._PrintEntries([test_entry, key_entry, value_entry])
 
   def _GenerateReportText(self):
     """Prints a result report to stdout.
@@ -485,17 +430,20 @@
     enabled, each test entry is followed by perf keyval entries from the test
     results.
     """
-    tests = self._GetSortedTests()
+
+    tests = self._results.keys()
+    tests.sort()
+
     width = self._GetTestColumnWidth()
 
     crashes = {}
     tests_pass = 0
     self._PrintDashLine(width)
 
-    for result in tests:
-      testdir = result['testdir']
-      test_entry = testdir if self._options.csv else testdir.ljust(width)
+    for test in tests:
+      test_entry = test if self._options.csv else test.ljust(width)
 
+      result = self._results[test]
       status_entry = self._GenStatusString(result['status'])
       if result['status']:
         color = Color.GREEN
@@ -508,17 +456,16 @@
       info = result.get('info', {})
       if self._options.csv and self._options.info:
         if info:
-          test_entries.extend(['%s=%s' % (k, info[k])
-                               for k in sorted(info.keys())])
+          test_entries.extend([info[k] for k in sorted(info.keys())])
         if not result['status'] and result['error_msg']:
-          test_entries.append('reason="%s"' % result['error_msg'])
+          test_entries.append('"%s"' % result['error_msg'])
 
       self._PrintEntries(test_entries)
       self._PrintErrors(test_entry, result['error_msg'])
 
       # Print out error log for failed tests.
       if not result['status']:
-        self._PrintErrorLogs(testdir, test_entry)
+        self._PrintErrorLogs(test, test_entry)
 
       # Emit the perf keyvals entries. There will be no entries if the
       # --no-perf option is specified.
@@ -564,10 +511,10 @@
 
   def Run(self):
     """Runs report generation."""
-    self._CollectAllResults()
+    self._CollectResults()
     self._GenerateReportText()
-    for d in self._results:
-      if not d['status'] or (self._options.crash_detection and d['crashes']):
+    for v in self._results.itervalues():
+      if not v['status'] or (self._options.crash_detection and v['crashes']):
         sys.exit(1)
 
 
@@ -602,9 +549,6 @@
                     ' [default: \'%default\']')
   parser.add_option('--no-strip', dest='strip', const='', action='store_const',
                     help='Don\'t strip a prefix from test directory names')
-  parser.add_option('--sort-chron', dest='sort_chron', action='store_true',
-                    default=False,
-                    help='Sort results by datetime instead of by test name.')
   parser.add_option('--no-debug', dest='print_debug', action='store_false',
                     default=True,
                     help='Don\'t print out logs when tests fail.')