[tools/perf] Add system_health_smoke_test that run all system health stories

tools/perf/benchmarks/system_health_smoke_test.py

+# Copyright 2016 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.
+
+"""Run all the system health stories used by system health benchmarks.

[petrcermak, 2016/06/29 21:32:52]

nit: I'd move "the" in front of "system health benchmarks" instead

[nednguyen, 2016/06/29 21:49:36]

Done.

+
+Only memory benchmarks are used when running these stories to make the total
+cycle time manageable. Other system health benchmarks should be using the same
+stories as memory's ones, only with fewer actions (no memory dumping).

[petrcermak, 2016/06/29 21:32:53]

nitL s/memory's ones/the memory ones/

[nednguyen, 2016/06/29 21:49:36]

Done.

+"""
+
+import unittest
+
+from core import perf_benchmark
+
+from telemetry import benchmark as benchmark_module
+from telemetry import decorators
+from telemetry.core import discover
+from telemetry.internal.browser import browser_finder
+from telemetry.testing import options_for_unittests
+from telemetry.testing import progress_reporter
+
+from benchmarks import system_health
+
+
+# We only cover memory system health
+_SH_BENCHMARKS_TO_SMOKE_TEST = [
+  system_health.DesktopMemorySystemHealth,
+  system_health.MobileMemorySystemHealth,
+]
+
+
+def GetSystemHealthBenchmarksToSmokeTest():
+  sh_benchmark_classes = discover.DiscoverClassesInModule(
+      system_health, perf_benchmark.PerfBenchmark,
+      index_by_class_name=True).values()
+  return list(b for b in sh_benchmark_classes if
+              b.Name().startswith('system_health.memory'))
+
+
+_DISABLED_TESTS = [
+  # crbug.com/624474
+  'benchmarks.system_health_smoke_test.SystemHealthBenchmarkSmokeTest.system_health.memory_desktop.load:tools:dropbox',  # pylint: disable=line-too-long
+]
+
+
+def _GenerateSmokeTestCase(benchmark_class, story_to_smoke_test):
+
+

[petrcermak, 2016/06/29 21:32:52]

Please remove at least one blank line here (or both)

[nednguyen, 2016/06/29 21:49:36]

Done.

+  # NOTE TO SHERIFFS: DO NOT DISABLE THIS TEST.
+  #
+  # This smoke test dynamically tests all system health user stories. So
+  # disabling it for one  failing or flaky benchmark would disable a much

[petrcermak, 2016/06/29 21:32:52]

you have a double space here after "one" and after "coverage" on the line below.

[nednguyen, 2016/06/29 21:49:36]

Done.

+  # wider swath of coverage  than is usally intended. Instead, if a test is
+  # failing, disable it by putting it in _DISABLED_TESTS list above.

[petrcermak, 2016/06/29 21:32:53]

nit: s/in/into the/

[nednguyen, 2016/06/29 21:49:36]

Done.

+  @benchmark_module.Disabled('chromeos')  # crbug.com/351114
+  def runTest(self):

[petrcermak, 2016/06/29 21:32:52]

method names should use CamelCase (not camelCase)

[nednguyen, 2016/06/29 21:49:36]

Done.

+
+    class SinglePageBenchmark(benchmark_class):  # pylint: disable=no-init
+

[petrcermak, 2016/06/29 21:32:53]

nit: I think that this would be more readable with this blank line removed

[nednguyen, 2016/06/29 21:49:36]

Done.

+      def CreateStorySet(self, options):
+        # pylint: disable=super-on-old-class
+        story_set = super(SinglePageBenchmark, self).CreateStorySet(options)
+        assert story_to_smoke_test in story_set.stories
+        story_set.stories = [story_to_smoke_test]
+        return story_set
+
+    options = GenerateBenchmarkOptions(benchmark_class)
+    # Only measure a single story so that this test cycles reasonably quickly.
+    options.pageset_repeat = 1

[petrcermak, 2016/06/29 21:32:52]

