Bug 1261456 - Combine support-files listed in [DEFAULT] with any listed per-test rather than overriding. draft
authorChris Manchester <cmanchester@mozilla.com>
Thu, 07 Apr 2016 14:51:47 -0700
changeset 348684 8cee5d810d12a7aafd558d539ff6b3f078e7b012
parent 348658 96a17fbe9d6fd434cfce5c03f24b5169210fc204
child 517897 4b066c2c6f5337793d1e7b2e31ae4d9c0174d733
push id14878
push usercmanchester@mozilla.com
push dateThu, 07 Apr 2016 21:51:58 +0000
bugs1261456
milestone48.0a1
Bug 1261456 - Combine support-files listed in [DEFAULT] with any listed per-test rather than overriding. 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 it is forbidden, and globally,
+                # where it is permitted. If a support file appears multiple times for a single
+                # test, there are unecessary 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()