[Telemetry] Add CanRunOnPlatform to story_runner and benchmark class.

telemetry/telemetry/benchmark.py

Index: telemetry/telemetry/benchmark.py
diff --git a/telemetry/telemetry/benchmark.py b/telemetry/telemetry/benchmark.py
index 3cea7caa5cd21ff0108665eaf34c33414e432ff4..04dad73967b8b6427b78541a12971bbd5ee0b99c 100644
--- a/telemetry/telemetry/benchmark.py
+++ b/telemetry/telemetry/benchmark.py
@@ -63,6 +63,7 @@ class Benchmark(command_line.Command):
   options = {}
   page_set = None
   test = timeline_based_measurement.TimelineBasedMeasurement
+  SUPPORTED_PLATFORMS = [expectations.ALL]
 
   def __init__(self, max_failures=None):
     """Creates a new Benchmark.
@@ -82,6 +83,16 @@ class Benchmark(command_line.Command):
     # See https://github.com/catapult-project/catapult/issues/3708
 
 
+  def _CanRunOnPlatform(self, platform, finder_options):
+    for p in self.SUPPORTED_PLATFORMS:
+      # This is reusing StoryExpectation code, so it is a bit unintuitive. We
+      # are trying to detect the opposite of the usual case in StoryExpectations
+      # so we want to return True when ShouldDisable returns true, even though
+      # we do not want to disable.
+      if p.ShouldDisable(platform, finder_options):

[charliea (OOO until 10-5), 2017/08/22 17:22:18]

I understand why this works the way that it does, and have no problem
implementing it w/ this note in the short term, but it seems like we should file
a bug to move to a more sensible world in the future.

It seems part of the problem is that, like their private names, expectations.ALL
or expectations.ALL_MAC aren't really expectations, but rather test conditions
to determine whether a platform meets a given criteria. (I might prefer
"platform condition" or "platform filter", given how overloaded the word test is
already within Telemetry, though.) They only become expectations once we combine
them with what we expect to happen on that platform, e.g. "browse_google should
be disabled on ALL_WIN", or "system_health should be enabled on ALL_MAC".

However, currently baked into them is the idea that they only exist for
disabling things - thus the "ShouldDisable()" method attached to them. It seems
like it'd be more sensible to change that method to "MatchesPlatform()" or
something. Then this would become:

if p.MatchesPlatform(platform, finder_options):
  return True

I don't think that we should do this in this CL, but it does seem like some
technical debt that we should file a bug about cleaning up and note that here.

[rnephew (Reviews Here), 2017/08/22 17:33:20]

The expectations file has the TestCondition class defined inside of it. Those
could be moved to their own file. They were designed in a way that was
StoryExpectations-centric. A good refactor in a follow up CL would help make
things clearer, I agree. 

I will add a todo to the code.

+        return True
+    return False
+
   # pylint: disable=unused-argument
   @classmethod
   def ShouldDisable(cls, possible_browser):