Bug 1261456 - Combine support-files listed in [DEFAULT] with any listed per-test rather than overriding. r=gps
authorChris Manchester <cmanchester@mozilla.com>
Mon, 11 Apr 2016 11:21:20 -0700
changeset 292623 68779538bf0028b89d5d4268c5619ca15037e92f
parent 292622 76411e442ff3c9e758edba9bad6bea9ef6d0ab06
child 292624 d4f4469ef5ff98c6b48da34fc963572aa6b946b5
push id74926
push usercmanchester@mozilla.com
push dateMon, 11 Apr 2016 18:21:25 +0000
treeherdermozilla-inbound@68779538bf00 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgps
bugs1261456
milestone48.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 1261456 - Combine support-files listed in [DEFAULT] with any listed per-test rather than overriding. r=gps This requires a change to how we process test manifests in the build system: now, whenever we see a support file mentioned in a manifest, we require that file isn't already in that test's support files, but if we see a support file that was already seen in some other test, the entry is ignored, but it is not an error. As a result of this change, several duplicate support-files entries needed to be removed. MozReview-Commit-ID: G0juyxzcaB8
dom/base/test/mochitest.ini
dom/browser-element/mochitest/mochitest.ini
dom/svg/test/mochitest.ini
image/test/unit/xpcshell.ini
python/mozbuild/mozbuild/test/frontend/data/test-manifest-dupes/bar.js
python/mozbuild/mozbuild/test/frontend/data/test-manifest-dupes/foo.js
python/mozbuild/mozbuild/test/frontend/data/test-manifest-dupes/mochitest.ini
python/mozbuild/mozbuild/test/frontend/data/test-manifest-dupes/moz.build
python/mozbuild/mozbuild/test/frontend/data/test-manifest-dupes/test_baz.js
python/mozbuild/mozbuild/test/frontend/test_emitter.py
python/mozbuild/mozbuild/testing.py
testing/mozbase/manifestparser/manifestparser/ini.py
testing/mozbase/manifestparser/tests/default-suppfiles.ini
testing/mozbase/manifestparser/tests/test_default_overrides.py
testing/mozbase/manifestparser/tests/test_default_skipif.py
--- a/dom/base/test/mochitest.ini
+++ b/dom/base/test/mochitest.ini
@@ -668,27 +668,22 @@ skip-if = buildapp == 'b2g'
 [test_bug693875.html]
 [test_bug694754.xhtml]
 [test_bug696301-1.html]
 [test_bug696301-2.html]
 [test_bug698381.html]
 [test_bug698384.html]
 [test_bug704063.html]
 [test_bug704320_http_http.html]
-support-files = referrerHelper.js
 [test_bug704320_http_https.html]
-support-files = referrerHelper.js
 [test_bug704320_https_http.html]
-support-files = referrerHelper.js
 skip-if = buildapp == 'b2g'  # b2g (https://example.com not working bug 1162353)
 [test_bug704320_https_https.html]
-support-files = referrerHelper.js
 skip-if = buildapp == 'b2g'  # b2g (https://example.com not working bug 1162353)
 [test_bug704320_policyset.html]
-support-files = referrerHelper.js
 [test_bug704320_policyset2.html]
 skip-if = os == "mac" # fails intermittently - bug 1101288
 [test_bug704320_preload.html]
 [test_bug707142.html]
 [test_bug708620.html]
 [test_bug711047.html]
 [test_bug711180.html]
 [test_bug719533.html]
