[tools/perf] Support test path formats in json output formatter

This CL uses correct test path format to extract benchmark and story names
from the test path. It also removes the url unquoting, since we no longer
want to encode test paths in Telemetry.

Bug: 981349
Change-Id: I87957778fc832e06afad13b6ebad5e2d548211bc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1901085
Commit-Queue: Mikhail Khokhlov <khokhlov@google.com>
Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713447}

tools/perf/core/results_processor/command_line.py

@@ -17,10 +17,7 @@ import sys
 from py_utils import cloud_storage
 
 from core.results_processor import formatters
-
-
-TELEMETRY_TEST_PATH_FORMAT = 'telemetry'
-GTEST_TEST_PATH_FORMAT = 'gtest'
+from core.results_processor import util
 
 
 def ArgumentParser(standalone=False):
@@ -66,8 +63,8 @@ def ArgumentParser(standalone=False):
       help='Label to identify the results generated by this run.')
   group.add_argument(
       '--test-path-format', metavar='FORMAT',
-      choices=[TELEMETRY_TEST_PATH_FORMAT, GTEST_TEST_PATH_FORMAT],
-      default=TELEMETRY_TEST_PATH_FORMAT,
+      choices=[util.TELEMETRY_TEST_PATH_FORMAT, util.GTEST_TEST_PATH_FORMAT],
+      default=util.TELEMETRY_TEST_PATH_FORMAT,
       help=Sentences(
           'How to interpret the testPath attribute.',
           'Available options: %(choices)s. Default: %(default)s.'))

tools/perf/core/results_processor/formatters/json3_output.py

@@ -12,7 +12,6 @@ import collections
 import datetime
 import json
 import os
-import urllib
 
 from core.results_processor import util
 
@@ -22,14 +21,14 @@ OUTPUT_FILENAME = 'test-results.json'
 
 def ProcessIntermediateResults(test_results, options):
   """Process intermediate results and write output in output_dir."""
-  results = Convert(test_results, options.output_dir)
+  results = Convert(test_results, options.output_dir, options.test_path_format)
   output_file = os.path.join(options.output_dir, OUTPUT_FILENAME)
   with open(output_file, 'w') as f:
     json.dump(results, f, sort_keys=True, indent=4, separators=(',', ': '))
   return output_file
 
 
-def Convert(test_results, base_dir):
+def Convert(test_results, base_dir, test_path_format):
   """Convert intermediate results to the JSON Test Results Format.
 
   Args:
@@ -44,8 +43,7 @@ def Convert(test_results, base_dir):
   status_counter = collections.Counter()
 
   for result in test_results:
-    benchmark_name, story_name = result['testPath'].split('/')
-    story_name = urllib.unquote(story_name)
+    benchmark_name, story_name = util.SplitTestPath(result, test_path_format)
     actual_status = result['status']
     expected_status = actual_status if result['expected'] else 'PASS'
     status_counter[actual_status] += 1

tools/perf/core/results_processor/formatters/json3_output_unittest.py

@@ -12,10 +12,12 @@ from core.results_processor import testing
 class Json3OutputTest(unittest.TestCase):
   def setUp(self):
     self.base_dir = 'base_dir'
+    self.test_path_format = 'telemetry'
 
   def Convert(self, test_results):
     test_results_copy = copy.deepcopy(test_results)
-    results = json3_output.Convert(test_results_copy, self.base_dir)
+    results = json3_output.Convert(
+        test_results_copy, self.base_dir, self.test_path_format)
     # Convert should not modify the original intermediate results.
     self.assertEqual(test_results_copy, test_results)
     return results
@@ -51,6 +53,8 @@ class Json3OutputTest(unittest.TestCase):
     self.assertNotIn('shard', test_result)
     self.assertEqual(results['num_failures_by_type'], {'PASS': 1})
 
+  # TODO(crbug.com/983993): Remove this test when all stories have
+  # url-friendly names without special characters.
   def testUrlAsStoryName(self):
     results = self.Convert([
         testing.TestResult('benchmark/http%3A%2F%2Fexample.com')

tools/perf/core/results_processor/processor.py

@@ -218,15 +218,6 @@ def _GetTraceUrl(test_result):
                else trace_artifact.get('filePath'))
 
 
-def _SplitTestPath(test_result, test_path_format):
-  if test_path_format == command_line.TELEMETRY_TEST_PATH_FORMAT:
-    return test_result['testPath'].split('/', 1)
-  elif test_path_format == command_line.GTEST_TEST_PATH_FORMAT:
-    return test_result['testPath'].split('.', 1)
-  else:
-    raise ValueError('Unknown test path format: %s', test_path_format)
-
-
 def AddDiagnosticsToHistograms(test_result, test_suite_start, results_label,
                                test_path_format):
   """Add diagnostics to all histograms of a test result.
@@ -247,7 +238,7 @@ def AddDiagnosticsToHistograms(test_result, test_suite_start, results_label,
       test_result['_histograms'].AddSharedDiagnosticToAllHistograms(
           name, generic_set.GenericSet(diag))
 
-  test_suite, test_case = _SplitTestPath(test_result, test_path_format)
+  test_suite, test_case = util.SplitTestPath(test_result, test_path_format)
   if 'startTime' in test_result:
     test_start_ms = util.IsoTimestampToEpoch(test_result['startTime']) * 1e3
   else:

tools/perf/core/results_processor/util.py

@@ -7,6 +7,11 @@ import datetime
 import logging
 import multiprocessing
 from multiprocessing.dummy import Pool as ThreadPool
+import urllib
+
+
+TELEMETRY_TEST_PATH_FORMAT = 'telemetry'
+GTEST_TEST_PATH_FORMAT = 'gtest'
 
 
 def ApplyInParallel(function, work_list, on_failure=None):
@@ -49,6 +54,31 @@ def ApplyInParallel(function, work_list, on_failure=None):
     pool.terminate()
 
 
+def SplitTestPath(test_result, test_path_format):
+  """ Split a test path into test suite name and test case name.
+
+  Telemetry and Gtest have slightly different test path formats.
+  Telemetry uses '{benchmark_name}/{story_name}', e.g.
+  'system_health.common_desktop/load:news:cnn:2018'.
+  Gtest uses '{test_suite_name}.{test_case_name}', e.g.
+  'ZeroToFiveSequence/LuciTestResultParameterizedTest.Variant'
+  """
+  if test_path_format == TELEMETRY_TEST_PATH_FORMAT:
+    separator = '/'
+  elif test_path_format == GTEST_TEST_PATH_FORMAT:
+    separator = '.'
+  else:
+    raise ValueError('Unknown test path format: %s' % test_path_format)
+
+  # TODO(crbug.com/981349): Remove this after test paths are no longer
+  # url-quoted.
+  test_path = urllib.unquote(test_result['testPath'])
+  if separator not in test_path:
+    raise ValueError('Invalid test path: %s' % test_path)
+
+  return test_path.split(separator, 1)
+
+
 def IsoTimestampToEpoch(timestamp):
   """Convert ISO formatted time to seconds since epoch."""
   try: