Bug 1312520 - Store and process manifest-level defaults in the build system separately from individual tests. r=gps draft
authorChris Manchester <cmanchester@mozilla.com>
Mon, 24 Oct 2016 20:10:19 -0700
changeset 428948 07e91f2fd68a81c084f219e246544ed1f4c3c38c
parent 428947 37618a19d16fc90962cdc51f247fd6137b199a93
child 534853 e2c1b35408360d702bca1a846f5fc2717ba6ee8b
push id33436
push userbmo:cmanchester@mozilla.com
push dateMon, 24 Oct 2016 20:11:53 +0000
reviewersgps
bugs1312520
milestone52.0a1
Bug 1312520 - Store and process manifest-level defaults in the build system separately from individual tests. r=gps MozReview-Commit-ID: 1dSMAaOqToJ
python/mozbuild/mozbuild/backend/common.py
python/mozbuild/mozbuild/frontend/data.py
python/mozbuild/mozbuild/frontend/emitter.py
python/mozbuild/mozbuild/test/backend/test_recursivemake.py
python/mozbuild/mozbuild/test/frontend/data/test-manifest-dupes/mochitest.ini
python/mozbuild/mozbuild/test/test_testing.py
python/mozbuild/mozbuild/testing.py
--- a/python/mozbuild/mozbuild/backend/common.py
+++ b/python/mozbuild/mozbuild/backend/common.py
@@ -170,41 +170,36 @@ class TestManager(object):
 
     def __init__(self, config):
         self.config = config
         self.topsrcdir = mozpath.normpath(config.topsrcdir)
 
         self.tests_by_path = defaultdict(list)
         self.installs_by_path = defaultdict(list)
         self.deferred_installs = set()
-        self.manifest_default_support_files = {}
+        self.manifest_defaults = {}
 
-    def add(self, t, flavor, topsrcdir, default_supp_files):
+    def add(self, t, flavor, topsrcdir):
         t = dict(t)
         t['flavor'] = flavor
 
         path = mozpath.normpath(t['path'])
         assert mozpath.basedir(path, [topsrcdir])
 
         key = path[len(topsrcdir)+1:]
         t['file_relpath'] = key
         t['dir_relpath'] = mozpath.dirname(key)
 
-        # Support files are propagated from the default section to individual
-        # tests by the manifest parser, but we end up storing a lot of
-        # redundant data due to the huge number of support files.
-        # So if we have support files that are the same as the manifest default
-        # we track that separately, per-manifest instead of per-test, to save
-        # space.
-        supp_files = t.get('support-files')
-        if supp_files and supp_files == default_supp_files:
-            self.manifest_default_support_files[t['manifest']] = default_supp_files
-            del t['support-files']
+        self.tests_by_path[key].append(t)
 
-        self.tests_by_path[key].append(t)
+    def add_defaults(self, manifest):
+        if not hasattr(manifest, 'manifest_defaults'):
+            return
+        for sub_manifest, defaults in manifest.manifest_defaults.items():
+            self.manifest_defaults[sub_manifest] = defaults
 
     def add_installs(self, obj, topsrcdir):
         for src, (dest, _) in obj.installs.iteritems():
             key = src[len(topsrcdir)+1:]
             self.installs_by_path[key].append((src, dest))
         for src, pat, dest in obj.pattern_installs:
             key = mozpath.join(src[len(topsrcdir)+1:], pat)
             self.installs_by_path[key].append((src, pat, dest))
@@ -231,18 +226,18 @@ class CommonBackend(BuildBackend):
         self._configs = set()
         self._ipdl_sources = set()
 
     def consume_object(self, obj):
         self._configs.add(obj.config)
 
         if isinstance(obj, TestManifest):
             for test in obj.tests:
-                self._test_manager.add(test, obj.flavor, obj.topsrcdir,
-                                       obj.default_support_files)
+                self._test_manager.add(test, obj.flavor, obj.topsrcdir)
+            self._test_manager.add_defaults(obj.manifest)
             self._test_manager.add_installs(obj, obj.topsrcdir)
 
         elif isinstance(obj, XPIDLFile):
             # TODO bug 1240134 tracks not processing XPIDL files during
             # artifact builds.
             self._idl_manager.register_idl(obj)
 
         elif isinstance(obj, ConfigFileSubstitution):
