From 19074bdb469b55206ad91e9801cdc763f3b4acec Mon Sep 17 00:00:00 2001
From: M Starch <LeStarch@googlemail.com>
Date: Mon, 25 Mar 2024 19:03:13 -0700
Subject: [PATCH] Fixing check_arguments for consistency; added UT

---
 .../common/communication/adapters/ip.py           |  4 ++--
 .../common/communication/adapters/uart.py         | 10 +++++-----
 src/fprime_gds/executables/cli.py                 |  2 +-
 test/fprime_gds/test_plugins.py                   | 15 +++++++++++++++
 4 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/src/fprime_gds/common/communication/adapters/ip.py b/src/fprime_gds/common/communication/adapters/ip.py
index bdd89ef9..0a964f2d 100644
--- a/src/fprime_gds/common/communication/adapters/ip.py
+++ b/src/fprime_gds/common/communication/adapters/ip.py
@@ -195,14 +195,14 @@ def register_communication_plugin(cls):
         return cls
 
     @classmethod
-    def check_arguments(cls, args):
+    def check_arguments(cls, address, port, server=True):
         """
         Code that should check arguments of this adapter. If there is a problem with this code, then a "ValueError"
         should be raised describing the problem with these arguments.
 
         :param args: arguments as dictionary
         """
-        check_port(args["address"], args["port"])
+        check_port(address, port)
 
 
 class IpHandler(abc.ABC):
diff --git a/src/fprime_gds/common/communication/adapters/uart.py b/src/fprime_gds/common/communication/adapters/uart.py
index 98a51fd8..20b43719 100644
--- a/src/fprime_gds/common/communication/adapters/uart.py
+++ b/src/fprime_gds/common/communication/adapters/uart.py
@@ -175,7 +175,7 @@ def register_communication_plugin(cls):
         return cls
 
     @classmethod
-    def check_arguments(cls, args):
+    def check_arguments(cls, device, baud):
         """
         Code that should check arguments of this adapter. If there is a problem with this code, then a "ValueError"
         should be raised describing the problem with these arguments.
@@ -183,16 +183,16 @@ def check_arguments(cls, args):
         :param args: arguments as dictionary
         """
         ports = map(lambda info: info.device, list_ports.comports(include_links=True))
-        if args["device"] not in ports:
-            msg = f"Serial port '{args['device']}' not valid. Available ports: {ports}"
+        if device not in ports:
+            msg = f"Serial port '{device}' not valid. Available ports: {ports}"
             raise ValueError(
                 msg
             )
         # Note: baud rate may not *always* work. These are a superset
         try:
-            baud = int(args["baud"])
+            baud = int(baud)
         except ValueError:
-            msg = f"Serial baud rate '{args['baud']}' not integer. Use one of: {SerialAdapter.BAUDS}"
+            msg = f"Serial baud rate '{baud}' not integer. Use one of: {SerialAdapter.BAUDS}"
             raise ValueError(
                 msg
             )
diff --git a/src/fprime_gds/executables/cli.py b/src/fprime_gds/executables/cli.py
index b7c390dd..20c7e528 100644
--- a/src/fprime_gds/executables/cli.py
+++ b/src/fprime_gds/executables/cli.py
@@ -419,7 +419,7 @@ def extract_plugin_arguments(args, plugin) -> Dict[str, Any]:
         }
         # Check arguments or yield a Value error
         if hasattr(plugin, "check_arguments"):
-            plugin.check_arguments(filled_arguments)
+            plugin.check_arguments(**filled_arguments)
         return filled_arguments
 
 
diff --git a/test/fprime_gds/test_plugins.py b/test/fprime_gds/test_plugins.py
index 1f911d5e..8221d73b 100644
--- a/test/fprime_gds/test_plugins.py
+++ b/test/fprime_gds/test_plugins.py
@@ -111,6 +111,12 @@ def get_arguments(cls):
             },
         }
 
+    @classmethod
+    def check_arguments(cls, my_fancy_arg, fancy_2):
+        """ Check arguments to raise ValueError """
+        if fancy_2 < 0:
+            raise ValueError("Must be positive")
+
 
 class StartFunction(GdsFunction):
     """ A plugin implementation that starts a function
@@ -275,6 +281,15 @@ def test_plugin_arguments(plugins):
     assert args.framing_selection_instance.fancy_2 == int(a_number), "Integer argument did not process"
 
 
+def test_plugin_check_arguments(plugins):
+    """ Tests that arguments are validated in plugins """
+    a_string = "a_string"
+    a_number = "-20"
+    to_parse = ["--framing", "good-with-args", "--my-fancy-arg", a_string, "--my-fancy-arg-with-dest", a_number]
+    with pytest.raises(SystemExit):
+        args, _ = ParserBase.parse_args([PluginArgumentParser,], arguments=to_parse)
+
+
 @pytest.mark.parametrize("start_up", [(f"{__name__}:StartFunction", [])], indirect=True)
 def test_start_function(start_up):
     """ Test start-up functions """