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

telemetry/telemetry/story/expectations_unittest.py

Index: telemetry/telemetry/story/expectations_unittest.py
diff --git a/telemetry/telemetry/story/expectations_unittest.py b/telemetry/telemetry/story/expectations_unittest.py
new file mode 100644
index 0000000000000000000000000000000000000000..25b658ea7b13df6a5c6dce692acd07138aa38bfb
--- /dev/null
+++ b/telemetry/telemetry/story/expectations_unittest.py
@@ -0,0 +1,160 @@
+# 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 unittest
+
+from telemetry.story import expectations
+from telemetry.testing import fakes
+
+
+class MockState(object):
+  def __init__(self):
+    self.platform = fakes.FakePlatform()
+
+
+class MockStory(object):
+  def __init__(self, name):
+    self._name = name
+
+  @property
+  def display_name(self):
+    return self._name
+
+
+class TestConditionTest(unittest.TestCase):
+  def testAll(self):
+    p = fakes.FakePlatform()
+    self.assertTrue(expectations.ALL.ShouldDisable(p))
+
+  def testAllWin(self):

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

nit: I think you should actually split most of these tests in two: for example,
testAllWinDisabledReturnsReason and testAllWinEnabledReturnsNone

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

Done.

+    p = fakes.FakePlatform()
+    p.SetOSName('win')
+    self.assertTrue(expectations.ALL_WIN.ShouldDisable(p))

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

Rather than asserting true, I think you should assert that the returned reason
is equal. Regular callers can treat the value as truthy/falsy all they want, but
in the unit test, it's our job to be as precise as possible when verifying the
function's inputs and outputs

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

Thats part of the StoryExpectation, not part of the TestCondition. The given
test condition just returns True if htat condition is true (ALL_MAC returns true
if its a mac). The 'reason' is stored in the SToryExpectations part and is
tested bellow. 

[charliea (OOO until 10-5), 2017/05/16 16:09:27]

Ah, okay, sorry about that. Thanks!

+    p.SetOSName('not_windows')
+    self.assertFalse(expectations.ALL_WIN.ShouldDisable(p))
+
+  def testAllLinux(self):
+    p = fakes.FakePlatform()
+    p.SetOSName('linux')
+    self.assertTrue(expectations.ALL_LINUX.ShouldDisable(p))
+    p.SetOSName('not_linux')
+    self.assertFalse(expectations.ALL_LINUX.ShouldDisable(p))
+
+  def testAllMac(self):
+    p = fakes.FakePlatform()
+    p.SetOSName('mac')
+    self.assertTrue(expectations.ALL_MAC.ShouldDisable(p))
+    p.SetOSName('not_mac')
+    self.assertFalse(expectations.ALL_MAC.ShouldDisable(p))
+
+  def testAllAndroid(self):
+    p = fakes.FakePlatform()
+    p.SetOSName('android')
+    self.assertTrue(expectations.ALL_ANDROID.ShouldDisable(p))
+    p.SetOSName('not_android')
+    self.assertFalse(expectations.ALL_ANDROID.ShouldDisable(p))
+
+  def testAllDesktop(self):
+    true_platforms = ['win', 'mac', 'linux']
+    false_platforms = ['android']
+    p = fakes.FakePlatform()
+
+    for plat in true_platforms:
+      p.SetOSName(plat)
+      self.assertTrue(expectations.ALL_DESKTOP.ShouldDisable(p))
+    for plat in false_platforms:
+      p.SetOSName(plat)
+      self.assertFalse(expectations.ALL_DESKTOP.ShouldDisable(p))
+
+  def testAllMobile(self):
+    false_platforms = ['win', 'mac', 'linux']
+    true_platforms = ['android']
+    p = fakes.FakePlatform()
+
+    for plat in true_platforms:
+      p.SetOSName(plat)
+      self.assertTrue(expectations.ALL_MOBILE.ShouldDisable(p))
+    for plat in false_platforms:
+      p.SetOSName(plat)
+      self.assertFalse(expectations.ALL_MOBILE.ShouldDisable(p))
+
+  def testNoBattOr(self):
+    p = fakes.FakePlatform()
+    p.SetBattOrDetected(False)
+    self.assertTrue(expectations.NO_BATTOR.ShouldDisable(p))
+    p.SetBattOrDetected(True)
+    self.assertFalse(expectations.ALL_WIN.ShouldDisable(p))
+
+
+class StoryExpectationsTest(unittest.TestCase):
+  def setUp(self):
+    self.platform = fakes.FakePlatform()
+
+  def testCantDisableAfterInit(self):
+    e = expectations.StoryExpectations()
+    with self.assertRaises(AssertionError):
+      e.DisableBenchmark(['test'], 'test')
+    with self.assertRaises(AssertionError):
+      e.DisableStory('story', ['platform'], 'reason')
+
+  def testDisableBenchmark(self):
+    class FooExpectations(expectations.StoryExpectations):
+      def SetExpectations(self):
+        self.DisableBenchmark([expectations.ALL_WIN], 'crbug.com/123')
+
+    e = FooExpectations()
+    self.platform.SetOSName('win')
+
+    reason = e.IsBenchmarkDisabled(self.platform)
+    self.assertTrue(reason)
+    self.assertEqual(reason, 'crbug.com/123')
+
+    self.platform.SetOSName('android')
+    reason = e.IsBenchmarkDisabled(self.platform)
+    self.assertIsNone(reason)
+
+  def testDisableStoryMultipleConditions(self):
+    class FooExpectations(expectations.StoryExpectations):
+      def SetExpectations(self):
+        self.DisableStory(
+            'multi', [expectations.ALL_WIN], 'crbug.com/123')
+        self.DisableStory(
+            'multi', [expectations.ALL_MAC], 'crbug.com/456')
+
+    e = FooExpectations()
+
+    self.platform.SetOSName('mac')
+    reason = e.IsStoryDisabled(
+        MockStory('multi'), self.platform)
+    self.assertTrue(reason)
+    self.assertEqual(reason, 'crbug.com/456')
+
+  def testDisableStoryOneCondition(self):
+    class FooExpectations(expectations.StoryExpectations):
+      def SetExpectations(self):
+        self.DisableStory(
+            'disable', [expectations.ALL_WIN], 'crbug.com/123')
+
+    e = FooExpectations()
+
+    self.platform.SetOSName('win')
+    reason = e.IsStoryDisabled(
+        MockStory('disable'), self.platform)
+    self.assertTrue(reason)

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

(here and elsewhere in this file where you assert twice on the same value) is
self.assertTrue necessary? It seems like it's strictly less descriptive than the
following assertEqual

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

Done.

+    self.assertEqual(reason, 'crbug.com/123')
+    self.platform.SetOSName('mac')
+    reason = e.IsStoryDisabled(
+        MockStory('disbled'), self.platform)

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

nit: disabled?

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

Done.

+    self.assertFalse(reason)
+    self.assertIsNone(reason)
+
+  def testDisableStoryWithLongName(self):
+    class FooExpectations(expectations.StoryExpectations):
+      def SetExpectations(self):
+        self.DisableStory(
+            '123456789012345678901234567890123456789012345678901',
+            [expectations.ALL], 'Too Long')
+
+    with self.assertRaises(AssertionError):
+      FooExpectations()