Bug 1234955 - Make TEST_DIRS a SPECIAL_VARIABLE. r=gps
authorMike Hommey <mh+mozilla@glandium.org>
Thu, 24 Dec 2015 07:50:57 +0900
changeset 277557 1bcc9c3365866a40b5b6c2a76d71d79065465797
parent 277556 b8386d08ba9da514cc6871fa17bb0db6c4194013
child 277558 9e904a370af3abd5499625355bcaa4d6ee438e14
push id29823
push userryanvm@gmail.com
push dateSat, 26 Dec 2015 01:16:54 +0000
treeherdermozilla-central@691f2e687e46 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgps
bugs1234955
milestone46.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 1234955 - Make TEST_DIRS a SPECIAL_VARIABLE. r=gps Using TEST_DIRS is nothing more than a shortcut for if CONFIG['ENABLE_TESTS']: DIRS += [...] As such, we might as well remove it being a separate variable, and use some Context magic to just fill DIRS when ENABLE_TESTS is set. The security/manager/ssl/tests/unit/moz.build change ensures that the order of DIRS before the change is kept, not because it matters, but because it allows to confirm that nothing else is modified by this change.
config/rules.mk
python/mozbuild/mozbuild/backend/recursivemake.py
python/mozbuild/mozbuild/frontend/context.py
python/mozbuild/mozbuild/frontend/data.py
python/mozbuild/mozbuild/frontend/emitter.py
python/mozbuild/mozbuild/frontend/reader.py
python/mozbuild/mozbuild/test/backend/test_recursivemake.py
python/mozbuild/mozbuild/test/frontend/data/traversal-all-vars/moz.build
python/mozbuild/mozbuild/test/frontend/test_emitter.py
python/mozbuild/mozbuild/test/frontend/test_reader.py
security/manager/ssl/tests/unit/moz.build
--- a/config/rules.mk
+++ b/config/rules.mk
@@ -48,29 +48,16 @@ endif # -s
 
 ################################################################################
 # Testing frameworks support
 ################################################################################
 
 testxpcobjdir = $(DEPTH)/_tests/xpcshell
 
 ifdef ENABLE_TESTS
-
-# Add test directories to the regular directories list. TEST_DIRS should
-# arguably have the same status as other *_DIRS variables. It is coded this way
-# until Makefiles stop using the "ifdef ENABLE_TESTS; DIRS +=" convention.
-#
-# The current developer workflow expects tests to be updated when processing
-# the default target. If we ever change this implementation, the behavior
-# should be preserved or the change should be widely communicated. A
-# consequence of not processing test dir targets during the default target is
-# that changes to tests may not be updated and code could assume to pass
-# locally against non-current test code.
-DIRS += $(TEST_DIRS)
-
 ifdef CPP_UNIT_TESTS
 ifdef COMPILE_ENVIRONMENT
 
 # Compile the tests to $(DIST)/bin.  Make lots of niceties available by default
 # through TestHarness.h, by modifying the list of includes and the libs against
 # which stuff links.
 SIMPLE_PROGRAMS += $(CPP_UNIT_TESTS)
 INCLUDES += -I$(DIST)/include/testing
