Bug 1258497: support test platform pretty names; r?sfink draft
authorDustin J. Mitchell <dustin@mozilla.com>
Thu, 05 May 2016 17:10:45 +0000
changeset 364483 1f63e71b12e87e4318612f437e22c4ee3b2b444e
parent 364482 ab5c6bac7fb1bbfad52eba9f4eada2affed07cd8
child 364484 90e43e742dbd6ad8e64fd5edec2c3f58557ace75
push id17466
push userdmitchell@mozilla.com
push dateFri, 06 May 2016 18:22:06 +0000
reviewerssfink
bugs1258497
milestone49.0a1
Bug 1258497: support test platform pretty names; r?sfink MozReview-Commit-ID: sqQgToBCse
taskcluster/docs/attributes.rst
taskcluster/taskgraph/test/test_try_option_syntax.py
taskcluster/taskgraph/try_option_syntax.py
--- a/taskcluster/docs/attributes.rst
+++ b/taskcluster/docs/attributes.rst
@@ -53,18 +53,18 @@ unittest_flavor
 
 If a unittest suite has subdivisions, those are represented as flavors.  Not
 all suites have flavors, in which case this attribute should be omitted (but is
 sometimes set to match the suite).
 
 unittest_try_name
 =================
 
-This is the name used to refer to a unit test via try syntax.  It may not match
-either of ``unittest_suite`` or ``unittest_flavor``.
+(deprecated) This is the name used to refer to a unit test via try syntax.  It
+may not match either of ``unittest_suite`` or ``unittest_flavor``.
 
 test_chunk
 ==========
 
 This is the chunk number of a chunked test suite (talos or unittest).  Note
 that this is a string!
 
 legacy_kind
