Bug 1254374 - Add various failure tests to test_configure.py. r=nalexander

At the same time, improve some of the failures handling paths.

python/mozbuild/mozbuild/configure/__init__.py

@@ -41,8 +41,8 @@ class ConfigureError(Exception):
 class DependsFunction(object):
     '''Sandbox-visible representation of @depends functions.'''
     def __call__(self, *arg, **kwargs):
-        raise RuntimeError('The `%s` function may not be called'
-                           % self.__name__)
+        raise ConfigureError('The `%s` function may not be called'
+                             % self.__name__)
 
 
 class SandboxedGlobal(dict):
@@ -107,6 +107,7 @@ class ConfigureSandbox(dict):
         dict.__setitem__(self, '__builtins__', self.BUILTINS)
 
         self._paths = []
+        self._all_paths = set()
         self._templates = set()
         # Store the real function and its dependencies, behind each
         # DependsFunction generated from @depends.
@@ -179,16 +180,18 @@ class ConfigureSandbox(dict):
 
         if self._paths:
             path = mozpath.join(mozpath.dirname(self._paths[-1]), path)
+            path = mozpath.normpath(path)
             if not mozpath.basedir(path, (mozpath.dirname(self._paths[0]),)):
                 raise ConfigureError(
                     'Cannot include `%s` because it is not in a subdirectory '
                     'of `%s`' % (path, mozpath.dirname(self._paths[0])))
         else:
             path = mozpath.realpath(mozpath.abspath(path))
-        if path in self._paths:
+        if path in self._all_paths:
             raise ConfigureError(
                 'Cannot include `%s` because it was included already.' % path)
         self._paths.append(path)
+        self._all_paths.add(path)
 
         source = open(path, 'rb').read()
 
@@ -209,7 +212,7 @@ class ConfigureSandbox(dict):
             if arg in self._implied_options:
                 frameinfo, reason = self._implied_options[arg]
                 raise ConfigureError(
-                    '`%s`, emitted from `%s` line `%d`, was not handled.'
+                    '`%s`, emitted from `%s` line %d, is unknown.'
                     % (without_value, frameinfo[1], frameinfo[2]))
             raise InvalidOptionError('Unknown option: %s' % without_value)
 
@@ -275,11 +278,9 @@ class ConfigureSandbox(dict):
         kwargs = {k: self._resolve(v) for k, v in kwargs.iteritems()}
         option = Option(*args, **kwargs)
         if option.name in self._options:
-            raise ConfigureError('Option `%s` already defined'
-                                 % self._options[option.name].option)
+            raise ConfigureError('Option `%s` already defined' % option.option)
         if option.env in self._options:
-            raise ConfigureError('Option `%s` already defined'
-                                 % self._options[option.env].option)
+            raise ConfigureError('Option `%s` already defined' % option.env)
         if option.name:
             self._options[option.name] = option
         if option.env:
@@ -346,7 +347,7 @@ class ConfigureSandbox(dict):
             else:
                 raise TypeError(
                     "Cannot use object of type '%s' as argument to @depends"
-                    % type(arg))
+                    % type(arg).__name__)
             resolved_args.append(resolved_arg)
         dependencies = tuple(dependencies)
 
@@ -385,7 +386,7 @@ class ConfigureSandbox(dict):
         what = self._resolve(what)
         if what:
             if not isinstance(what, types.StringTypes):
-                raise TypeError("Unexpected type: '%s'" % type(what))
+                raise TypeError("Unexpected type: '%s'" % type(what).__name__)
             self.exec_file(what)
 
     def template_impl(self, func):