@@ -371,18 +366,20 @@ class CommonBackend(BuildBackend):
         self._handle_ipdl_sources(ipdl_dir, sorted_ipdl_sources, unified_source_mapping)
 
         for config in self._configs:
             self.backend_input_files.add(config.source)
 
         # Write out a machine-readable file describing every test.
         topobjdir = self.environment.topobjdir
         with self._write_file(mozpath.join(topobjdir, 'all-tests.json')) as fh:
-            json.dump([self._test_manager.tests_by_path,
-                       self._test_manager.manifest_default_support_files], fh)
+            json.dump(self._test_manager.tests_by_path, fh)
+
+        with self._write_file(mozpath.join(topobjdir, 'test-defaults.json')) as fh:
+            json.dump(self._test_manager.manifest_defaults, fh)
 
         path = mozpath.join(self.environment.topobjdir, 'test-installs.json')
         with self._write_file(path) as fh:
             json.dump({k: v for k, v in self._test_manager.installs_by_path.items()
                        if k in self._test_manager.deferred_installs},
                       fh,
                       sort_keys=True,
                       indent=4)
--- a/python/mozbuild/mozbuild/frontend/data.py
+++ b/python/mozbuild/mozbuild/frontend/data.py
@@ -637,21 +637,16 @@ class TestManifest(ContextDerived):
         'manifest_relpath',
 
         # The relative path of the parsed manifest within the objdir.
         'manifest_obj_relpath',
 
         # If this manifest is a duplicate of another one, this is the
         # manifestparser.TestManifest of the other one.
         'dupe_manifest',
-
-        # The support files appearing in the DEFAULT section of this
-        # manifest. This enables a space optimization in all-tests.json,
-        # see the comment in mozbuild/backend/common.py.
-        'default_support_files',
     )
 
     def __init__(self, context, path, manifest, flavor=None,
             install_prefix=None, relpath=None, dupe_manifest=False):
         ContextDerived.__init__(self, context)
 
         assert flavor in all_test_flavors()
 
@@ -663,17 +658,16 @@ class TestManifest(ContextDerived):
         self.manifest_relpath = relpath
         self.manifest_obj_relpath = relpath
         self.dupe_manifest = dupe_manifest
         self.installs = {}
         self.pattern_installs = []
         self.tests = []
         self.external_installs = set()
         self.deferred_installs = set()
-        self.default_support_files = None
 
 
 class LocalInclude(ContextDerived):
     """Describes an individual local include path."""
 
     __slots__ = (
         'path',
     )
--- a/python/mozbuild/mozbuild/frontend/emitter.py
+++ b/python/mozbuild/mozbuild/frontend/emitter.py
@@ -1195,28 +1195,26 @@ class TreeMetadataEmitter(LoggingMixin):
 
         path = mozpath.normpath(mozpath.join(context.srcdir, manifest_path))
         manifest_dir = mozpath.dirname(path)
         manifest_reldir = mozpath.dirname(mozpath.relpath(path,
             context.config.topsrcdir))
         install_prefix = mozpath.join(install_root, install_subdir)
 
         try:
-            defaults = mpmanifest.manifest_defaults[os.path.normpath(path)]
             if not mpmanifest.tests:
                 raise SandboxValidationError('Empty test manifest: %s'
                     % path, context)
 
+            defaults = mpmanifest.manifest_defaults[os.path.normpath(path)]
             obj = TestManifest(context, path, mpmanifest, flavor=flavor,
                 install_prefix=install_prefix,
                 relpath=mozpath.join(manifest_reldir, mozpath.basename(path)),
                 dupe_manifest='dupe-manifest' in defaults)
 
