Bug 1313716 - Don't provide a blank subsuite as a default in the manifestparser. r=ahal
authorChris Manchester <cmanchester@mozilla.com>
Fri, 28 Oct 2016 11:07:21 -0700
changeset 320131 425c0602ccb486822c4974aa5f7d367597d2b259
parent 320130 c2b23c06acfe4086c26e844ffb9bd94e01c6a252
child 320132 97db1163c72973f0027e25591b09819a903c2b50
push id30883
push userryanvm@gmail.com
push dateSat, 29 Oct 2016 13:12:57 +0000
treeherdermozilla-central@dc422956242b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersahal
bugs1313716
milestone52.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1313716 - Don't provide a blank subsuite as a default in the manifestparser. r=ahal This causes consumers managing defaults themselves to fail to find a default subsuite for tests, because the manifestparser will have provided a blank default value by the time they incorporate defaults into a test definition. This patch removes the provided defaults and updates a number of places assuming the 'subsuite' field is always present. MozReview-Commit-ID: 1jPy52VmEPr
testing/mach_commands.py
testing/mochitest/mach_commands.py
testing/mozbase/manifestparser/manifestparser/manifestparser.py
testing/mozbase/manifestparser/tests/test_convert_directory.py
testing/mozbase/manifestparser/tests/test_default_overrides.py
testing/mozbase/manifestparser/tests/test_manifestparser.py
--- a/testing/mach_commands.py
+++ b/testing/mach_commands.py
@@ -315,17 +315,17 @@ class Test(MachCommandBase):
                 res = self._mach_context.commands.dispatch(
                     suite['mach_command'], self._mach_context,
                     **suite['kwargs'])
                 if res:
                     status = res
 
         buckets = {}
         for test in run_tests:
-            key = (test['flavor'], test['subsuite'])
+            key = (test['flavor'], test.get('subsuite', ''))
             buckets.setdefault(key, []).append(test)
 
         for (flavor, subsuite), tests in sorted(buckets.items()):
             if flavor not in TEST_FLAVORS:
                 print(UNKNOWN_FLAVOR % flavor)
                 status = 1
                 continue
 
--- a/testing/mochitest/mach_commands.py
+++ b/testing/mochitest/mach_commands.py
@@ -373,27 +373,27 @@ class MachCommands(MachCommandBase):
 
         suites = defaultdict(list)
         unsupported = set()
         for test in tests:
             # Filter out non-mochitests and unsupported flavors.
             if test['flavor'] not in ALL_FLAVORS:
                 continue
 
-            key = (test['flavor'], test['subsuite'])
+            key = (test['flavor'], test.get('subsuite', ''))
             if test['flavor'] not in flavors:
                 unsupported.add(key)
                 continue
 
             if subsuite == 'default':
                 # "--subsuite default" means only run tests that don't have a subsuite
-                if test['subsuite']:
+                if test.get('subsuite'):
                     unsupported.add(key)
                     continue
-            elif subsuite and test['subsuite'] != subsuite:
+            elif subsuite and test.get('subsuite', '') != subsuite:
                 unsupported.add(key)
                 continue
 
             suites[key].append(test)
 
         # This is a hack to introduce an option in mach to not send
         # filtered tests to the mochitest harness. Mochitest harness will read
         # the master manifest in that case.
--- a/testing/mozbase/manifestparser/manifestparser/manifestparser.py
+++ b/testing/mozbase/manifestparser/manifestparser/manifestparser.py
@@ -147,20 +147,16 @@ class ManifestParser(object):
         sections = read_ini(fp=fp, variables=defaults, strict=self.strict,
                             handle_defaults=self._handle_defaults)
         self.manifest_defaults[filename] = defaults
 
         parent_section_found = False
 
         # get the tests
         for section, data in sections:
-            subsuite = ''
-            if 'subsuite' in data:
-                subsuite = data['subsuite']
-
             # In case of defaults only, no other section than parent: has to
             # be processed.
             if defaults_only and not section.startswith('parent:'):
                 continue
 
             # read the parent manifest if specified
             if section.startswith('parent:'):
                 parent_section_found = True
@@ -216,17 +212,16 @@ class ManifestParser(object):
                 # When the rootdir is unknown, the relpath needs to be left
                 # unchanged. We use an empty string as rootdir in that case,
                 # which leaves relpath unchanged after slicing.
                 if path.startswith(rootdir):
                     _relpath = path[len(rootdir):]
                 else:
                     _relpath = relpath(path, rootdir)
 
-            test['subsuite'] = subsuite
             test['path'] = path
             test['relpath'] = _relpath
 
             if parentmanifest is not None:
                 # If a test was included by a parent manifest we may need to
                 # indicate that in the test object for the sake of identifying
                 # a test, particularly in the case a test file is included by
                 # multiple manifests.
