From 35d0dc3d807ddbe3358fd81307379c73ee31bafb Mon Sep 17 00:00:00 2001
From: Pete Baughman <pete.baughman@apex.ai>
Date: Tue, 16 Apr 2019 13:43:14 -0700
Subject: [PATCH] Better error messages for bad parametrization

Signed-off-by: Pete Baughman <pete.baughman@apex.ai>
---
 .../apex_launchtest/apex_runner.py            | 27 ++++++++++
 .../test/test_apex_runner_validation.py       | 51 ++++++++++++++-----
 2 files changed, 65 insertions(+), 13 deletions(-)

diff --git a/apex_launchtest/apex_launchtest/apex_runner.py b/apex_launchtest/apex_launchtest/apex_runner.py
index e5da681..e0b6a3c 100644
--- a/apex_launchtest/apex_launchtest/apex_runner.py
+++ b/apex_launchtest/apex_launchtest/apex_runner.py
@@ -241,4 +241,31 @@ def validate(self):
         # Make sure the function signature of the launch configuration
         # generator is correct
         for run in self._test_runs:
+            # Drill down into any parametrized test descriptions and make sure the argument names
+            # are correct.  A simpler check can use getcallargs, but then you won't get a very
+            # helpful message.
+            base_fn = inspect.unwrap(run.test_description_function)
+            base_args = inspect.getfullargspec(base_fn)
+            base_args = base_args.args + base_args.kwonlyargs
+
+            # Check that the parametrized arguments all have a place to go
+            for argname in run.param_args.keys():
+                if argname not in base_args:
+                    raise Exception(
+                        'Could not find an argument in generate_test_description matching '
+                        "prametrized argument '{}'".format(argname)
+                    )
+
+            # Check for extra args in generate_test_description
+            for argname in base_args:
+                if argname == 'ready_fn':
+                    continue
+                if argname not in run.param_args.keys():
+                    raise Exception(
+                        "generate_test_description has unexpected extra argument '{}'".format(
+                            argname
+                        )
+                    )
+
+            # This is a double-check
             inspect.getcallargs(run.test_description_function, ready_fn=lambda: None)
diff --git a/apex_launchtest/test/test_apex_runner_validation.py b/apex_launchtest/test/test_apex_runner_validation.py
index 4ced902..746d794 100644
--- a/apex_launchtest/test/test_apex_runner_validation.py
+++ b/apex_launchtest/test/test_apex_runner_validation.py
@@ -12,10 +12,18 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import imp
 import unittest
 
+import apex_launchtest
 from apex_launchtest.apex_runner import ApexRunner
-from apex_launchtest.loader import TestRun as TR  # Named TR so pytest doesn't think it's a test
+from apex_launchtest.loader import LoadTestsFromPythonModule
+
+
+def make_test_run_for_dut(generate_test_description_function):
+    module = imp.new_module('test_module')
+    module.generate_test_description = generate_test_description_function
+    return LoadTestsFromPythonModule(module)
 
 
 class TestApexRunnerValidation(unittest.TestCase):
@@ -23,24 +31,41 @@ class TestApexRunnerValidation(unittest.TestCase):
     def test_catches_bad_signature(self):
 
         dut = ApexRunner(
-            [TR(
-                test_description_function=lambda: None,
-                param_args={},
-                pre_shutdown_tests=None,
-                post_shutdown_tests=None,
-            )]
+            make_test_run_for_dut(
+                lambda: None
+            )
         )
 
         with self.assertRaises(TypeError):
             dut.validate()
 
         dut = ApexRunner(
-            [TR(
-                test_description_function=lambda ready_fn: None,
-                param_args={},
-                pre_shutdown_tests=None,
-                post_shutdown_tests=None,
-            )]
+            make_test_run_for_dut(
+                lambda ready_fn: None
+            )
         )
 
         dut.validate()
+
+    def test_too_many_arguments(self):
+
+        dut = ApexRunner(
+            make_test_run_for_dut(lambda ready_fn, extra_arg: None)
+        )
+
+        with self.assertRaisesRegex(Exception, "unexpected extra argument 'extra_arg'"):
+            dut.validate()
+
+    def test_bad_parametrization_argument(self):
+
+        @apex_launchtest.parametrize('bad_argument', [1, 2, 3])
+        def bad_launch_description(ready_fn):
+            pass # pragma: no cover
+
+        dut = ApexRunner(
+            make_test_run_for_dut(bad_launch_description)
+        )
+
+        with self.assertRaisesRegex(Exception, 'Could not find an argument') as cm:
+            dut.validate()
+        self.assertIn('bad_argument', str(cm.exception))