-            obj.default_support_files = defaults.get('support-files')
-
             filtered = mpmanifest.tests
 
             # Jetpack add-on tests are expected to be generated during the
             # build process so they won't exist here.
             if flavor != 'jetpack-addon':
                 missing = [t['name'] for t in filtered if not os.path.exists(t['path'])]
                 if missing:
                     raise SandboxValidationError('Test manifest (%s) lists '
@@ -1258,16 +1256,19 @@ class TreeMetadataEmitter(LoggingMixin):
                 if package_tests:
                     manifest_relpath = mozpath.relpath(test['path'],
                         mozpath.dirname(test['manifest']))
                     obj.installs[mozpath.normpath(test['path'])] = \
                         ((mozpath.join(out_dir, manifest_relpath)), True)
 
                 process_support_files(test)
 
+            for path, m_defaults in mpmanifest.manifest_defaults.items():
+                process_support_files(m_defaults)
+
             # We also copy manifests into the output directory,
             # including manifests from [include:foo] directives.
             for mpath in mpmanifest.manifests():
                 mpath = mozpath.normpath(mpath)
                 out_path = mozpath.join(out_dir, mozpath.basename(mpath))
                 obj.installs[mpath] = (out_path, False)
 
             # Some manifests reference files that are auto generated as
@@ -1276,18 +1277,18 @@ class TreeMetadataEmitter(LoggingMixin):
             # FUTURE we should be able to detect autogenerated files from
             # other build metadata. Once we do that, we can get rid of this.
             for f in defaults.get('generated-files', '').split():
                 # We re-raise otherwise the stack trace isn't informative.
                 try:
                     del obj.installs[mozpath.join(manifest_dir, f)]
                 except KeyError:
                     raise SandboxValidationError('Error processing test '
-                       'manifest %s: entry in generated-files not present '
-                       'elsewhere in manifest: %s' % (path, f), context)
+                        'manifest %s: entry in generated-files not present '
+                        'elsewhere in manifest: %s' % (path, f), context)
 
             yield obj
         except (AssertionError, Exception):
             raise SandboxValidationError('Error processing test '
                 'manifest file %s: %s' % (path,
                     '\n'.join(traceback.format_exception(*sys.exc_info()))),
                 context)
 
--- a/python/mozbuild/mozbuild/test/backend/test_recursivemake.py
+++ b/python/mozbuild/mozbuild/test/backend/test_recursivemake.py
@@ -523,17 +523,17 @@ class TestRecursiveMakeBackend(BackendTe
             '[include:dir1/xpcshell.ini]',
             '[include:xpcshell.ini]',
         ])
 
         all_tests_path = mozpath.join(env.topobjdir, 'all-tests.json')
         self.assertTrue(os.path.exists(all_tests_path))
 
         with open(all_tests_path, 'rt') as fh:
-            o, _ = json.load(fh)
+            o = json.load(fh)
 
             self.assertIn('xpcshell.js', o)
             self.assertIn('dir1/test_bar.js', o)
 
             self.assertEqual(len(o['xpcshell.js']), 1)
 
     def test_test_manifest_pattern_matches_recorded(self):
         """Pattern matches in test manifests' support-files should be recorded."""