Why don't you put these two lines inside GenerateBenchmarkOptions?

[nednguyen, 2016/06/29 21:49:36]

Done.

+    options.page_repeat = 1
+    possible_browser = browser_finder.FindBrowser(options)
+    if possible_browser is None:
+      self.skipTest('Cannot find the browser to run the test.')
+    if (SinglePageBenchmark.ShouldDisable(possible_browser) or
+        not decorators.IsEnabled(benchmark_class, possible_browser)[0]):
+      self.skipTest('Benchmark %s is disabled' % SinglePageBenchmark.Name())
+
+    if self.id() in _DISABLED_TESTS:
+      self.skipTest('Test is explictly disabled')
+
+    self.assertEqual(0, SinglePageBenchmark().Run(options),
+                     msg='Failed: %s' % benchmark_class)
+
+  # We attach the test method to SystemHealthBenchmarkSmokeTest dynamically
+  # so that we can set the test method name to include
+  # '<benchmark class name>.<story display name>'.
+  test_method_name = '%s.%s' % (
+      benchmark_class.Name(), story_to_smoke_test.display_name)
+
+  class SystemHealthBenchmarkSmokeTest(unittest.TestCase):
+    pass
+
+  setattr(SystemHealthBenchmarkSmokeTest, test_method_name, runTest)
+
+  return SystemHealthBenchmarkSmokeTest(methodName=test_method_name)
+
+
+def GenerateBenchmarkOptions(benchmark_class):

[petrcermak, 2016/06/29 21:32:53]

I suppose this should be private (since it's only called by a private function)

[nednguyen, 2016/06/29 21:49:36]

Done.

+  # Set the benchmark's default arguments.
+  options = options_for_unittests.GetCopy()

[petrcermak, 2016/06/29 21:32:52]

This whole method is super-complicated and I have absolutely no idea what it
means. Is it copied from somewhere else (duplication)? It seems like the smoke
tests shouldn't have to know about all the ordering of parsing etc. If you want
to leave this for some other time, please at least add a TODO. Thanks :-)

[nednguyen, 2016/06/29 21:49:36]

Done.

+  options.output_format = 'none'
+  parser = options.CreateParser()
+
+  benchmark_class.AddCommandLineArgs(parser)
+  benchmark_module.AddCommandLineArgs(parser)
+  benchmark_class.SetArgumentDefaults(parser)
+  options.MergeDefaultValues(parser.get_default_values())
+
+  benchmark_class.ProcessCommandLineArgs(None, options)
+  benchmark_module.ProcessCommandLineArgs(None, options)
+  return options
+
+
+def load_tests(loader, standard_tests, pattern):
+  del loader, standard_tests, pattern  # unused
+  suite = progress_reporter.TestSuite()
+  benchmark_classes = GetSystemHealthBenchmarksToSmokeTest()
+  assert benchmark_classes, 'This list should never be empty'
+  for benchmark_class in benchmark_classes:
+
+    # HACK: these options should be derived from options_for_unittests which is

[petrcermak, 2016/06/29 21:32:53]

nit: s/is/are/

[nednguyen, 2016/06/29 21:49:36]

Done.

+    # the resolved options from run_tests' arguments. However, options is only

[petrcermak, 2016/06/29 21:32:52]

nit: s/is/are/

[nednguyen, 2016/06/29 21:49:36]

Done.

+    # parsed during test time which happens after load_tests is called.
+    # Since none of our system health benchmark create stories based on command

[petrcermak, 2016/06/29 21:32:52]

nit: s/benchmark/benchmarks/ and s/create/creates/

[nednguyen, 2016/06/29 21:49:36]

Done.

+    # line options, it should be ok to pass options=None to CreateStorySet.
+    for story_to_smoke_test in (
+        benchmark_class().CreateStorySet(options=None).stories):
+      suite.addTest(
+          _GenerateSmokeTestCase(benchmark_class, story_to_smoke_test))
+
+  return suite