@@ -449,16 +450,20 @@ class ConfigureSandbox(dict):
         '''
         for value, required in (
                 (_import, True), (_from, False), (_as, False)):
-            if not isinstance(value, types.StringTypes) and not (
-                    required or value is None):
-                raise TypeError("Unexpected type: '%s'" % type(value))
+
+            if not isinstance(value, types.StringTypes) and (
+                    required or value is not None):
+                raise TypeError("Unexpected type: '%s'" % type(value).__name__)
             if value is not None and not self.RE_MODULE.match(value):
                 raise ValueError("Invalid argument to @imports: '%s'" % value)
 
         def decorator(func):
-            if func in self._prepared_functions:
+            if func in self._templates:
                 raise ConfigureError(
-                    '@imports must appear after other decorators')
+                    '@imports must appear after @template')
+            if func in self._depends:
+                raise ConfigureError(
+                    '@imports must appear after @depends')
             # For the imports to apply in the order they appear in the
             # .configure file, we accumulate them in reverse order and apply
             # them later.
@@ -503,7 +508,7 @@ class ConfigureSandbox(dict):
         if name is None:
             return
         if not isinstance(name, types.StringTypes):
-            raise TypeError("Unexpected type: '%s'" % type(name))
+            raise TypeError("Unexpected type: '%s'" % type(name).__name__)
         if name in data:
             raise ConfigureError(
                 "Cannot add '%s' to configuration: Key already "
@@ -587,7 +592,7 @@ class ConfigureSandbox(dict):
                     reason = (self._raw_options.get(possible_reasons[0]) or
                               possible_reasons[0].option)
 
-        if not reason or not isinstance(value, DependsFunction):
+        if not reason:
             raise ConfigureError(
                 "Cannot infer what implies '%s'. Please add a `reason` to "
                 "the `imply_option` call."
@@ -606,7 +611,7 @@ class ConfigureSandbox(dict):
             elif isinstance(value, tuple):
                 value = PositiveOptionValue(value)
             else:
-                raise TypeError("Unexpected type: '%s'" % type(value))
+                raise TypeError("Unexpected type: '%s'" % type(value).__name__)
 
             option = value.format(option)
             self._helper.add(option, 'implied')
@@ -617,7 +622,7 @@ class ConfigureSandbox(dict):
         for @depends, and @template.
         '''
         if not inspect.isfunction(func):
-            raise TypeError("Unexpected type: '%s'" % type(func))
+            raise TypeError("Unexpected type: '%s'" % type(func).__name__)
         if func in self._prepared_functions:
             return func, func.func_globals
 

python/mozbuild/mozbuild/test/configure/test_configure.py

@@ -10,7 +10,10 @@ import sys
 import textwrap
 import unittest
 
-from mozunit import main
+from mozunit import (
+    main,
+    MockedOpen,
+)
 
 from mozbuild.configure.options import (
     InvalidOptionError,
@@ -41,6 +44,12 @@ class TestConfigure(unittest.TestCase):
             self.assertEquals('', out.getvalue())
         return config
 
+    def moz_configure(self, source):
+        return MockedOpen({
+            os.path.join(test_data_path,
+                         'moz.configure'): textwrap.dedent(source)
+        })
+
     def test_defaults(self):
         config = self.get_config()
         self.maxDiff = None
@@ -545,6 +554,331 @@ class TestConfigure(unittest.TestCase):
             "Cannot infer what implies '--enable-bar'. Please add a `reason` "
             "to the `imply_option` call.")
 