@@ -849,18 +849,17 @@ class TestRecursiveMakeBackend(BackendTe
     def test_install_manifests_package_tests(self):
         """Ensure test suites honor package_tests=False."""
         env = self._consume('test-manifests-package-tests', RecursiveMakeBackend)
 
         all_tests_path = mozpath.join(env.topobjdir, 'all-tests.json')
         self.assertTrue(os.path.exists(all_tests_path))
 
         with open(all_tests_path, 'rt') as fh:
-            o, _ = json.load(fh)
-
+            o = json.load(fh)
             self.assertIn('mochitest.js', o)
             self.assertIn('not_packaged.java', o)
 
         man_dir = mozpath.join(env.topobjdir, '_build_manifests', 'install')
         self.assertTrue(os.path.isdir(man_dir))
 
         full = mozpath.join(man_dir, '_test_files')
         self.assertTrue(os.path.exists(full))
--- a/python/mozbuild/mozbuild/test/frontend/data/test-manifest-dupes/mochitest.ini
+++ b/python/mozbuild/mozbuild/test/frontend/data/test-manifest-dupes/mochitest.ini
@@ -1,8 +1,7 @@
 # Any copyright is dedicated to the Public Domain.
 # http://creativecommons.org/publicdomain/zero/1.0/
 
 [DEFAULT]
-support-files = foo.js bar.js
+support-files = bar.js foo.js bar.js
 
 [test_baz.js]
-support-files = bar.js
\ No newline at end of file
--- a/python/mozbuild/mozbuild/test/test_testing.py
+++ b/python/mozbuild/mozbuild/test/test_testing.py
@@ -17,17 +17,17 @@ from mozunit import main
 from mozbuild.base import MozbuildObject
 from mozbuild.testing import (
     TestMetadata,
     TestResolver,
 )
 
 
 ALL_TESTS_JSON = b'''
-[{
+{
     "accessible/tests/mochitest/actions/test_anchors.html": [
         {
             "dir_relpath": "accessible/tests/mochitest/actions",
             "expected": "pass",
             "file_relpath": "accessible/tests/mochitest/actions/test_anchors.html",
             "flavor": "a11y",
             "here": "/Users/gps/src/firefox/accessible/tests/mochitest/actions",
             "manifest": "/Users/gps/src/firefox/accessible/tests/mochitest/actions/a11y.ini",
@@ -149,38 +149,45 @@ ALL_TESTS_JSON = b'''
             "manifest": "/home/chris/m-c/devtools/client/markupview/test/browser.ini",
             "name": "browser_markupview_copy_image_data.js",
             "path": "/home/chris/m-c/obj-dbg/_tests/testing/mochitest/browser/devtools/client/markupview/test/browser_markupview_copy_image_data.js",
             "relpath": "devtools/client/markupview/test/browser_markupview_copy_image_data.js",
             "subsuite": "devtools",
             "tags": "devtools"
         }
    ]
-}, {
-   "/Users/gps/src/firefox/toolkit/mozapps/update/test/unit/xpcshell_updater.ini": "\\ndata/**\\nxpcshell_updater.ini"
-}]'''.strip()
+}'''.strip()
+
+TEST_DEFAULTS = b'''{
+    "/Users/gps/src/firefox/toolkit/mozapps/update/test/unit/xpcshell_updater.ini": {"support-files": "\\ndata/**\\nxpcshell_updater.ini"}
+}'''
 
 
 class Base(unittest.TestCase):
     def setUp(self):
         self._temp_files = []
 
     def tearDown(self):
         for f in self._temp_files:
             del f
 
         self._temp_files = []
 
     def _get_test_metadata(self):
-        f = NamedTemporaryFile()
-        f.write(ALL_TESTS_JSON)
-        f.flush()
-        self._temp_files.append(f)
+        all_tests = NamedTemporaryFile()
+        all_tests.write(ALL_TESTS_JSON)
+        all_tests.flush()
+        self._temp_files.append(all_tests)
 
-        return TestMetadata(filename=f.name)
+        test_defaults = NamedTemporaryFile()
+        test_defaults.write(TEST_DEFAULTS)
+        test_defaults.flush()
+        self._temp_files.append(test_defaults)
+
+        return TestMetadata(all_tests.name, test_defaults=test_defaults.name)
 
 
 class TestTestMetadata(Base):
     def test_load(self):
         t = self._get_test_metadata()
         self.assertEqual(len(t._tests_by_path), 8)
 
         self.assertEqual(len(list(t.tests_with_flavor('xpcshell'))), 3)
@@ -241,16 +248,18 @@ class TestTestResolver(Base):
             shutil.rmtree(d)
 
     def _get_resolver(self):
         topobjdir = tempfile.mkdtemp()
         self._temp_dirs.append(topobjdir)
 
         with open(os.path.join(topobjdir, 'all-tests.json'), 'wt') as fh:
             fh.write(ALL_TESTS_JSON)
+        with open(os.path.join(topobjdir, 'test-defaults.json'), 'wt') as fh:
+            fh.write(TEST_DEFAULTS)
 
         o = MozbuildObject(self.FAKE_TOPSRCDIR, None, None, topobjdir=topobjdir)
 
         # Monkey patch the test resolver to avoid tests failing to find make
         # due to our fake topscrdir.
         TestResolver._run_make = lambda *a, **b: None
 
         return o._spawn(TestResolver)
