[Telemetry] Add Expectation module that enables disabling benchmarks/stories

telemetry/telemetry/story/expectations.py

Index: telemetry/telemetry/story/expectations.py
diff --git a/telemetry/telemetry/story/expectations.py b/telemetry/telemetry/story/expectations.py
new file mode 100644
index 0000000000000000000000000000000000000000..1d5292718b64be49c27488e87cf9ad0d90acc435
--- /dev/null
+++ b/telemetry/telemetry/story/expectations.py
@@ -0,0 +1,165 @@
+# Copyright 2017 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.
+
+import logging
+
+
+class StoryExpectations(object):
+  """An object that contains disabling expectations for benchmarks and stories.
+
+  Example Usage:
+  class FooBenchmarkExpectations(expectations.StoryExpectations):
+    def SetExpectations(self):
+      self.DisableBenchmark([expectations.ALL_WIN], 'crbug.com/123')
+      self.DisableBenchmark([expectations.ALL_MOBILE], 'Desktop Benchmark')
+      self.DisableStory('story_name1', [expectations.ALL_MAC], 'crbug.com/456')
+      self.DisableStory('story_name2', [expectations.ALL], 'crbug.com/789')
+      ...
+  """
+  def __init__(self):
+    self._disabled_platforms = []
+    self._expectations = {}
+    self._frozen = False
+    self.SetExpectations()
+    self._Freeze()
+
+  def SetExpectations(self):
+    """Sets the Expectations for test disabling
+
+    Override in subclasses to disable tests."""
+    pass
+
+  def _Freeze(self):
+    self._frozen = True
+
+  def DisableBenchmark(self, conditions, reason):
+    """Disable benchmark under conditions pased to method.

[charliea (OOO until 10-5), 2017/05/15 21:15:41]

s/pased/passed

[charliea (OOO until 10-5), 2017/05/15 21:15:42]

supernit suggestion: use "given" instead of "passed to method"

It's a little bit less awkward, especially when you have lots of arguments. This
would make it just "Disables the benchmark under the given conditions."

[rnephew (Reviews Here), 2017/05/15 21:36:06]

Done.

[rnephew (Reviews Here), 2017/05/15 21:36:06]

Done.

+
+    Example:
+      DisableBenchmark([expectations.ALL_MOBILE], 'Desktop benchmark')
+      DisableBenchmark([expectations.ALL_WIN], 'crbug.com/123')
+
+    Args:
+      conditions: List of _TestCondition subclasses.
+      reason: Reason for disabling the benchmark.
+    """
+    assert reason, 'A reason for disabling must be given.'
+    assert not self._frozen, ('Cannot disable benchmark on a frozen '
+                              'StoryExpectation object.')
+    for condition in conditions:
+      assert isinstance(condition, _TestCondition)
+
+    self._disabled_platforms.append((conditions, reason))
+
+  def IsBenchmarkDisabled(self, platform):
+    """Returns if the benchmark has been disabled for the platform.

[charliea (OOO until 10-5), 2017/05/15 21:15:41]

I think that you can collapse this whole thing down and get rid of the "Returns"
clause at the bottom:

"""Returns the reason the benchmark was disabled, or None if the benchmark is
enabled.

Args:
  platform: A platform object
"""

[rnephew (Reviews Here), 2017/05/15 21:36:06]

Done.

+
+    Args:
+      platform: A platform object.
+
+    Returns:
+      Reason if disabled, None otherwise.
+    """
+    for conditions, reason in self._disabled_platforms:
+      for condition in conditions:
+        if condition.ShouldDisable(platform):
+          logging.info('Benchmark disabled on %s due to %s.',
+                       condition, reason)
+          return reason
+    return None
+
+  def DisableStory(self, story_name, conditions, reason):
+    """Disable story under given conditions.

[charliea (OOO until 10-5), 2017/05/15 21:15:41]

supernit: s/Disables story under given conditions./Disables the story under the
given conditions.

[rnephew (Reviews Here), 2017/05/15 21:36:06]

Done.

+
+    Example:
+      DisableStory('story_name', [expectations.ALL_WIN], 'crbug.com/123')
+
+    Args:
+      story_name: Name of the story to disable passed as a string.
+      conditions: List of _TestCondition subclasses.
+      reason: Reason for disabling the story.
+    """
+    assert reason, 'A reason for disabling must be given.'
+    assert len(story_name) < 50, (
+        "Story name exceeds limit of 50 characters. This limit is in place to "
+        "encourage Telemetry benchmark owners to use short, simple story names "
+        "(e.g. 'google_search_images', not "
+        "'http://www.google.com/images/1234/abc')."
+
+    )
+    assert not self._frozen, ('Cannot disable stories on a frozen '
+                              'StoryExpectation object.')
+    for condition in conditions:
+      assert isinstance(condition, _TestCondition)
+    if not self._expectations.get(story_name):
+      self._expectations[story_name] = []
+    self._expectations[story_name].append((conditions, reason))
+
+  def IsStoryDisabled(self, story, platform):
+    """Returns if the story has been disabled for the current platform.

[charliea (OOO until 10-5), 2017/05/15 21:15:42]

See notes for IsBenchmarkDisabled

[rnephew (Reviews Here), 2017/05/15 21:36:06]

Done.

+
+    Args:
+      story: Story object that contains a display_name property.
+      platform: A platform object.
+
+    Returns:
+      Reason if disabled, None otherwise.
+    """
+

[charliea (OOO until 10-5), 2017/05/15 21:15:42]

supernit: delete empty first line

[rnephew (Reviews Here), 2017/05/15 21:36:06]

Done.

+    for conditions, reason in self._expectations.get(story.display_name, []):
+      for condition in conditions:
+        if condition.ShouldDisable(platform):
+          logging.info('%s is disabled on %s due to %s.',
+                       story.display_name, condition, reason)
+          return reason
+    return None
+
+
+class _TestCondition(object):
+  def ShouldDisable(self, platform):
+    raise NotImplementedError
+
+  def __str__(self):
+    raise NotImplementedError
+
+
+class _TestConditionByPlatformList(_TestCondition):
+  def __init__(self, platforms, name):
+    self._platforms = platforms
+    self._name = name
+
+  def ShouldDisable(self, platform):
+    return platform.GetOSName() in self._platforms
+
+  def __str__(self):
+    return self._name
+
+
+class _AllTestCondition(_TestCondition):
+  def ShouldDisable(self, _):
+    return True
+
+  def __str__(self):
+    return 'All Platforms'
+
+
+class _NoBattOrTestCondition(_TestCondition):
+  # Example use case for NO_BATTOR test condition:
+  #   If a trace is too long to be processed if it contains power data.
+  def ShouldDisable(self, platform):
+    return not platform.HasBattOrConnected()
+
+  def __str__(self):
+    return 'Platforms with no BattOr'
+
+
+ALL = _AllTestCondition()
+ALL_MAC = _TestConditionByPlatformList(['mac'], 'Mac Platforms')
+ALL_WIN = _TestConditionByPlatformList(['win'], 'Win Platforms')
+ALL_LINUX = _TestConditionByPlatformList(['linux'], 'Linux Platforms')
+ALL_ANDROID = _TestConditionByPlatformList(['android'], 'Android Platforms')
+ALL_DESKTOP = _TestConditionByPlatformList(
+    ['mac', 'linux', 'win'], 'Desktop Platforms')
+ALL_MOBILE = _TestConditionByPlatformList(['android'], 'Mobile Platforms')
+NO_BATTOR = _NoBattOrTestCondition()