--- a/testing/mozbase/manifestparser/tests/test_convert_directory.py
+++ b/testing/mozbase/manifestparser/tests/test_convert_directory.py
@@ -55,26 +55,22 @@ class TestDirectoryConversion(unittest.T
         stub = self.create_stub()
         try:
             stub = stub.replace(os.path.sep, "/")
             self.assertTrue(os.path.exists(stub) and os.path.isdir(stub))
 
             # Make a manifest for it
             manifest = convert([stub])
             out_tmpl = """[%(stub)s/bar]
-subsuite = 
 
 [%(stub)s/fleem]
-subsuite = 
 
 [%(stub)s/foo]
-subsuite = 
 
 [%(stub)s/subdir/subfile]
-subsuite = 
 
 """  # noqa
             self.assertEqual(str(manifest), out_tmpl % dict(stub=stub))
         except:
             raise
         finally:
             shutil.rmtree(stub)  # cleanup
 
--- a/testing/mozbase/manifestparser/tests/test_default_overrides.py
+++ b/testing/mozbase/manifestparser/tests/test_default_overrides.py
@@ -2,16 +2,17 @@
 
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this file,
 # You can obtain one at http://mozilla.org/MPL/2.0/.
 
 import os
 import unittest
 from manifestparser import ManifestParser
+from manifestparser import combine_fields
 
 here = os.path.dirname(os.path.abspath(__file__))
 
 
 class TestDefaultSkipif(unittest.TestCase):
     """Tests applying a skip-if condition in [DEFAULT] and || with the value for the test"""
 
     def test_defaults(self):
@@ -88,10 +89,27 @@ class TestOmitDefaults(unittest.TestCase
         }
         for path, defaults in expected_defaults.items():
             self.assertIn(path, parser.manifest_defaults)
             actual_defaults = parser.manifest_defaults[path]
             for key, value in defaults.items():
                 self.assertIn(key, actual_defaults)
                 self.assertEqual(value, actual_defaults[key])
 
+
+class TestSubsuiteDefaults(unittest.TestCase):
+    """Test that subsuites are handled correctly when managing defaults
+    outside of the manifest parser."""
+    def test_subsuite_defaults(self):
+        manifest = os.path.join(here, 'default-subsuite.ini')
+        parser = ManifestParser(manifests=(manifest,), handle_defaults=False)
+        expected_subsuites = {
+            'test1': 'baz',
+            'test2': 'foo',
+        }
+        defaults = parser.manifest_defaults[manifest]
+        for test in parser.tests:
+            value = combine_fields(defaults, test)
+            self.assertEqual(expected_subsuites[value['name']],
+                             value['subsuite'])
+
 if __name__ == '__main__':
     unittest.main()
--- a/testing/mozbase/manifestparser/tests/test_manifestparser.py
+++ b/testing/mozbase/manifestparser/tests/test_manifestparser.py
@@ -109,22 +109,20 @@ class TestManifestParser(unittest.TestCa
 
         # Write the output to a manifest:
         buffer = StringIO()
         parser.write(fp=buffer, global_kwargs={'foo': 'bar'})
         expected_output = """[DEFAULT]
 foo = bar
 
 [fleem]
-subsuite = 
 
 [include/flowers]
 blue = ocean
 red = roses
-subsuite = 
 yellow = submarine"""  # noqa
 
         self.assertEqual(buffer.getvalue().strip(),
                          expected_output)
 
     def test_invalid_path(self):
         """
         Test invalid path should not throw when not strict
@@ -152,17 +150,17 @@ yellow = submarine"""  # noqa
         # DEFAULT values should be the ones from level 1
         self.assertEqual(parser.get('name', x='level_1'),
                          ['test_3'])
 
         # Write the output to a manifest:
         buffer = StringIO()
         parser.write(fp=buffer, global_kwargs={'x': 'level_1'})
         self.assertEqual(buffer.getvalue().strip(),
-                         '[DEFAULT]\nx = level_1\n\n[test_3]\nsubsuite =')
+                         '[DEFAULT]\nx = level_1\n\n[test_3]')
 
     def test_parent_defaults(self):
         """
         Test downstream variables should overwrite upstream variables
         """
         parent_example = os.path.join(here, 'parent', 'level_1', 'level_2',
                                       'level_3', 'level_3_default.ini')
         parser = ManifestParser(manifests=(parent_example,))
@@ -177,17 +175,17 @@ yellow = submarine"""  # noqa
         # DEFAULT values should be the ones from level 3
         self.assertEqual(parser.get('name', x='level_3'),
                          ['test_3'])
 
         # Write the output to a manifest:
         buffer = StringIO()
         parser.write(fp=buffer, global_kwargs={'x': 'level_3'})
         self.assertEqual(buffer.getvalue().strip(),
-                         '[DEFAULT]\nx = level_3\n\n[test_3]\nsubsuite =')
+                         '[DEFAULT]\nx = level_3\n\n[test_3]')
 
     def test_parent_defaults_include(self):
         parent_example = os.path.join(here, 'parent', 'include', 'manifest.ini')
         parser = ManifestParser(manifests=(parent_example,))
 
         # global defaults should inherit all includes
         self.assertEqual(parser.get('name', top='data'),
                          ['testFirst.js', 'testSecond.js'])