[tools/perf] Add unittests coverage for system health benchmarks

tools/perf/benchmarks/benchmark_unittest.py

Index: tools/perf/benchmarks/benchmark_unittest.py
diff --git a/tools/perf/benchmarks/benchmark_unittest.py b/tools/perf/benchmarks/benchmark_unittest.py
index 82cb87e0e05413750c8a6a24146e35b1871bde8b..b6298cd8d4bd4c1e7ef293d6602396e036be7570 100644
--- a/tools/perf/benchmarks/benchmark_unittest.py
+++ b/tools/perf/benchmarks/benchmark_unittest.py
@@ -7,7 +7,10 @@
 from collections import defaultdict
 import os
 import unittest
+import sys

[petrcermak, 2016/08/03 08:49:27]

nit: sort order ('s' < 'u')

[nednguyen, 2016/08/03 15:41:36]

Done.

 
+from benchmarks import system_health

[petrcermak, 2016/08/03 08:49:27]

nit: I would probably do `from benchmarks import system_health as
system_health_benchmark` so that the name is unambiguous throughout the file.
WDYT?

[nednguyen, 2016/08/03 15:41:36]

Done.

+from page_sets.system_health import system_health_stories

[petrcermak, 2016/08/03 08:49:26]

nit: sort order ('b' < 'c' < 'p'). If you want to keep the SH imports together,
then I suggest you separate them using a blank line.

[nednguyen, 2016/08/03 15:41:36]

Done.

 from core import perf_benchmark
 
 from telemetry import benchmark as benchmark_module
@@ -27,6 +30,11 @@ def _GetAllPerfBenchmarks():
       index_by_class_name=True).values()
 
 
+def _GetAllSystemHealthBenchmarks():
+  return [b for b in _GetAllPerfBenchmarks() if
+          sys.modules[b.__module__] == system_health]
+
+
 def _BenchmarkOptionsTestGenerator(benchmark):
   def testBenchmarkOptions(self):  # pylint: disable=unused-argument
     """Invalid options will raise benchmark.InvalidOptionsError."""
@@ -51,6 +59,30 @@ class TestNoBenchmarkNamesDuplication(unittest.TestCase):
                         'found: %s' % (n, str(names_to_benchmarks[n])))
 
 
+class TestSystemHealthBenchmarks(unittest.TestCase):
+
+  def testNaming(self):
+    for s in _GetAllSystemHealthBenchmarks():

[petrcermak, 2016/08/03 08:49:27]

nit: could you change the variable to |b| as well so that it's consistent with
the rest of the test?

[nednguyen, 2016/08/03 15:41:36]

Done.

+      self.assertTrue(
+          s.Name().startswith('system_health.'),
+          '%s must have name started with "system_health." prefix')

[petrcermak, 2016/08/03 08:49:26]

you forgot to add `% s`

[petrcermak, 2016/08/03 08:49:27]

also, I wonder whether '%r' wouldn't be more applicable:

%s -> 'module.ClassName'
%r -> '<class module.ClassName at 0x7f83d1757870>'

Alternatively, how about we use '%s' with `b.Name()`?

[nednguyen, 2016/08/03 15:41:36]

Done.

+
+  def testShouldTearDownStateAfterEachStoryRunIsTrue(self):
+    for b in _GetAllSystemHealthBenchmarks():
+      self.assertTrue(
+          b.ShouldTearDownStateAfterEachStoryRun(),
+          '%s has ShouldTearDownStateAfterEachStoryRun set to False' % b)
+
+  def testSystemHealthStorySetAreUsed(self):
+    for b in _GetAllSystemHealthBenchmarks():
+      if b is system_health.WebviewStartupSystemHealthBenchmark:
+          continue

[petrcermak, 2016/08/03 08:49:27]

nit: this should be indented only 2 spaces (-2)

[petrcermak, 2016/08/03 08:49:27]

Does this mean that the WebView benchmark shouldn't be a part of System Health
or it should use the SH story set?

[nednguyen, 2016/08/03 15:41:36]

Probably it should use the SH story set. We probably should add blank case to sh
story et.

+      self.assertIsInstance(
+          b().CreateStorySet(None),
+          system_health_stories.SystemHealthStorySet,
+          '%s does not use SystemHealthStorySet' % b)
+
+
 class TestNoOverrideCustomizeBrowserOptions(unittest.TestCase):
 
   def runTest(self):
@@ -81,12 +113,12 @@ def _AddBenchmarkOptionsTests(suite):
     setattr(BenchmarkOptionsTest, benchmark.Name(),
             _BenchmarkOptionsTestGenerator(benchmark))
     suite.addTest(BenchmarkOptionsTest(benchmark.Name()))
-  suite.addTest(TestNoBenchmarkNamesDuplication())
-  suite.addTest(TestNoOverrideCustomizeBrowserOptions())
 
 
 def load_tests(loader, standard_tests, pattern):
-  del loader, standard_tests, pattern  # unused
+  del loader, pattern  # unused
   suite = progress_reporter.TestSuite()
+  for t in standard_tests:
+    suite.addTests(t)
   _AddBenchmarkOptionsTests(suite)
   return suite