--- a/python/mozbuild/mozbuild/testing.py
+++ b/python/mozbuild/mozbuild/testing.py
@@ -46,40 +46,39 @@ def rewrite_test_base(test, new_base, ho
 
 class TestMetadata(object):
     """Holds information about tests.
 
     This class provides an API to query tests active in the build
     configuration.
     """
 
-    def __init__(self, filename=None):
+    def __init__(self, all_tests, test_defaults=None):
         self._tests_by_path = OrderedDefaultDict(list)
         self._tests_by_flavor = defaultdict(set)
         self._test_dirs = set()
 
-        if filename:
-            with open(filename, 'rt') as fh:
-                test_data, manifest_support_files = json.load(fh)
-
-                for path, tests in test_data.items():
-                    for metadata in tests:
-                        # Many tests inherit their "support-files" from a manifest
-                        # level default, so we store these separately to save
-                        # disk space, and propagate them to each test when
-                        # de-serializing here.
-                        manifest = metadata['manifest']
-                        support_files = manifest_support_files.get(manifest)
-                        if support_files and 'support-files' not in metadata:
-                            metadata['support-files'] = support_files
-                        self._tests_by_path[path].append(metadata)
-                        self._test_dirs.add(os.path.dirname(path))
-
-                        flavor = metadata.get('flavor')
-                        self._tests_by_flavor[flavor].add(path)
+        with open(all_tests, 'rt') as fh:
+            test_data = json.load(fh)
+        defaults = None
+        if test_defaults:
+            with open(test_defaults, 'rt') as fh:
+                defaults = json.load(fh)
+        for path, tests in test_data.items():
+            for metadata in tests:
+                if defaults:
+                    manifest = metadata['manifest']
+                    manifest_defaults = defaults.get(manifest)
+                    if manifest_defaults:
+                        metadata = manifestparser.combine_fields(manifest_defaults,
+                                                                 metadata)
+                self._tests_by_path[path].append(metadata)
+                self._test_dirs.add(os.path.dirname(path))
+                flavor = metadata.get('flavor')
+                self._tests_by_flavor[flavor].add(path)
 
     def tests_with_flavor(self, flavor):
         """Obtain all tests having the specified flavor.
 
         This is a generator of dicts describing each test.
         """
 
         for path in sorted(self._tests_by_flavor.get(flavor, [])):
@@ -178,18 +177,21 @@ class TestResolver(MozbuildObject):
         MozbuildObject.__init__(self, *args, **kwargs)
 
         # If installing tests is going to result in re-generating the build
         # backend, we need to do this here, so that the updated contents of
         # all-tests.json make it to the set of tests to run.
         self._run_make(target='run-tests-deps', pass_thru=True,
                        print_directory=False)
 
-        self._tests = TestMetadata(filename=os.path.join(self.topobjdir,
-            'all-tests.json'))
+        self._tests = TestMetadata(os.path.join(self.topobjdir,
+                                                'all-tests.json'),
+                                   test_defaults=os.path.join(self.topobjdir,
+                                                              'test-defaults.json'))
+
         self._test_rewrites = {
             'a11y': os.path.join(self.topobjdir, '_tests', 'testing',
                 'mochitest', 'a11y'),
             'browser-chrome': os.path.join(self.topobjdir, '_tests', 'testing',
                 'mochitest', 'browser'),
             'jetpack-package': os.path.join(self.topobjdir, '_tests', 'testing',
                 'mochitest', 'jetpack-package'),
             'jetpack-addon': os.path.join(self.topobjdir, '_tests', 'testing',
@@ -498,17 +500,18 @@ def install_test_files(topsrcdir, topobj
                 remove_unaccounted=False)
 
 
 # Convenience methods for test manifest reading.
 def read_manifestparser_manifest(context, manifest_path):
     path = mozpath.normpath(mozpath.join(context.srcdir, manifest_path))
     return manifestparser.TestManifest(manifests=[path], strict=True,
                                        rootdir=context.config.topsrcdir,
-                                       finder=context._finder)
+                                       finder=context._finder,
+                                       omit_defaults=True)
 
 def read_reftest_manifest(context, manifest_path):
     import reftest
     path = mozpath.normpath(mozpath.join(context.srcdir, manifest_path))
     manifest = reftest.ReftestManifest(finder=context._finder)
     manifest.load(path)
     return manifest