--- a/taskcluster/taskgraph/test/test_try_option_syntax.py
+++ b/taskcluster/taskgraph/test/test_try_option_syntax.py
@@ -179,16 +179,37 @@ class TestTryOptionSyntax(unittest.TestC
 
     def test_u_platforms(self):
         "-u gtest[linux,win32] selects the linux and win32 platforms for gtest"
         tos = TryOptionSyntax('try: -u gtest[linux,win32]', graph_with_jobs)
         self.assertEqual(sorted(tos.unittests), sorted([
             {'test': 'gtest', 'platforms': ['linux', 'win32']},
         ]))
 
+    def test_u_platforms_pretty(self):
+        "-u gtest[Ubuntu] selects the linux and linux64 platforms for gtest"
+        tos = TryOptionSyntax('try: -u gtest[Ubuntu]', graph_with_jobs)
+        self.assertEqual(sorted(tos.unittests), sorted([
+            {'test': 'gtest', 'platforms': ['linux', 'linux64']},
+        ]))
+
+    def test_u_platforms_negated(self):
+        "-u gtest[-linux] selects no platforms for gtest"
+        tos = TryOptionSyntax('try: -u gtest[-linux]', graph_with_jobs)
+        self.assertEqual(sorted(tos.unittests), sorted([
+            {'test': 'gtest', 'platforms': []},
+        ]))
+
+    def test_u_platforms_negated_pretty(self):
+        "-u gtest[Ubuntu,-x64] selects just linux for gtest"
+        tos = TryOptionSyntax('try: -u gtest[Ubuntu,-x64]', graph_with_jobs)
+        self.assertEqual(sorted(tos.unittests), sorted([
+            {'test': 'gtest', 'platforms': ['linux']},
+        ]))
+
     def test_u_chunks_platforms(self):
         "-u gtest-1[linux,win32] selects the linux and win32 platforms for chunk 1 of gtest"
         tos = TryOptionSyntax('try: -u gtest-1[linux,win32]', graph_with_jobs)
         self.assertEqual(sorted(tos.unittests), sorted([
             {'test': 'gtest', 'platforms': ['linux', 'win32'], 'only_chunks': set('1')},
         ]))
 
     def test_u_chunks_platform_alias(self):
--- a/taskcluster/taskgraph/try_option_syntax.py
+++ b/taskcluster/taskgraph/try_option_syntax.py
@@ -62,16 +62,40 @@ UNITTEST_ALIASES = {
     'web-platform-test': re.compile(r'^web-platform-tests.*$'),
     'web-platform-tests': re.compile(r'^web-platform-tests.*$'),
     'web-platform-tests-e10s': re.compile(r'^web-platform-tests-e10s.*$'),
     'web-platform-tests-reftests': re.compile(r'^web-platform-tests-reftests.*$'),
     'web-platform-tests-reftests-e10s': re.compile(r'^web-platform-tests-reftests-e10s.*$'),
     'xpcshell': re.compile(r'^xpcshell.*$'),
 }
 
+# unittest platforms can be specified by substring of the "pretty name", which
+# is basically the old Buildbot builder name.  This dict has {pretty name,
+# [test_platforms]} translations, This includes only the most commonly-used
+# substrings.  This is intended only for backward-compatibility.  New test
+# platforms should have their `test_platform` spelled out fully in try syntax.
+UNITTEST_PLATFORM_PRETTY_NAMES = {
+    'Ubuntu': ['linux', 'linux64'],
+    'x64': ['linux64'],
+    # other commonly-used substrings for platforms not yet supported with
+    # in-tree taskgraphs:
+    #'10.10': [..TODO..],
+    #'10.10.5': [..TODO..],
+    #'10.6': [..TODO..],
+    #'10.8': [..TODO..],
+    #'Android 2.3 API9': [..TODO..],
+    #'Android 4.3 API15+': [..TODO..],
+    #'Windows 7':  [..TODO..],
+    #'Windows 7 VM': [..TODO..],
+    #'Windows 8':  [..TODO..],
+    #'Windows XP': [..TODO..],
+    #'win32': [..TODO..],
+    #'win64': [..TODO..],
+}
+
 # We have a few platforms for which we want to do some "extra" builds, or at
 # least build-ish things.  Sort of.  Anyway, these other things are implemented
 # as different "platforms".
 RIDEALONG_BUILDS = {
     'linux64': [
         'sm-plain',
         'sm-arm-sim',
         'sm-compacting',
@@ -184,17 +208,21 @@ class TryOptionSyntax(object):
             - unittest_arg is == 'all' (meaning use the list of jobs for that job type)
             - unittest_arg is comma string which needs to be parsed
         '''
 
         # Empty job list case...
         if unittest_arg is None or unittest_arg == 'none':
             return []
 
-        tests = self.parse_test_opts(unittest_arg)
+        all_platforms = set([t.attributes['test_platform']
+                             for t in full_task_graph.tasks.itervalues()
+                             if 'test_platform' in t.attributes])
+
+        tests = self.parse_test_opts(unittest_arg, all_platforms)
 
         if not tests:
             return []
 
         all_tests = set([t.attributes['unittest_try_name']
                          for t in full_task_graph.tasks.itervalues()
                          if 'unittest_try_name' in t.attributes])
 
@@ -207,38 +235,57 @@ class TryOptionSyntax(object):
                 # If there are platform restrictions copy them across the list.
                 if 'platforms' in all_entry:
                     entry['platforms'] = list(all_entry['platforms'])
                 results.append(entry)
             return self.parse_test_chunks(all_tests, results)
         else:
             return self.parse_test_chunks(all_tests, tests)
 
-    def parse_test_opts(self, input_str):
+    def parse_test_opts(self, input_str, all_platforms):
         '''
-        Test argument parsing is surprisingly complicated with the "restrictions"
-        logic this function is responsible for parsing this out into a easier to
-        work with structure like { test: '..', platforms: ['..'] }
+        Parse `testspec,testspec,..`, where each testspec is a test name
+        optionally followed by a list of platforms or negated platforms in
+        `[]`.
         '''
 
         # Final results which we will return.
         tests = []
 
         cur_test = {}
         token = ''
         in_platforms = False
 
+        def normalize_platforms():
+            if 'platforms' not in cur_test:
+                return
+            platforms = []
+            for platform in cur_test['platforms']:
+                if platform[0] == '-':
+                    platforms = [p for p in platforms if p != platform[1:]]
+                else:
+                    platforms.append(platform)
+            cur_test['platforms'] = platforms
+
         def add_test(value):
+            normalize_platforms()
             cur_test['test'] = value.strip()
             tests.insert(0, cur_test)
 
         def add_platform(value):
-            # Ensure platforms exists...
-            cur_test['platforms'] = cur_test.get('platforms', [])
-            cur_test['platforms'].insert(0, value.strip())
+            platform = value.strip()
+            if platform[0] == '-':
+                negated = True
+                platform = platform[1:]
+            else:
+                negated = False
+            platforms = UNITTEST_PLATFORM_PRETTY_NAMES.get(platform, [platform])
+            if negated:
+                platforms = ["-" + p for p in platforms]
+            cur_test['platforms'] = platforms + cur_test.get('platforms', [])
 
         # This might be somewhat confusing but we parse the string _backwards_ so
         # there is no ambiguity over what state we are in.
         for char in reversed(input_str):
 
             # , indicates exiting a state
             if char == ',':