+    def test_imply_option_failures(self):
+        with self.assertRaises(ConfigureError) as e:
+            with self.moz_configure('''
+                imply_option('--with-foo', ('a',), 'bar')
+            '''):
+                self.get_config()
+
+        self.assertEquals(e.exception.message,
+                          "`--with-foo`, emitted from `%s` line 2, is unknown."
+                          % mozpath.join(test_data_path, 'moz.configure'))
+
+        with self.assertRaises(TypeError) as e:
+            with self.moz_configure('''
+                imply_option('--with-foo', 42, 'bar')
+
+                option('--with-foo', help='foo')
+                @depends('--with-foo')
+                def foo(value):
+                    return value
+            '''):
+                self.get_config()
+
+        self.assertEquals(e.exception.message,
+                          "Unexpected type: 'int'")
+
+    def test_option_failures(self):
+        with self.assertRaises(ConfigureError) as e:
+            with self.moz_configure('option("--with-foo", help="foo")'):
+                self.get_config()
+
+        self.assertEquals(
+            e.exception.message,
+            'Option `--with-foo` is not handled ; reference it with a @depends'
+        )
+
+        with self.assertRaises(ConfigureError) as e:
+            with self.moz_configure('''
+                option("--with-foo", help="foo")
+                option("--with-foo", help="foo")
+            '''):
+                self.get_config()
+
+        self.assertEquals(
+            e.exception.message,
+            'Option `--with-foo` already defined'
+        )
+
+        with self.assertRaises(ConfigureError) as e:
+            with self.moz_configure('''
+                option(env="MOZ_FOO", help="foo")
+                option(env="MOZ_FOO", help="foo")
+            '''):
+                self.get_config()
+
+        self.assertEquals(
+            e.exception.message,
+            'Option `MOZ_FOO` already defined'
+        )
+
+        with self.assertRaises(ConfigureError) as e:
+            with self.moz_configure('''
+                option('--with-foo', env="MOZ_FOO", help="foo")
+                option(env="MOZ_FOO", help="foo")
+            '''):
+                self.get_config()
+
+        self.assertEquals(
+            e.exception.message,
+            'Option `MOZ_FOO` already defined'
+        )
+
+        with self.assertRaises(ConfigureError) as e:
+            with self.moz_configure('''
+                option(env="MOZ_FOO", help="foo")
+                option('--with-foo', env="MOZ_FOO", help="foo")
+            '''):
+                self.get_config()
+
+        self.assertEquals(
+            e.exception.message,
+            'Option `MOZ_FOO` already defined'
+        )
+
+        with self.assertRaises(ConfigureError) as e:
+            with self.moz_configure('''
+                option('--with-foo', env="MOZ_FOO", help="foo")
+                option('--with-foo', help="foo")
+            '''):
+                self.get_config()
+
+        self.assertEquals(
+            e.exception.message,
+            'Option `--with-foo` already defined'
+        )
+
+    def test_include_failures(self):
+        with self.assertRaises(ConfigureError) as e:
+            with self.moz_configure('include("../foo.configure")'):
+                self.get_config()
+
+        self.assertEquals(
+            e.exception.message,
+            'Cannot include `%s` because it is not in a subdirectory of `%s`'
+            % (mozpath.normpath(mozpath.join(test_data_path, '..',
+                                             'foo.configure')),
+               mozpath.normsep(test_data_path))
+        )
+
+        with self.assertRaises(ConfigureError) as e:
+            with self.moz_configure('''
+                include('extra.configure')
+                include('extra.configure')
+            '''):
+                self.get_config()
+
+        self.assertEquals(
+            e.exception.message,
+            'Cannot include `%s` because it was included already.'
+            % mozpath.normpath(mozpath.join(test_data_path,
+                                            'extra.configure'))
+        )
+
+        with self.assertRaises(TypeError) as e:
+            with self.moz_configure('''
+                include(42)
+            '''):
+                self.get_config()
+
+        self.assertEquals(e.exception.message, "Unexpected type: 'int'")
+
+    def test_sandbox_failures(self):
+        with self.assertRaises(KeyError) as e:
+            with self.moz_configure('''
+                include = 42
+            '''):
+                self.get_config()
+
+        self.assertEquals(e.exception.message, 'Cannot reassign builtins')
+
+        with self.assertRaises(KeyError) as e:
+            with self.moz_configure('''
+                foo = 42
+            '''):
+                self.get_config()
+
+        self.assertEquals(e.exception.message,
+                          'Cannot assign `foo` because it is neither a '
+                          '@depends nor a @template')
+
+    def test_depends_failures(self):
+        with self.assertRaises(ConfigureError) as e:
+            with self.moz_configure('''
+                @depends()
+                def foo():
+                    return
+            '''):
+                self.get_config()
+
+        self.assertEquals(e.exception.message,
+                          "@depends needs at least one argument")
+
+        with self.assertRaises(ConfigureError) as e:
+            with self.moz_configure('''
+                @depends('--with-foo')
+                def foo(value):
+                    return value
+            '''):
+                self.get_config()
+
+        self.assertEquals(e.exception.message,
+                          "'--with-foo' is not a known option. Maybe it's "
+                          "declared too late?")
+
+        with self.assertRaises(ConfigureError) as e:
+            with self.moz_configure('''
+                @depends('--with-foo=42')
+                def foo(value):
+                    return value
+            '''):
+                self.get_config()
+
+        self.assertEquals(e.exception.message,
+                          "Option must not contain an '='")
+
+        with self.assertRaises(TypeError) as e:
+            with self.moz_configure('''
+                @depends(42)
+                def foo(value):
+                    return value
+            '''):
+                self.get_config()
+
+        self.assertEquals(e.exception.message,
+                          "Cannot use object of type 'int' as argument "
+                          "to @depends")
+
+        with self.assertRaises(ConfigureError) as e:
+            with self.moz_configure('''
+                @depends('--help')
+                def foo(value):
+                    yield
+            '''):
+                self.get_config()
+
+        self.assertEquals(e.exception.message,
+                          "Cannot decorate generator functions with @depends")
+
+        with self.assertRaises(ConfigureError) as e:
+            with self.moz_configure('''
+                option('--foo', help='foo')
+                @depends('--foo')
+                def foo(value):
+                    return value
+
+                @depends('--help', foo)
+                def bar(help, foo):
+                    return
+            '''):
+                self.get_config()
+
+        self.assertEquals(e.exception.message,
+                          "`bar` depends on '--help' and `foo`. "
+                          "`foo` must depend on '--help'")
+
+        with self.assertRaises(TypeError) as e:
+            with self.moz_configure('''
+                depends('--help')(42)
+            '''):
+                self.get_config()
+
+        self.assertEquals(e.exception.message,
+                          "Unexpected type: 'int'")
+
+        with self.assertRaises(ConfigureError) as e:
+            with self.moz_configure('''
+                option('--foo', help='foo')
+                @depends('--foo')
+                def foo(value):
+                    return value
+
+                include(foo)
+            '''):
+                self.get_config()
+
+        self.assertEquals(e.exception.message,
+                          "Missing @depends for `foo`: '--help'")
+
+        with self.assertRaises(ConfigureError) as e:
+            with self.moz_configure('''
+                option('--foo', help='foo')
+                @depends('--foo')
+                def foo(value):
+                    return value
+
+                foo()
+            '''):
+                self.get_config()
+
+        self.assertEquals(e.exception.message,
+                          "The `foo` function may not be called")
+
+    def test_imports_failures(self):
+        with self.assertRaises(ConfigureError) as e:
+            with self.moz_configure('''
+                @imports('os')
+                @template
+                def foo(value):
+                    return value
+            '''):
+                self.get_config()
+
+        self.assertEquals(e.exception.message,
+                          '@imports must appear after @template')
+
+        with self.assertRaises(ConfigureError) as e:
+            with self.moz_configure('''
+                option('--foo', help='foo')
+                @imports('os')
+                @depends('--foo')
+                def foo(value):
+                    return value
+            '''):
+                self.get_config()
+
+        self.assertEquals(e.exception.message,
+                          '@imports must appear after @depends')
+
+        for import_ in (
+            "42",
+            "_from=42, _import='os'",
+            "_from='os', _import='path', _as=42",
+        ):
+            with self.assertRaises(TypeError) as e:
+                with self.moz_configure('''
+                    @imports(%s)
+                    @template
+                    def foo(value):
+                        return value
+                ''' % import_):
+                    self.get_config()
+
+            self.assertEquals(e.exception.message, "Unexpected type: 'int'")
+
+        with self.assertRaises(TypeError) as e:
+            with self.moz_configure('''
+                @imports('os', 42)
+                @template
+                def foo(value):
+                    return value
+            '''):
+                self.get_config()
+
+        self.assertEquals(e.exception.message, "Unexpected type: 'int'")
+
+        with self.assertRaises(ValueError) as e:
+            with self.moz_configure('''
+                @imports('os*')
+                def foo(value):
+                    return value
+            '''):
+                self.get_config()
+
+        self.assertEquals(e.exception.message,
+                          "Invalid argument to @imports: 'os*'")
+
 
 if __name__ == '__main__':
     main()