[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 the first page of one benchmark for every module.

[petrcermak, 2016/06/29 06:53:27]

I don't think that this is the case. You seem to be running ALL stories for SH
benchmarks.

[nednguyen, 2016/06/29 18:16:41]

Update the description completely. I was just copying & pasting :P

+
+Only benchmarks that have a composable measurement are included.

[petrcermak, 2016/06/29 06:53:27]

Not sure what this means.

[nednguyen, 2016/06/29 18:16:41]

Removed this part.

+Ideally this test would be comprehensive, however, running one page
+of every benchmark would run impractically long.
+"""
+
+import sys
+import unittest
+
+from telemetry import benchmark as benchmark_module
+from telemetry import decorators
+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 benchmarks because other system health
+# benchmarks should be using the same stories as memory's ones, only with less

[petrcermak, 2016/06/29 06:53:27]

nit: s/less/fewer/ ("actions" are countable)

[nednguyen, 2016/06/29 18:16:41]

Done.

+# actions (no memory dumping).
+_SH_BENCHMARKS_TO_SMOKE_TEST = [
+  system_health.DesktopMemorySystemHealth,
+  system_health.MobileMemorySystemHealth,
+]
+
+
+_DISABLED_TESTS = [
+  'benchmarks.system_health_smoke_test.SystemHealthBenchmarkSmokeTest.system_health.memory_desktop.load:tools:dropbox',  # pylint: disable=line-too-long

[petrcermak, 2016/06/29 06:53:28]

Could you please add a bug for each of these?

[nednguyen, 2016/06/29 18:16:41]

Done.

Looks like memory_mobile.load:games:bubbles is no longer failing. But we need to
keep an eye on it though

+  'benchmarks.system_health_smoke_test.SystemHealthBenchmarkSmokeTest.system_health.memory_mobile.load:games:bubbles',  # pylint: disable=line-too-long
+]
+
+
+def SmokeTestGenerator(benchmark_class, story_to_smoke_test):
+  # NOTE TO SHERIFFS: DO NOT DISABLE THIS TEST.
+  #
+  # This smoke test dynamically tests all benchmarks. So disabling it for one

[petrcermak, 2016/06/29 06:53:27]

s/all benchmarks/all system health user stories/ ?

[nednguyen, 2016/06/29 18:16:41]

Done.

+  # failing or flaky benchmark would disable a much wider swath of coverage
+  # than is usally intended. Instead, if a test is failing, disable it by
+  # putting it in _DISABLED_TESTS list above.
+
+  @benchmark_module.Disabled('chromeos')  # crbug.com/351114
+  def SmokeTest(self):
+    # Only measure a single story so that this test cycles reasonably quickly.
+    benchmark_class.options['pageset_repeat'] = 1

[petrcermak, 2016/06/29 06:53:27]

Isn't this modifying the top-level benchmark? Why not set this on the
SinglePageBenchmark options?

[nednguyen, 2016/06/29 18:16:41]

Done. Set these in options to make sure instead.

+    benchmark_class.options['page_repeat'] = 1
+
+    class SinglePageBenchmark(benchmark_class):  # pylint: disable=no-init
+
+      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)
+    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)
+
+  return SmokeTest
+
+
+def GenerateBenchmarkOptions(benchmark_class):
+  # Set the benchmark's default arguments.
+  options = options_for_unittests.GetCopy()
+  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()
+  for benchmark_class in _SH_BENCHMARKS_TO_SMOKE_TEST:
+    assert sys.modules[benchmark_class.__module__] is system_health

[petrcermak, 2016/06/29 06:53:27]

This seems unnecessary

[nednguyen, 2016/06/29 18:16:42]

I was wanting to make sure that people don't add non sh benchmark by accident.
But with the new method of gathering benchmark to smoke test, it's no longer
necessary.

+    class SystemHealthBenchmarkSmokeTest(unittest.TestCase):
+      pass
+
+    # HACK: this options should be derived from options_for_unittests which is

[petrcermak, 2016/06/29 06:53:28]

nit: s/this options/these options/

[nednguyen, 2016/06/29 18:16:41]

Done.

+    # the resolved options from run_tests' arguments. However, options is only
+    # parsed during test time which happens after load_tests is called.
+    # Since none of our system health benchmark create stories based on

[petrcermak, 2016/06/29 06:53:27]

This comment is not complete.

+    options = None
+
+    for story_to_smoke_test in (
+        benchmark_class().CreateStorySet(options).stories):

[petrcermak, 2016/06/29 06:53:27]

Why do you name |options| when you could just pass None here?

[nednguyen, 2016/06/29 18:16:42]

I thought it was easier to add documentation there. Anyway, just use named
param.

+      test_method_name = '%s.%s' % (
+          benchmark_class.Name(), story_to_smoke_test.display_name)
+      method = SmokeTestGenerator(benchmark_class, story_to_smoke_test)
+      setattr(SystemHealthBenchmarkSmokeTest, test_method_name, method)

[petrcermak, 2016/06/29 06:53:27]

This seems unnecessarily complicated. Why don't you just define the whole class
in SmokeTestGenerator (as opposed to defining just the test method)? You could
do something like:

for benchmark_class in _SH_BENCHMARKS_TO_SMOKE_TEST:
  for story_to_smoke_test in benchmark_class().CreateStorySet(None).stories:
    _AddSmokeTestForStory(suite, story_to_smoke_test)

[nednguyen, 2016/06/29 18:16:41]

Because we need to make the test method name contains "<benchmark name>.<story
name>".

Even if we define the whole class with the test method directly, we still need
to do some name manipulating logic there. Anyway, I think it's a good idea to
move this to _AddSmokeTestForStory

+
+      suite.addTest(SystemHealthBenchmarkSmokeTest(test_method_name))

[petrcermak, 2016/06/29 06:53:28]

I suggest you name the parameter:

SystemHealthBenchmarkSmokeTest(methodName=test_method_name)
                               ^^^^^^^^^^^

to make the code more readable

[nednguyen, 2016/06/29 18:16:41]

Done.

+
+  return suite