interpreter: use typed_*args for test and benchmark

this also requires some changes to the Rust module, as it calls into the
test code.

mesonbuild/interpreter/__init__.py

@@ -16,7 +16,7 @@
 
 """Meson interpreter."""
 
-from .interpreter import Interpreter, permitted_test_kwargs, permitted_dependency_kwargs
+from .interpreter import Interpreter, permitted_dependency_kwargs
 from .compiler import CompilerHolder
 from .interpreterobjects import (ExecutableHolder, BuildTargetHolder, CustomTargetHolder,
                                  CustomTargetIndexHolder, MachineHolder, Test,

mesonbuild/interpreter/interpreter.py

@@ -188,17 +188,24 @@ known_build_target_kwargs = (
     {'target_type'}
 )
 
-permitted_test_kwargs = {
-    'args',
-    'depends',
-    'env',
-    'priority',
-    'protocol',
-    'should_fail',
-    'suite',
-    'timeout',
-    'workdir',
-}
+TEST_KWARGS: T.List[KwargInfo] = [
+    KwargInfo('args', ContainerTypeInfo(list, (str, mesonlib.File, TargetHolder)),
+              listify=True, default=[]),
+    KwargInfo('should_fail', bool, default=False),
+    KwargInfo('timeout', int, default=30),
+    KwargInfo('workdir', str, default=None,
+              validator=lambda x: 'must be an absolute path' if not os.path.isabs(x) else None),
+    KwargInfo('protocol', str,
+              default='exitcode',
+              validator=lambda x: 'value must be one of "exitcode", "tap", "gtest", "rust"' if x not in {'exitcode', 'tap', 'gtest', 'rust'} else None,
+              since_values={'gtest': '0.55.0', 'rust': '0.57.0'}),
+    KwargInfo('depends', ContainerTypeInfo(list, (CustomTargetHolder, BuildTargetHolder)),
+              listify=True, default=[], since='0.46.0'),
+    KwargInfo('priority', int, default=0, since='0.52.0'),
+    # TODO: env needs reworks of the way the environment variable holder itself works probably
+    KwargInfo('env', (EnvironmentVariablesHolder, list, dict, str)),
+    KwargInfo('suite', ContainerTypeInfo(list, str), listify=True, default=['']),  # yes, a list of empty string
+]
 
 permitted_dependency_kwargs = {
     'allow_fallback',
@@ -1952,30 +1959,21 @@ This will become a hard error in the future.''' % kwargs['input'], location=self
         self.generators.append(gen)
         return gen
 
-    @FeatureNewKwargs('benchmark', '0.46.0', ['depends'])
-    @FeatureNewKwargs('benchmark', '0.52.0', ['priority'])
-    @permittedKwargs(permitted_test_kwargs)
     @typed_pos_args('benchmark', str, (ExecutableHolder, JarHolder, ExternalProgramHolder, mesonlib.File))
-    def func_benchmark(self, node: mparser.FunctionNode,
+    @typed_kwargs('benchmark', *TEST_KWARGS)
+    def func_benchmark(self, node: mparser.BaseNode,
                        args: T.Tuple[str, T.Union[ExecutableHolder, JarHolder, ExternalProgramHolder, mesonlib.File]],
-                       kwargs) -> None:
-        # is_parallel isn't valid here, so make sure it isn't passed
-        if 'is_parallel' in kwargs:
-            del kwargs['is_parallel']
+                       kwargs: 'kwargs.FuncBenchmark') -> None:
         self.add_test(node, args, kwargs, False)
 
-    @FeatureNewKwargs('test', '0.46.0', ['depends'])
-    @FeatureNewKwargs('test', '0.52.0', ['priority'])
-    @permittedKwargs(permitted_test_kwargs | {'is_parallel'})
     @typed_pos_args('test', str, (ExecutableHolder, JarHolder, ExternalProgramHolder, mesonlib.File))
-    def func_test(self, node: mparser.FunctionNode,
+    @typed_kwargs('benchmark', *TEST_KWARGS, KwargInfo('is_parallel', bool, default=True))
+    def func_test(self, node: mparser.BaseNode,
                   args: T.Tuple[str, T.Union[ExecutableHolder, JarHolder, ExternalProgramHolder, mesonlib.File]],
-                  kwargs) -> None:
-        if kwargs.get('protocol') == 'gtest':
-            FeatureNew.single_use('"gtest" protocol for tests', '0.55.0', self.subproject)
+                  kwargs: 'kwargs.FuncTest') -> None:
         self.add_test(node, args, kwargs, True)
 
-    def unpack_env_kwarg(self, kwargs) -> build.EnvironmentVariables:
+    def unpack_env_kwarg(self, kwargs: T.Union[EnvironmentVariablesHolder, T.Dict[str, str], T.List[str]]) -> build.EnvironmentVariables:
         envlist = kwargs.get('env', EnvironmentVariablesHolder())
         if isinstance(envlist, EnvironmentVariablesHolder):
             env = envlist.held_object
@@ -1989,9 +1987,9 @@ This will become a hard error in the future.''' % kwargs['input'], location=self
             env = env.held_object
         return env
 
-    def make_test(self, node: mparser.FunctionNode,
+    def make_test(self, node: mparser.BaseNode,
                   args: T.Tuple[str, T.Union[ExecutableHolder, JarHolder, ExternalProgramHolder, mesonlib.File]],
-                  kwargs: T.Dict[str, T.Any]) -> Test:
+                  kwargs: 'kwargs.BaseTest') -> Test:
         name = args[0]
         if ':' in name:
             mlog.deprecation(f'":" is not allowed in test name "{name}", it has been replaced with "_"',
@@ -2001,48 +1999,32 @@ This will become a hard error in the future.''' % kwargs['input'], location=self
         if isinstance(exe, mesonlib.File):
             exe = self.func_find_program(node, args[1], {})
 
-        par = kwargs.get('is_parallel', True)
-        if not isinstance(par, bool):
-            raise InterpreterException('Keyword argument is_parallel must be a boolean.')
-        cmd_args = unholder(extract_as_list(kwargs, 'args'))
-        for i in cmd_args:
-            if not isinstance(i, (str, mesonlib.File, build.Target)):
-                raise InterpreterException('Command line arguments must be strings, files or targets.')
         env = self.unpack_env_kwarg(kwargs)
-        should_fail = kwargs.get('should_fail', False)
-        if not isinstance(should_fail, bool):
-            raise InterpreterException('Keyword argument should_fail must be a boolean.')
-        timeout = kwargs.get('timeout', 30)
-        if not isinstance(timeout, int):
-            raise InterpreterException('Timeout must be an integer.')
-        if timeout <= 0:
-            FeatureNew('test() timeout <= 0', '0.57.0').use(self.subproject)
-        if 'workdir' in kwargs:
-            workdir = kwargs['workdir']
-            if not isinstance(workdir, str):
-                raise InterpreterException('Workdir keyword argument must be a string.')
-            if not os.path.isabs(workdir):
-                raise InterpreterException('Workdir keyword argument must be an absolute path.')
-        else:
-            workdir = None
-        protocol = kwargs.get('protocol', 'exitcode')
-        if protocol not in {'exitcode', 'tap', 'gtest', 'rust'}:
-            raise InterpreterException('Protocol must be one of "exitcode", "tap", "gtest", or "rust".')
-        suite = []
+
+        if kwargs['timeout'] <= 0:
+            FeatureNew.single_use('test() timeout <= 0', '0.57.0', self.subproject)
+
         prj = self.subproject if self.is_subproject() else self.build.project_name
-        for s in mesonlib.stringlistify(kwargs.get('suite', '')):
-            if len(s) > 0:
+
+        suite: T.List[str] = []
+        for s in kwargs['suite']:
+            if s:
                 s = ':' + s
             suite.append(prj.replace(' ', '_').replace(':', '_') + s)
-        depends = unholder(extract_as_list(kwargs, 'depends'))
-        for dep in depends:
-            if not isinstance(dep, (build.CustomTarget, build.BuildTarget)):
-                raise InterpreterException('Depends items must be build targets.')
-        priority = kwargs.get('priority', 0)
-        if not isinstance(priority, int):
-            raise InterpreterException('Keyword argument priority must be an integer.')
-        return Test(name, prj, suite, exe.held_object, depends, par, cmd_args,
-                    env, should_fail, timeout, workdir, protocol, priority)
+
+        return Test(name,
+                    prj,
+                    suite,
+                    exe.held_object,
+                    [d.held_object for d in kwargs['depends']],
+                    kwargs.get('is_parallel', False),
+                    [c.held_object if isinstance(c, ObjectHolder) else c for c in kwargs['args']],
+                    env,
+                    kwargs['should_fail'],
+                    kwargs['timeout'],
+                    kwargs['workdir'],
+                    kwargs['protocol'],
+                    kwargs['priority'])
 
     def add_test(self, node: mparser.BaseNode, args: T.List, kwargs: T.Dict[str, T.Any], is_base_test: bool):
         t = self.make_test(node, args, kwargs)

mesonbuild/interpreter/kwargs.py

@@ -6,9 +6,10 @@
 
 import typing as T
 
-from typing_extensions import TypedDict
+from typing_extensions import TypedDict, Literal
 
-from ..mesonlib import MachineChoice
+from ..mesonlib import MachineChoice, File
+from .interpreterobjects import BuildTargetHolder, CustomTargetHolder, EnvironmentVariablesHolder, TargetHolder
 
 
 class FuncAddProjectArgs(TypedDict):
@@ -24,3 +25,35 @@ class FuncAddProjectArgs(TypedDict):
 
     native: MachineChoice
     language: T.List[str]
+
+
+class BaseTest(TypedDict):
+
+    """Shared base for the Rust module."""
+
+    args: T.List[T.Union[str, File, TargetHolder]]
+    should_fail: bool
+    timeout: int
+    workdir: T.Optional[str]
+    depends: T.List[T.Union[CustomTargetHolder, BuildTargetHolder]]
+    priority: int
+    env: T.Union[EnvironmentVariablesHolder, T.List[str], T.Dict[str, str], str]
+    suite: T.List[str]
+
+
+class FuncBenchmark(BaseTest):
+
+    """Keyword Arguments shared between `test` and `benchmark`."""
+
+    protocol: Literal['exitcode', 'tap', 'gtest', 'rust']
+
+
+class FuncTest(FuncBenchmark):
+
+    """Keyword Arguments for `test`
+
+    `test` only adds the `is_prallel` argument over benchmark, so inherintance
+    is helpful here.
+    """
+
+    is_parallel: bool

mesonbuild/modules/unstable_rust.py

@@ -18,16 +18,28 @@ import typing as T
 from . import ExtensionModule, ModuleReturnValue
 from .. import mlog
 from ..build import BuildTarget, CustomTargetIndex, Executable, GeneratedList, InvalidArguments, IncludeDirs, CustomTarget
-from ..dependencies import Dependency, ExternalLibrary
-from ..interpreter import ExecutableHolder, BuildTargetHolder, CustomTargetHolder, permitted_test_kwargs
-from ..interpreterbase import InterpreterException, permittedKwargs, FeatureNew, typed_pos_args, noPosargs
+from ..interpreter.interpreter import TEST_KWARGS
+from ..interpreter.interpreterobjects import (
+    BuildTargetHolder,
+    CustomTargetHolder,
+    DependencyHolder,
+    ExecutableHolder,
+    ExternalLibraryHolder,
+)
+from ..interpreterbase import ContainerTypeInfo, InterpreterException, KwargInfo, permittedKwargs, FeatureNew, typed_kwargs, typed_pos_args, noPosargs
 from ..mesonlib import stringlistify, unholder, listify, typeslistify, File
 
 if T.TYPE_CHECKING:
     from . import ModuleState
     from ..interpreter import Interpreter
-    from ..programs import ExternalProgram
+    from ..interpreter import kwargs as _kwargs
     from ..interpreter.interpreter import SourceOutputs
+    from ..programs import ExternalProgram
+
+    class FuncTest(_kwargs.BaseTest):
+
+        dependencies: T.List[T.Union[DependencyHolder, ExternalLibraryHolder]]
+        is_parallel: bool
 
 
 class RustModule(ExtensionModule):
@@ -43,9 +55,18 @@ class RustModule(ExtensionModule):
             'bindgen': self.bindgen,
         })
 
-    @permittedKwargs(permitted_test_kwargs | {'dependencies'} ^ {'protocol'})
     @typed_pos_args('rust.test', str, BuildTargetHolder)
-    def test(self, state: 'ModuleState', args: T.Tuple[str, BuildTargetHolder], kwargs: T.Dict[str, T.Any]) -> ModuleReturnValue:
+    @typed_kwargs(
+        'rust.test',
+        *TEST_KWARGS,
+        KwargInfo('is_parallel', bool, default=False),
+        KwargInfo(
+            'dependencies',
+            ContainerTypeInfo(list, (DependencyHolder, ExternalLibraryHolder)),
+            listify=True,
+            default=[]),
+    )
+    def test(self, state: 'ModuleState', args: T.Tuple[str, BuildTargetHolder], kwargs: 'FuncTest') -> ModuleReturnValue:
         """Generate a rust test target from a given rust target.
 
         Rust puts it's unitests inside it's main source files, unlike most
@@ -91,7 +112,7 @@ class RustModule(ExtensionModule):
         base_target: BuildTarget = unholder(args[1])
         if not base_target.uses_rust():
             raise InterpreterException('Second positional argument to rustmod.test() must be a rust based target')
-        extra_args = stringlistify(kwargs.get('args', []))
+        extra_args = kwargs['args']
 
         # Delete any arguments we don't want passed
         if '--test' in extra_args:
@@ -104,17 +125,17 @@ class RustModule(ExtensionModule):
             del extra_args[i + 1]
             del extra_args[i]
         for i, a in enumerate(extra_args):
-            if a.startswith('--format='):
+            if isinstance(a, str) and a.startswith('--format='):
                 del extra_args[i]
                 break
 
-        dependencies = unholder(listify(kwargs.get('dependencies', [])))
-        for d in dependencies:
-            if not isinstance(d, (Dependency, ExternalLibrary)):
-                raise InvalidArguments('dependencies must be a dependency or external library')
+        dependencies = [d.held_object for d in kwargs['dependencies']]
+
+        # We need to cast here, as currently these don't have protocol in them, but test itself does.
+        tkwargs = T.cast('_kwargs.FuncTest', kwargs.copy())
 
-        kwargs['args'] = extra_args + ['--test', '--format', 'pretty']
-        kwargs['protocol'] = 'rust'
+        tkwargs['args'] = extra_args + ['--test', '--format', 'pretty']
+        tkwargs['protocol'] = 'rust'
 
         new_target_kwargs = base_target.kwargs.copy()
         # Don't mutate the shallow copied list, instead replace it with a new
@@ -130,11 +151,9 @@ class RustModule(ExtensionModule):
             new_target_kwargs
         )
 
-        assert isinstance(self.interpreter.current_node, mparser.FunctionNode), 'for mypy'
-
         e = ExecutableHolder(new_target, self.interpreter)
         test = self.interpreter.make_test(
-            self.interpreter.current_node, (name, e), kwargs)
+            self.interpreter.current_node, (name, e), tkwargs)
 
         return ModuleReturnValue(None, [e, test])
 

test cases/failing/23 rel testdir/test.json

@@ -1,7 +1,7 @@
 {
   "stdout": [
     {
-      "line": "test cases/failing/23 rel testdir/meson.build:4:0: ERROR: Workdir keyword argument must be an absolute path."
+      "line": "test cases/failing/23 rel testdir/meson.build:4:0: ERROR: benchmark keyword argument \"workdir\" must be an absolute path"
     }
   ]
 }