--- a/python/mozbuild/mozbuild/backend/recursivemake.py
+++ b/python/mozbuild/mozbuild/backend/recursivemake.py
@@ -902,23 +902,16 @@ class RecursiveMakeBackend(CommonBackend
             return (mozpath.relpath(d.translated, base) for d in dirs)
 
         if obj.dirs:
             fh.write('DIRS := %s\n' % ' '.join(
                 relativize(backend_file.objdir, obj.dirs)))
             self._traversal.add(backend_file.relobjdir,
                 dirs=relativize(self.environment.topobjdir, obj.dirs))
 
-        if obj.test_dirs:
-            fh.write('TEST_DIRS := %s\n' % ' '.join(
-                relativize(backend_file.objdir, obj.test_dirs)))
-            if self.environment.substs.get('ENABLE_TESTS', False):
-                self._traversal.add(backend_file.relobjdir,
-                    dirs=relativize(self.environment.topobjdir, obj.test_dirs))
-
         # The directory needs to be registered whether subdirectories have been
         # registered or not.
         self._traversal.add(backend_file.relobjdir)
 
         affected_tiers = set(obj.affected_tiers)
 
         for tier in set(self._may_skip.keys()) - affected_tiers:
             self._may_skip[tier].add(backend_file.relobjdir)
--- a/python/mozbuild/mozbuild/frontend/context.py
+++ b/python/mozbuild/mozbuild/frontend/context.py
@@ -1236,25 +1236,16 @@ VARIABLES = {
         Each name in this variable corresponds to a hosst executable built
         from the corresponding source file with the same base name.
 
         If the configuration token ``HOST_BIN_SUFFIX`` is set, its value will
         be automatically appended to each name. If a name already ends with
         ``HOST_BIN_SUFFIX``, the name will remain unchanged.
         """, None),
 
-    'TEST_DIRS': (ContextDerivedTypedList(SourcePath), list,
-        """Like DIRS but only for directories that contain test-only code.
-
-        If tests are not enabled, this variable will be ignored.
-
-        This variable may go away once the transition away from Makefiles is
-        complete.
-        """, None),
-
     'CONFIGURE_SUBST_FILES': (ContextDerivedTypedList(SourcePath, StrictOrderingOnAppendList), list,
         """Output files that will be generated using configure-like substitution.
 
         This is a substitute for ``AC_OUTPUT`` in autoconf. For each path in this
         list, we will search for a file in the srcdir having the name
         ``{path}.in``. The contents of this file will be read and variable
         patterns like ``@foo@`` will be substituted with the values of the
         ``AC_SUBST`` variables declared during configure.
@@ -1872,16 +1863,20 @@ FUNCTIONS = {
 
            - ``FINAL_TARGET`` is 'dist/other'
            - ``USE_LIBS`` is ['mylib', 'mozglue', 'otherlib']
            - ``PROGRAM`` is 'myprog'
 
         """),
 }
 
+
+TestDirsPlaceHolder = List()
+
+
 # Special variables. These complement VARIABLES.
 #
 # Each entry is a tuple of:
 #
 #  (function returning the corresponding value from a given context, type, docs)
 #
 SPECIAL_VARIABLES = {
     'TOPSRCDIR': (lambda context: context.config.topsrcdir, str,
@@ -1995,16 +1990,25 @@ SPECIAL_VARIABLES = {
         with Firefox. This variable defines them.
 
         To install modules in a subdirectory, use properties of this
         variable to control the final destination. e.g.
 
         ``TESTING_JS_MODULES.foo += ['module.jsm']``.
         """),
 
+    'TEST_DIRS': (lambda context: context['DIRS'] if context.config.substs.get('ENABLE_TESTS')
+                                  else TestDirsPlaceHolder, list,
+        """Like DIRS but only for directories that contain test-only code.
+
+        If tests are not enabled, this variable will be ignored.
+
+        This variable may go away once the transition away from Makefiles is
+        complete.
+        """),
 }
 
 # Deprecation hints.
 DEPRECATION_HINTS = {
     'CPP_UNIT_TESTS': '''
         Please use'
 
             CppUnitTests(['foo', 'bar'])
--- a/python/mozbuild/mozbuild/frontend/data.py
+++ b/python/mozbuild/mozbuild/frontend/data.py
@@ -91,24 +91,22 @@ class DirectoryTraversal(ContextDerived)
     existing recursive make backend while the transition to mozbuild frontend
     files is complete and we move to a more optimal build backend.
 
     Fields in this class correspond to similarly named variables in the
     frontend files.
     """
     __slots__ = (
         'dirs',
-        'test_dirs',
     )
 
     def __init__(self, context):
         ContextDerived.__init__(self, context)
 
         self.dirs = []
-        self.test_dirs = []
 
 
 class BaseConfigSubstitution(ContextDerived):
     """Base class describing autogenerated files as part of config.status."""
 
     __slots__ = (
         'input_path',
         'output_path',
--- a/python/mozbuild/mozbuild/frontend/emitter.py
+++ b/python/mozbuild/mozbuild/frontend/emitter.py
@@ -1279,17 +1279,16 @@ class TreeMetadataEmitter(LoggingMixin):
             if 'jar.mn' not in jar_manifests:
                 raise SandboxValidationError('A jar.mn exists but it '
                     'is not referenced in the moz.build file. '
                     'Please define JAR_MANIFESTS.', context)
 
     def _emit_directory_traversal_from_context(self, context):
         o = DirectoryTraversal(context)
         o.dirs = context.get('DIRS', [])
-        o.test_dirs = context.get('TEST_DIRS', [])
         o.affected_tiers = context.get_affected_tiers()
 
         # Some paths have a subconfigure, yet also have a moz.build. Those
         # shouldn't end up in self._external_paths.
         if o.objdir:
             self._external_paths -= { o.relobjdir }
 
         yield o
--- a/python/mozbuild/mozbuild/frontend/reader.py
+++ b/python/mozbuild/mozbuild/frontend/reader.py
@@ -1122,23 +1122,18 @@ class BuildReader(object):
         self._execution_time += time.time() - time_start
         self._file_count += len(context.all_paths)
 
         # Yield main context before doing any processing. This gives immediate
         # consumers an opportunity to change state before our remaining
         # processing is performed.
         yield context
 
-        # We first collect directories populated in variables.
-        dir_vars = ['DIRS']
-
-        if context.config.substs.get('ENABLE_TESTS', False) == '1':
-            dir_vars.append('TEST_DIRS')
-
-        dirs = [(v, context[v]) for v in dir_vars if v in context]
+        # We need the list of directories pre-gyp processing for later.
+        dirs = list(context.get('DIRS', []))
 
         curdir = mozpath.dirname(path)
 
         gyp_contexts = []
         for target_dir in context.get('GYP_DIRS', []):
             gyp_dir = context['GYP_DIRS'][target_dir]
             for v in ('input', 'variables'):
                 if not getattr(gyp_dir, v):
@@ -1178,30 +1173,29 @@ class BuildReader(object):
             yield subcontext
 
         # Traverse into referenced files.
 
         # It's very tempting to use a set here. Unfortunately, the recursive
         # make backend needs order preserved. Once we autogenerate all backend
         # files, we should be able to convert this to a set.
         recurse_info = OrderedDict()
-        for var, var_dirs in dirs:
-            for d in var_dirs:
-                if d in recurse_info:
-                    raise SandboxValidationError(
-                        'Directory (%s) registered multiple times in %s' % (
-                            mozpath.relpath(d.full_path, context.srcdir), var),
-                        context)
+        for d in dirs:
+            if d in recurse_info:
+                raise SandboxValidationError(
+                    'Directory (%s) registered multiple times' % (
+                        mozpath.relpath(d.full_path, context.srcdir)),
+                    context)
 
-                recurse_info[d] = {}
-                for key in sandbox.metadata:
-                    if key == 'exports':
-                        sandbox.recompute_exports()
+            recurse_info[d] = {}
+            for key in sandbox.metadata:
+                if key == 'exports':
+                    sandbox.recompute_exports()
 
-                    recurse_info[d][key] = dict(sandbox.metadata[key])
+                recurse_info[d][key] = dict(sandbox.metadata[key])
 
         for path, child_metadata in recurse_info.items():
             child_path = path.join('moz.build').full_path
 
             # Ensure we don't break out of the topsrcdir. We don't do realpath
             # because it isn't necessary. If there are symlinks in the srcdir,
             # that's not our problem. We're not a hosted application: we don't
             # need to worry about security too much.
@@ -1313,17 +1307,17 @@ class BuildReader(object):
 
         contexts = defaultdict(list)
         all_contexts = []
         for context in self.read_mozbuild(mozpath.join(topsrcdir, 'moz.build'),
                                           self.config, metadata=metadata):
             # Explicitly set directory traversal variables to override default
             # traversal rules.
             if not isinstance(context, SubContext):
-                for v in ('DIRS', 'GYP_DIRS', 'TEST_DIRS'):
+                for v in ('DIRS', 'GYP_DIRS'):
                     context[v][:] = []
 
                 context['DIRS'] = sorted(dirs[context.main_path])
 
             contexts[context.main_path].append(context)
             all_contexts.append(context)
 
         result = {}
--- a/python/mozbuild/mozbuild/test/backend/test_recursivemake.py
+++ b/python/mozbuild/mozbuild/test/backend/test_recursivemake.py
@@ -215,17 +215,27 @@ class TestRecursiveMakeBackend(BackendTe
         """Ensure backend.mk file is written out properly."""
         env = self._consume('stub0', RecursiveMakeBackend)
 
         p = mozpath.join(env.topobjdir, 'backend.mk')
 
         lines = [l.strip() for l in open(p, 'rt').readlines()[2:]]
         self.assertEqual(lines, [
             'DIRS := dir1 dir2',
-            'TEST_DIRS := dir3',
+        ])
+
+        # Make env.substs writable to add ENABLE_TESTS
+        env.substs = dict(env.substs)
+        env.substs['ENABLE_TESTS'] = '1'
+        self._consume('stub0', RecursiveMakeBackend, env=env)
+        p = mozpath.join(env.topobjdir, 'backend.mk')
+
+        lines = [l.strip() for l in open(p, 'rt').readlines()[2:]]
+        self.assertEqual(lines, [
+            'DIRS := dir1 dir2 dir3',
         ])
 
     def test_mtime_no_change(self):
         """Ensure mtime is not updated if file content does not change."""
 
         env = self._consume('stub0', RecursiveMakeBackend)
 
         makefile_path = mozpath.join(env.topobjdir, 'Makefile')
--- a/python/mozbuild/mozbuild/test/frontend/data/traversal-all-vars/moz.build
+++ b/python/mozbuild/mozbuild/test/frontend/data/traversal-all-vars/moz.build
@@ -1,6 +1,6 @@
 # -*- Mode: python; c-basic-offset: 4; indent-tabs-mode: nil; tab-width: 40 -*-
 # Any copyright is dedicated to the Public Domain.
 # http://creativecommons.org/publicdomain/zero/1.0/
 
 DIRS += ['regular']
-TEST_DIRS = ['test']
+TEST_DIRS += ['test']
--- a/python/mozbuild/mozbuild/test/frontend/test_emitter.py
+++ b/python/mozbuild/mozbuild/test/frontend/test_emitter.py
@@ -59,19 +59,19 @@ class TestEmitterBasic(unittest.TestCase
     def setUp(self):
         self._old_env = dict(os.environ)
         os.environ.pop('MOZ_OBJDIR', None)
 
     def tearDown(self):
         os.environ.clear()
         os.environ.update(self._old_env)
 
-    def reader(self, name):
+    def reader(self, name, enable_tests=False):
         config = MockConfig(mozpath.join(data_path, name), extra_substs=dict(
-            ENABLE_TESTS='1',
+            ENABLE_TESTS='1' if enable_tests else '',
             BIN_SUFFIX='.prog',
             OS_TARGET='WINNT',
         ))
 
         return BuildReader(config)
 
     def read_topsrcdir(self, reader, filter_common=True):
         emitter = TreeMetadataEmitter(reader.config)
@@ -89,17 +89,16 @@ class TestEmitterBasic(unittest.TestCase
 
     def test_dirs_traversal_simple(self):
         reader = self.reader('traversal-simple')
         objs = self.read_topsrcdir(reader, filter_common=False)
         self.assertEqual(len(objs), 4)
 
         for o in objs:
             self.assertIsInstance(o, DirectoryTraversal)
-            self.assertEqual(o.test_dirs, [])
             self.assertTrue(os.path.isabs(o.context_main_path))
             self.assertEqual(len(o.context_all_paths), 1)
 
         reldirs = [o.relativedir for o in objs]
         self.assertEqual(reldirs, ['', 'foo', 'foo/biz', 'bar'])
 
         self.assertEqual(objs[3].affected_tiers, {'misc'})
 
@@ -110,31 +109,48 @@ class TestEmitterBasic(unittest.TestCase
                 mozpath.join(reader.config.topsrcdir, 'bar')
             ], [
                 mozpath.join(reader.config.topsrcdir, 'foo', 'biz')
             ], [], []])
 
     def test_traversal_all_vars(self):
         reader = self.reader('traversal-all-vars')
         objs = self.read_topsrcdir(reader, filter_common=False)
+        self.assertEqual(len(objs), 2)
+
+        for o in objs:
+            self.assertIsInstance(o, DirectoryTraversal)
+
+        reldirs = set([o.relativedir for o in objs])
+        self.assertEqual(reldirs, set(['', 'regular']))
+
+        for o in objs:
+            reldir = o.relativedir
+
+            if reldir == '':
+                self.assertEqual([d.full_path for d in o.dirs], [
+                    mozpath.join(reader.config.topsrcdir, 'regular')])
+
+    def test_traversal_all_vars_enable_tests(self):
+        reader = self.reader('traversal-all-vars', enable_tests=True)
+        objs = self.read_topsrcdir(reader, filter_common=False)
         self.assertEqual(len(objs), 3)
 
         for o in objs:
             self.assertIsInstance(o, DirectoryTraversal)
 
         reldirs = set([o.relativedir for o in objs])
         self.assertEqual(reldirs, set(['', 'regular', 'test']))
 
         for o in objs:
             reldir = o.relativedir
 
             if reldir == '':
                 self.assertEqual([d.full_path for d in o.dirs], [
-                    mozpath.join(reader.config.topsrcdir, 'regular')])
-                self.assertEqual([d.full_path for d in o.test_dirs], [
+                    mozpath.join(reader.config.topsrcdir, 'regular'),
                     mozpath.join(reader.config.topsrcdir, 'test')])
 
     def test_config_file_substitution(self):
         reader = self.reader('config-file-substitution')
         objs = self.read_topsrcdir(reader)
         self.assertEqual(len(objs), 2)
 
         self.assertIsInstance(objs[0], ConfigFileSubstitution)
--- a/python/mozbuild/mozbuild/test/frontend/test_reader.py
+++ b/python/mozbuild/mozbuild/test/frontend/test_reader.py
@@ -67,16 +67,21 @@ class TestBuildReader(unittest.TestCase)
         self.assertTrue(os.path.exists(path))
 
         contexts = list(reader.read_mozbuild(path, reader.config,
             descend=False))
 
         self.assertEqual(len(contexts), 1)
 
     def test_dirs_traversal_all_variables(self):
+        reader = self.reader('traversal-all-vars')
+
+        contexts = list(reader.read_topsrcdir())
+        self.assertEqual(len(contexts), 2)
+
         reader = self.reader('traversal-all-vars', enable_tests=True)
 
         contexts = list(reader.read_topsrcdir())
         self.assertEqual(len(contexts), 3)
 
     def test_relative_dirs(self):
         # Ensure relative directories are traversed.
         reader = self.reader('traversal-relative-dirs')
@@ -217,18 +222,17 @@ class TestBuildReader(unittest.TestCase)
 
     def test_error_repeated_dir(self):
         reader = self.reader('reader-error-repeated-dir')
 
         with self.assertRaises(BuildReaderError) as bre:
             list(reader.read_topsrcdir())
 
         e = bre.exception
-        self.assertIn('Directory (foo) registered multiple times in DIRS',
-            str(e))
+        self.assertIn('Directory (foo) registered multiple times', str(e))
 
     def test_error_error_func(self):
         reader = self.reader('reader-error-error-func')
 
         with self.assertRaises(BuildReaderError) as bre:
             list(reader.read_topsrcdir())
 
         e = bre.exception
--- a/security/manager/ssl/tests/unit/moz.build
+++ b/security/manager/ssl/tests/unit/moz.build
@@ -1,15 +1,19 @@
 # -*- Mode: python; c-basic-offset: 4; indent-tabs-mode: nil; tab-width: 40 -*-
 # vim: set filetype=python:
 # 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/.
 
 DIRS += ['tlsserver']
+
+if not CONFIG['MOZ_NO_SMART_CARDS']:
+    DIRS += ['pkcs11testmodule']
+
 TEST_DIRS += [
     'bad_certs',
     'ocsp_certs',
     'test_cert_eku',
     'test_cert_embedded_null',
     'test_cert_keyUsage',
     'test_cert_sha1',
     'test_cert_signatures',
@@ -22,11 +26,8 @@ TEST_DIRS += [
     'test_keysize_ev',
     'test_name_constraints',
     'test_ocsp_fetch_method',
     'test_ocsp_url',
     'test_onecrl',
     'test_pinning_dynamic',
     'test_validity',
 ]
-
-if not CONFIG['MOZ_NO_SMART_CARDS']:
-    DIRS += ['pkcs11testmodule']