Reland of Create classes_util API, change discover to return a list i…

…nstead of a dict. (patchset chromium#1 id:1 of https://codereview.chromium.org/1263063003/)

Reason for revert:
The telemetry tests aren't broken. The perf_unittests are because they don't pick tests in a deterministic way. Reverting after a change to make them deterministic.

Original issue's description:
> Revert of Create classes_util API, change discover to return a list instead of a dict. (patchset chromium#5 id:120001 of https://codereview.chromium.org/1244223002/)
>
> Reason for revert:
> This appears to break the XP telemetry tests. Details on the bug.
>
> Original issue's description:
> > Create classes_util API, change discover to return a list instead of a dict.
> >
> > BUG=498968
> > CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:linux_perf_bisect;tryserver.chromium.perf:win_perf_bisect;tryserver.chromium.perf:android_nexus5_perf_bisect
> >
> > Committed: https://crrev.com/e6cbec4747aa7fe3c96b1cdb89de21ae77b30ab0
> > Cr-Commit-Position: refs/heads/master@{#341129}
>
> TBR=dtu@chromium.org,bengr@chromium.org,aiolos@chromium.org
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=498968
>
> Committed: https://crrev.com/c85e3e1af7e170d5f8ec2b012df6e9337d2352bb
> Cr-Commit-Position: refs/heads/master@{#341218}

TBR=dtu@chromium.org,bengr@chromium.org,avi@chromium.org
BUG=498968

CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:linux_perf_bisect;tryserver.chromium.perf:mac_perf_bisect;tryserver.chromium.perf:win_perf_bisect;tryserver.chromium.perf:android_nexus5_perf_bisect

Review URL: https://codereview.chromium.org/1262623007

Cr-Commit-Position: refs/heads/master@{#341646}

content/test/gpu/gpu_tests/gpu_test_base_unittest.py

@@ -5,10 +5,10 @@
 import unittest
 
 from telemetry import benchmark
-from telemetry.core import discover
 from telemetry.core import util
 from telemetry.story import story_set as story_set_module
 from telemetry.testing import fakes
+from telemetry.util import classes_util
 
 util.AddDirToPythonPath(util.GetTelemetryDir(), 'third_party', 'mock')
 import mock # pylint: disable=import-error
@@ -23,10 +23,8 @@ def _GetGpuDir(*subdirs):
 
 class NoOverridesTest(unittest.TestCase):
   def testValidatorBase(self):
-    all_validators = discover.DiscoverClasses(
-      _GetGpuDir('gpu_tests'), _GetGpuDir(),
-      gpu_test_base.ValidatorBase,
-      index_by_class_name=True).values()
+    all_validators = classes_util.DiscoverClasses(
+      _GetGpuDir('gpu_tests'), _GetGpuDir(), gpu_test_base.ValidatorBase)
     self.assertGreater(len(all_validators), 0)
     for validator in all_validators:
       self.assertEquals(gpu_test_base.ValidatorBase.ValidateAndMeasurePage,
@@ -35,10 +33,8 @@ def testValidatorBase(self):
                         % validator.__name__)
 
   def testPageBase(self):
-    all_pages = discover.DiscoverClasses(
-      _GetGpuDir(), _GetGpuDir(),
-      gpu_test_base.PageBase,
-      index_by_class_name=True).values()
+    all_pages = classes_util.DiscoverClasses(
+      _GetGpuDir(), _GetGpuDir(), gpu_test_base.PageBase)
     self.assertGreater(len(all_pages), 0)
     for page in all_pages:
       self.assertEquals(gpu_test_base.PageBase.RunNavigateSteps,

content/test/gpu/page_sets/__init__.py

@@ -4,14 +4,13 @@
 import os
 import sys
 
-from telemetry.core import discover
 from telemetry.story import story_set
+from telemetry.util import classes_util
 
 
 # Import all submodules' StorySet classes.
 start_dir = os.path.dirname(os.path.abspath(__file__))
 top_level_dir = os.path.dirname(start_dir)
 base_class = story_set.StorySet
-for cls in discover.DiscoverClasses(
-    start_dir, top_level_dir, base_class).values():
+for cls in classes_util.DiscoverClasses(start_dir, top_level_dir, base_class):
   setattr(sys.modules[__name__], cls.__name__, cls)

tools/chrome_proxy/integration_tests/chrome_proxy_pagesets/__init__.py

@@ -6,17 +6,17 @@
 import os
 import sys
 
-from telemetry.core import discover
 from telemetry import story
+from telemetry.util import classes_util
+
 import video
 
 
 # Import all submodules' StorySet classes.
 start_dir = os.path.dirname(os.path.abspath(__file__))
 top_level_dir = os.path.abspath(os.path.join(start_dir, os.pardir, os.pardir))
 base_class = story.StorySet
-for cls in discover.DiscoverClasses(
-    start_dir, top_level_dir, base_class).values():
+for cls in classes_util.DiscoverClasses(start_dir, top_level_dir, base_class):
   setattr(sys.modules[__name__], cls.__name__, cls)
 
 # DiscoverClasses makes the assumption that there is exactly one matching

tools/chrome_proxy/live_tests/pagesets/__init__.py

@@ -6,15 +6,14 @@
 import os
 import sys
 
-from telemetry.core import discover
 from telemetry import story
+from telemetry.util import classes_util
 
 
 # Import all submodules' StorySet classes.
 start_dir = os.path.dirname(os.path.abspath(__file__))
 top_level_dir = os.path.abspath(os.path.join(start_dir, os.pardir, os.pardir))
 base_class = story.StorySet
-for cls in discover.DiscoverClasses(
-    start_dir, top_level_dir, base_class).values():
+for cls in classes_util.DiscoverClasses(start_dir, top_level_dir, base_class):
   setattr(sys.modules[__name__], cls.__name__, cls)
 

tools/perf/benchmarks/benchmark_smoke_unittest.py

@@ -104,11 +104,11 @@ def load_tests(loader, standard_tests, pattern):
   benchmarks_dir = os.path.dirname(__file__)
   top_level_dir = os.path.dirname(benchmarks_dir)
 
-  # Using the default of |index_by_class_name=False| means that if a module
-  # has multiple benchmarks, only the last one is returned.
+  # Using the default of |one_class_per_module=True| means that if a module
+  # has multiple benchmarks, only the first one is returned.
   all_benchmarks = discover.DiscoverClasses(
       benchmarks_dir, top_level_dir, benchmark_module.Benchmark,
-      index_by_class_name=False).values()
+      one_class_per_module=True)
   for benchmark in all_benchmarks:
     if sys.modules[benchmark.__module__] in _BLACK_LIST_TEST_MODULES:
       continue

tools/perf/benchmarks/benchmark_unittest.py

@@ -11,9 +11,9 @@
 from core import perf_benchmark
 
 from telemetry import benchmark as benchmark_module
-from telemetry.core import discover
 from telemetry.internal.browser import browser_options
 from telemetry.testing import progress_reporter
+from telemetry.util import classes_util
 
 
 def _GetPerfDir(*subdirs):
@@ -22,9 +22,8 @@ def _GetPerfDir(*subdirs):
 
 
 def _GetAllPerfBenchmarks():
-  return discover.DiscoverClasses(
-      _GetPerfDir('benchmarks'), _GetPerfDir(), benchmark_module.Benchmark,
-      index_by_class_name=True).values()
+  return classes_util.DiscoverClasses(
+      _GetPerfDir('benchmarks'), _GetPerfDir(), benchmark_module.Benchmark)
 
 def _BenchmarkOptionsTestGenerator(benchmark):
   def testBenchmarkOptions(self):  # pylint: disable=W0613

tools/perf/benchmarks/skpicture_printer.py

@@ -5,15 +5,15 @@
 from core import perf_benchmark
 
 from telemetry import benchmark
-from telemetry.core import discover
 from telemetry import story
+from telemetry.util import classes_util
 
 from measurements import skpicture_printer
 
 
 def _MatchPageSetName(story_set_name, story_set_base_dir):
-  story_sets = discover.DiscoverClasses(story_set_base_dir, story_set_base_dir,
-                                        story.StorySet).values()
+  story_sets = classes_util.DiscoverClasses(
+      story_set_base_dir, story_set_base_dir, story.StorySet)
   for s in story_sets:
     if story_set_name == s.Name():
       return s

tools/perf/measurements/measurement_smoke_test.py

@@ -8,10 +8,10 @@
 import unittest
 
 from telemetry import benchmark as benchmark_module
-from telemetry.core import discover
 from telemetry.internal.browser import browser_options
 from telemetry.page import page_test
 from telemetry.testing import options_for_unittests
+from telemetry.util import classes_util
 from telemetry.web_perf import timeline_based_measurement
 
 
@@ -23,14 +23,14 @@ def _GetAllPossiblePageTestInstances():
 
   # Get all page test instances from measurement classes that are directly
   # constructable
-  all_measurement_classes = discover.DiscoverClasses(
+  all_measurement_classes = classes_util.DiscoverClasses(
       measurements_dir, top_level_dir, page_test.PageTest,
-      index_by_class_name=True, directly_constructable=True).values()
+      directly_constructable=True)
   for measurement_class in all_measurement_classes:
     page_test_instances.append(measurement_class())
 
-  all_benchmarks_classes = discover.DiscoverClasses(
-      benchmarks_dir, top_level_dir, benchmark_module.Benchmark).values()
+  all_benchmarks_classes = classes_util.DiscoverClasses(
+      benchmarks_dir, top_level_dir, benchmark_module.Benchmark)
 
   # Get all page test instances from defined benchmarks.
   # Note: since this depends on the command line options, there is no guaranteed

tools/perf/page_sets/__init__.py

@@ -6,14 +6,14 @@
 import os
 import sys
 
-from telemetry.core import discover
 from telemetry import story
+from telemetry.util import classes_util
 
 
 # Import all submodules' PageSet classes.
 start_dir = os.path.dirname(os.path.abspath(__file__))
 top_level_dir = os.path.dirname(start_dir)
 base_class = story.StorySet
-for cls in discover.DiscoverClasses(
-    start_dir, top_level_dir, base_class).values():
+for cls in classes_util.DiscoverClasses(
+    start_dir, top_level_dir, base_class):
   setattr(sys.modules[__name__], cls.__name__, cls)

tools/perf/profile_creators/profile_generator.py

@@ -13,19 +13,19 @@
 import tempfile
 
 from profile_creators import profile_extender
-from telemetry.core import discover
 from telemetry.core import util
 from telemetry.internal.browser import browser_finder
 from telemetry.internal.browser import browser_options
 from telemetry.internal import story_runner
+from telemetry.util import classes_util
 
 
 def _DiscoverProfileExtenderClasses():
   profile_extenders_dir = os.path.abspath(os.path.join(util.GetBaseDir(),
       os.pardir, 'perf', 'profile_creators'))
   base_dir = os.path.abspath(os.path.join(profile_extenders_dir, os.pardir))
 
-  profile_extenders_unfiltered = discover.DiscoverClasses(
+  profile_extenders_unfiltered = classes_util.DiscoverClassesByClassName(
       profile_extenders_dir, base_dir, profile_extender.ProfileExtender)
 
   # Remove 'extender' suffix from keys.

tools/telemetry/telemetry/benchmark_runner.py

@@ -14,11 +14,11 @@
 import sys
 
 from telemetry import benchmark
-from telemetry.core import discover
 from telemetry import decorators
 from telemetry.internal.browser import browser_finder
 from telemetry.internal.browser import browser_options
 from telemetry.internal.util import command_line
+from telemetry.util import classes_util
 
 
 def PrintBenchmarkList(benchmarks, possible_browser, output_pipe=sys.stdout):
@@ -264,10 +264,9 @@ def _MatchingCommands(string):
 def _Benchmarks(environment):
   benchmarks = []
   for search_dir in environment.benchmark_dirs:
-    benchmarks += discover.DiscoverClasses(search_dir,
+    benchmarks += classes_util.DiscoverClasses(search_dir,
                                            environment.top_level_dir,
-                                           benchmark.Benchmark,
-                                           index_by_class_name=True).values()
+                                           benchmark.Benchmark)
   return benchmarks
 
 def _MatchBenchmarkName(input_benchmark_name, environment, exact_matches=True):

tools/telemetry/telemetry/core/discover.py

@@ -7,12 +7,9 @@
 import os
 import re
 
-from telemetry import decorators
-from telemetry.internal.util import camel_case
 from telemetry.internal.util import classes as classes_module
 
 
-@decorators.Cache
 def DiscoverModules(start_dir, top_level_dir, pattern='*'):
   """Discover all modules in |start_dir| which match |pattern|.
 
@@ -49,12 +46,8 @@ def DiscoverModules(start_dir, top_level_dir, pattern='*'):
       modules.append(module)
   return modules
 
-
-# TODO(dtu): Normalize all discoverable classes to have corresponding module
-# and class names, then always index by class name.
-@decorators.Cache
 def DiscoverClasses(start_dir, top_level_dir, base_class, pattern='*',
-                    index_by_class_name=True, directly_constructable=False):
+                    one_class_per_module=False, directly_constructable=False):
   """Discover all classes in |start_dir| which subclass |base_class|.
 
   Base classes that contain subclasses are ignored by default.
@@ -64,24 +57,21 @@ def DiscoverClasses(start_dir, top_level_dir, base_class, pattern='*',
     top_level_dir: The top level of the package, for importing.
     base_class: The base class to search for.
     pattern: Unix shell-style pattern for filtering the filenames to import.
-    index_by_class_name: If True, use class name converted to
-        lowercase_with_underscores instead of module name in return dict keys.
+    one_class_per_module: If True, will only include the first class found in
+                each module.
     directly_constructable: If True, will only return classes that can be
         constructed without arguments
 
-  Returns:
-    dict of {module_name: class} or {underscored_class_name: class}
+  Returns: A list of classes.
   """
   modules = DiscoverModules(start_dir, top_level_dir, pattern)
-  classes = {}
+  classes = []
   for module in modules:
-    new_classes = DiscoverClassesInModule(
-        module, base_class, index_by_class_name, directly_constructable)
-    classes = dict(classes.items() + new_classes.items())
+    classes.extend(DiscoverClassesInModule(
+        module, base_class, one_class_per_module, directly_constructable))
   return classes
 
-@decorators.Cache
-def DiscoverClassesInModule(module, base_class, index_by_class_name=False,
+def DiscoverClassesInModule(module, base_class, one_class_per_module=False,
                             directly_constructable=False):
   """Discover all classes in |module| which subclass |base_class|.
 
@@ -90,13 +80,12 @@ def DiscoverClassesInModule(module, base_class, index_by_class_name=False,
   Args:
     module: The module to search.
     base_class: The base class to search for.
-    index_by_class_name: If True, use class name converted to
-        lowercase_with_underscores instead of module name in return dict keys.
+    one_class_per_module: If True, will only include the first class found in
+                each module.
 
-  Returns:
-    dict of {module_name: class} or {underscored_class_name: class}
+  Returns: A list of classes.
   """
-  classes = {}
+  classes = []
   for _, obj in inspect.getmembers(module):
     # Ensure object is a class.
     if not inspect.isclass(obj):
@@ -115,14 +104,11 @@ def DiscoverClassesInModule(module, base_class, index_by_class_name=False,
     if obj.__module__ != module.__name__:
       continue
 
-    if index_by_class_name:
-      key_name = camel_case.ToUnderscore(obj.__name__)
-    else:
-      key_name = module.__name__.split('.')[-1]
     if (not directly_constructable or
         classes_module.IsDirectlyConstructable(obj)):
-      classes[key_name] = obj
-
+      classes.append(obj)
+      if one_class_per_module:
+        return classes
   return classes