@@ -734,19 +729,17 @@ skip-if = (buildapp == 'b2g' && toolkit 
 [test_bug927196.html]
 [test_bug982153.html]
 [test_bug1057176.html]
 [test_bug1070015.html]
 [test_bug1075702.html]
 [test_bug1101364.html]
 skip-if = buildapp == 'b2g' || toolkit == 'android'
 [test_bug1163743.html]
-support-files = referrerHelper.js
 [test_bug1165501.html]
-support-files = referrerHelper.js
 [test_img_referrer.html]
 [test_anchor_area_referrer.html]
 [test_anchor_area_referrer_changing.html]
 [test_anchor_area_referrer_invalid.html]
 [test_anchor_area_referrer_rel.html]
 [test_iframe_referrer.html]
 [test_iframe_referrer_changing.html]
 [test_iframe_referrer_invalid.html]
--- a/dom/browser-element/mochitest/mochitest.ini
+++ b/dom/browser-element/mochitest/mochitest.ini
@@ -78,17 +78,16 @@ support-files =
   browserElement_TargetTop.js
   browserElement_Titlechange.js
   browserElement_TopBarrier.js
   browserElement_VisibilityChange.js
   browserElement_XFrameOptions.js
   browserElement_XFrameOptionsAllowFrom.js
   browserElement_XFrameOptionsDeny.js
   browserElement_XFrameOptionsSameOrigin.js
-  browserElement_XFrameOptionsSameOrigin.js
   browserElement_GetContentDimensions.js
   browserElement_AudioChannel.js
   browserElement_AudioChannel_nested.js
   file_browserElement_ActiveStateChangeOnChangingMutedOrVolume.html
   file_browserElement_AlertInFrame.html
   file_browserElement_AlertInFrame_Inner.html
   file_browserElement_AllowEmbedAppsInNestedOOIframe.html
   file_browserElement_AppFramePermission.html
--- a/dom/svg/test/mochitest.ini
+++ b/dom/svg/test/mochitest.ini
@@ -1,26 +1,25 @@
 [DEFAULT]
 support-files =
-  bbox-helper.svg
+  MutationEventChecker.js
   a_href_destination.svg
   a_href_helper_01.svg
   a_href_helper_02_03.svg
   a_href_helper_04.svg
   animated-svg-image-helper.html
   animated-svg-image-helper.svg
   bbox-helper.svg
   bounds-helper.svg
-  getBBox-method-helper.svg
   dataTypes-helper.svg
   fragments-helper.svg
+  getBBox-method-helper.svg
   getCTM-helper.svg
   getSubStringLength-helper.svg
   matrixUtils.js
-  MutationEventChecker.js
   object-delayed-intrinsic-size.sjs
   pointer-events.js
   scientific-helper.svg
   selectSubString-helper.svg
   switch-helper.svg
   text-helper-scaled.svg
   text-helper-selection.svg
   text-helper.svg
--- a/image/test/unit/xpcshell.ini
+++ b/image/test/unit/xpcshell.ini
@@ -21,17 +21,16 @@ support-files =
   image2jpg32x32-win.png
   image2jpg32x32.jpg
   image2jpg32x32.png
   image3.ico
   image3ico16x16.png
   image3ico32x32.png
   image4.gif
   image4gif16x16bmp24bpp.ico
-  image4gif16x16bmp24bpp.ico
   image4gif16x16bmp32bpp.ico
   image4gif32x32bmp24bpp.ico
   image4gif32x32bmp32bpp.ico
   image_load_helpers.js
 
 
 [test_async_notification.js]
 # Bug 1156452: frequent crash on Android 4.3
new file mode 100644
new file mode 100644
new file mode 100644
--- /dev/null
+++ b/python/mozbuild/mozbuild/test/frontend/data/test-manifest-dupes/mochitest.ini
@@ -0,0 +1,8 @@
+# Any copyright is dedicated to the Public Domain.
+# http://creativecommons.org/publicdomain/zero/1.0/
+
+[DEFAULT]
+support-files = foo.js bar.js
+
+[test_baz.js]
+support-files = bar.js
\ No newline at end of file
new file mode 100644
--- /dev/null
+++ b/python/mozbuild/mozbuild/test/frontend/data/test-manifest-dupes/moz.build
@@ -0,0 +1,4 @@
+# Any copyright is dedicated to the Public Domain.
+# http://creativecommons.org/publicdomain/zero/1.0/
+
+MOCHITEST_MANIFESTS += ['mochitest.ini']
new file mode 100644
--- a/python/mozbuild/mozbuild/test/frontend/test_emitter.py
+++ b/python/mozbuild/mozbuild/test/frontend/test_emitter.py
@@ -424,16 +424,26 @@ class TestEmitterBasic(unittest.TestCase
 
     def test_test_manifest_just_support_files(self):
         """A test manifest with no tests but support-files is not supported."""
         reader = self.reader('test-manifest-just-support')
 
         with self.assertRaisesRegexp(SandboxValidationError, 'Empty test manifest'):
             self.read_topsrcdir(reader)
 
+    def test_test_manifest_dupe_support_files(self):
+        """A test manifest with dupe support-files in a single test is not
+        supported.
+        """
+        reader = self.reader('test-manifest-dupes')
+
+        with self.assertRaisesRegexp(SandboxValidationError, 'bar.js appears multiple times '
+            'in a test manifest under a support-files field, please omit the duplicate entry.'):
+            self.read_topsrcdir(reader)
+
     def test_test_manifest_absolute_support_files(self):
         """Support files starting with '/' are placed relative to the install root"""
         reader = self.reader('test-manifest-absolute-support')
 
         objs = self.read_topsrcdir(reader)
         self.assertEqual(len(objs), 1)
         o = objs[0]
         self.assertEqual(len(o.installs), 3)
--- a/python/mozbuild/mozbuild/testing.py
+++ b/python/mozbuild/mozbuild/testing.py
@@ -293,16 +293,17 @@ WEB_PLATFORM_TESTS_FLAVORS = ('web-platf
 def all_test_flavors():
     return ([v[0] for v in TEST_MANIFESTS.values()] +
             list(REFTEST_FLAVORS) +
             list(WEB_PLATFORM_TESTS_FLAVORS) +
             ['python'])
 
 class TestInstallInfo(object):
     def __init__(self):
+        self.seen = set()
         self.pattern_installs = []
         self.installs = []
         self.external_installs = set()
         self.deferred_installs = set()
 
     def __ior__(self, other):
         self.pattern_installs.extend(other.pattern_installs)
         self.installs.extend(other.installs)
@@ -332,33 +333,44 @@ class SupportFilesConverter(object):
         #                 the tests for this harness (examples are "testing/mochitest",
         #                 "xpcshell").
         #  manifest_dir - Absoulute path to the (srcdir) directory containing the
         #                 manifest that included this test
         #  out_dir - The path relative to $objdir/_tests used as the destination for the
         #            test, based on the relative path to the manifest in the srcdir,
         #            the install_root, and 'install-to-subdir', if present in the manifest.
         info = TestInstallInfo()
-        for thing, seen in self._fields:
-            value = test.get(thing, '')
-            # We need to memoize on the basis of both the path and the output
-            # directory for the benefit of tests specifying 'install-to-subdir'.
-            if (value, out_dir) in seen:
-                continue
-            seen.add((value, out_dir))
+        for field, seen in self._fields:
+            value = test.get(field, '')
             for pattern in value.split():
-                if thing == 'generated-files':
+
+                # We track uniqueness locally (per test) where duplicates are forbidden,
+                # and globally, where they are permitted. If a support file appears multiple
+                # times for a single test, there are unnecessary entries in the manifest. But
+                # many entries will be shared across tests that share defaults.
+                # We need to memoize on the basis of both the path and the output
+                # directory for the benefit of tests specifying 'install-to-subdir'.
+                key = field, pattern, out_dir
+                if key in info.seen:
+                    raise ValueError("%s appears multiple times in a test manifest under a %s field,"
+                                     " please omit the duplicate entry." % (pattern, field))
+                info.seen.add(key)
+                if key in seen:
+                    continue
+                seen.add(key)
+
+                if field == 'generated-files':
                     info.external_installs.add(mozpath.normpath(mozpath.join(out_dir, pattern)))
                 # '!' indicates our syntax for inter-directory support file
                 # dependencies. These receive special handling in the backend.
                 elif pattern[0] == '!':
                     info.deferred_installs.add(pattern)
                 # We only support globbing on support-files because
                 # the harness doesn't support * for head and tail.
-                elif '*' in pattern and thing == 'support-files':
+                elif '*' in pattern and field == 'support-files':
                     info.pattern_installs.append((manifest_dir, pattern, out_dir))
                 # "absolute" paths identify files that are to be
                 # placed in the install_root directory (no globs)
                 elif pattern[0] == '/':
                     full = mozpath.normpath(mozpath.join(manifest_dir,
                                                          mozpath.basename(pattern)))
                     info.installs.append((full, mozpath.join(install_root, pattern[1:])))
                 else:
@@ -370,17 +382,17 @@ class SupportFilesConverter(object):
                     # entry type.
                     if not full.startswith(manifest_dir):
                         # If it's a support file, we install the file
                         # into the current destination directory.
                         # This implementation makes installing things
                         # with custom prefixes impossible. If this is
                         # needed, we can add support for that via a
                         # special syntax later.
-                        if thing == 'support-files':
+                        if field == 'support-files':
                             dest_path = mozpath.join(out_dir,
                                                      os.path.basename(pattern))
                         # If it's not a support file, we ignore it.
                         # This preserves old behavior so things like
                         # head files doesn't get installed multiple
                         # times.
                         else:
                             continue
--- a/testing/mozbase/manifestparser/manifestparser/ini.py
+++ b/testing/mozbase/manifestparser/manifestparser/ini.py
@@ -107,16 +107,22 @@ def read_ini(fp, variables=None, default
 
     # return the default section only if requested
     if defaults_only:
         return [(default, variables)]
 
     # interpret the variables
     def interpret_variables(global_dict, local_dict):
         variables = global_dict.copy()
-        if 'skip-if' in local_dict and 'skip-if' in variables:
-            local_dict['skip-if'] = "(%s) || (%s)" % (variables['skip-if'].split('#')[0], local_dict['skip-if'].split('#')[0])
+
+        # These variables are combinable when they appear both in default
+        # and per-entry.
+        for field_name, pattern in (('skip-if', '(%s) || (%s)'),
+                                    ('support-files', '%s %s')):
+            local_value, global_value = local_dict.get(field_name), variables.get(field_name)
+            if local_value and global_value:
+                local_dict[field_name] = pattern % (global_value.split('#')[0], local_value.split('#')[0])
         variables.update(local_dict)
 
         return variables
 
     sections = [(i, interpret_variables(variables, j)) for i, j in sections]
     return sections
new file mode 100644
--- /dev/null
+++ b/testing/mozbase/manifestparser/tests/default-suppfiles.ini
@@ -0,0 +1,9 @@
+[DEFAULT]
+support-files = foo.js # a comment
+
+[test1]
+[test2]
+support-files = bar.js # another comment
+[test3]
+foo = bar
+
rename from testing/mozbase/manifestparser/tests/test_default_skipif.py
rename to testing/mozbase/manifestparser/tests/test_default_overrides.py
--- a/testing/mozbase/manifestparser/tests/test_default_skipif.py
+++ b/testing/mozbase/manifestparser/tests/test_default_overrides.py
@@ -6,17 +6,17 @@
 
 import os
 import unittest
 from manifestparser import ManifestParser
 
 here = os.path.dirname(os.path.abspath(__file__))
 
 class TestDefaultSkipif(unittest.TestCase):
-    """test applying a skip-if condition in [DEFAULT] and || with the value for the test"""
+    """Tests applying a skip-if condition in [DEFAULT] and || with the value for the test"""
 
 
     def test_defaults(self):
 
         default = os.path.join(here, 'default-skipif.ini')
         parser = ManifestParser(manifests=(default,))
         for test in parser.tests:
             if test['name'] == 'test1':
@@ -27,10 +27,27 @@ class TestDefaultSkipif(unittest.TestCas
                 self.assertEqual(test['skip-if'], "(os == 'win' && debug ) || (os == 'win')")
             elif test['name'] == 'test4':
                 self.assertEqual(test['skip-if'], "(os == 'win' && debug ) || (os == 'win' && debug)")
             elif test['name'] == 'test5':
                 self.assertEqual(test['skip-if'], "os == 'win' && debug # a pesky comment")
             elif test['name'] == 'test6':
                 self.assertEqual(test['skip-if'], "(os == 'win' && debug ) || (debug )")
 
+class TestDefaultSupportFiles(unittest.TestCase):
+    """Tests combining support-files field in [DEFAULT] with the value for a test"""
+
+    def test_defaults(self):
+
+        default = os.path.join(here, 'default-suppfiles.ini')
+        parser = ManifestParser(manifests=(default,))
+        expected_supp_files = {
+            'test1': 'foo.js # a comment',
+            'test2': 'foo.js  bar.js ',
+            'test3': 'foo.js # a comment',
+        }
+        for test in parser.tests:
+            expected = expected_supp_files[test['name']]
+            self.assertEqual(test['support-files'], expected)
+
+
 if __name__ == '__main__':
     unittest.main()