From 337288fcc046c7b49293bc05bfe5834598fab8c3 Mon Sep 17 00:00:00 2001
From: Jiri Podivin <jpodivin@redhat.com>
Date: Wed, 26 May 2021 10:00:19 +0200
Subject: [PATCH] Refactoring of the cli parseractions module

Condition simplified, new formatting mechanism
introduced in the existing code.

Test coverage for the module included.

Signed-off-by: Jiri Podivin <jpodivin@redhat.com>
Change-Id: I5a7d22a8c58246c937eb1a15314476229dc66566
---
 validations_libs/cli/parseractions.py         |  16 ++-
 validations_libs/tests/cli/fakes.py           |   8 ++
 .../tests/cli/test_parseractions.py           | 104 ++++++++++++++++++
 3 files changed, 123 insertions(+), 5 deletions(-)
 create mode 100644 validations_libs/tests/cli/test_parseractions.py

diff --git a/validations_libs/cli/parseractions.py b/validations_libs/cli/parseractions.py
index 68b4e116..6fec6cb6 100644
--- a/validations_libs/cli/parseractions.py
+++ b/validations_libs/cli/parseractions.py
@@ -33,13 +33,19 @@ class KeyValueAction(argparse.Action):
             setattr(namespace, self.dest, {})
 
         # Add value if an assignment else remove it
-        if '=' in values and values.count('=') == 1:
+        if values.count('=') == 1:
             values_list = values.split('=', 1)
             if '' == values_list[0]:
-                msg = ("Property key must be specified: %s")
-                raise argparse.ArgumentTypeError(msg % str(values))
+                msg = (
+                    "Property key must be specified: {}"
+                ).format(str(values))
+
+                raise argparse.ArgumentTypeError(msg)
             else:
                 getattr(namespace, self.dest, {}).update([values_list])
         else:
-            msg = ("Expected 'key=value' type, but got: %s")
-            raise argparse.ArgumentTypeError(msg % str(values))
+            msg = (
+                "Expected 'key=value' type, but got: {}"
+            ).format(str(values))
+
+            raise argparse.ArgumentTypeError(msg)
diff --git a/validations_libs/tests/cli/fakes.py b/validations_libs/tests/cli/fakes.py
index ba801f0e..73d5a493 100644
--- a/validations_libs/tests/cli/fakes.py
+++ b/validations_libs/tests/cli/fakes.py
@@ -40,3 +40,11 @@ class BaseCommand(TestCase):
     def setUp(self):
         super(BaseCommand, self).setUp()
         self.app = app.ValidationCliApp()
+
+KEYVALUEACTION_VALUES = {
+    'valid': 'foo=bar',
+    'invalid_noeq': 'foo>bar',
+    'invalid_multieq': 'foo===bar',
+    'invalid_nokey': '=bar',
+    'invalid_multikey': 'foo=bar,fizz=buzz'
+}
diff --git a/validations_libs/tests/cli/test_parseractions.py b/validations_libs/tests/cli/test_parseractions.py
new file mode 100644
index 00000000..2b3b7b94
--- /dev/null
+++ b/validations_libs/tests/cli/test_parseractions.py
@@ -0,0 +1,104 @@
+#   Copyright 2021 Red Hat, Inc.
+#
+#   Licensed under the Apache License, Version 2.0 (the "License"); you may
+#   not use this file except in compliance with the License. You may obtain
+#   a copy of the License at
+#
+#        http://www.apache.org/licenses/LICENSE-2.0
+#
+#   Unless required by applicable law or agreed to in writing, software
+#   distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#   WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#   License for the specific language governing permissions and limitations
+#   under the License.
+#
+try:
+    from unittest import mock
+except ImportError:
+    import mock
+
+from unittest import TestCase
+from validations_libs.cli import parseractions
+
+import argparse
+from validations_libs.tests.cli.fakes import KEYVALUEACTION_VALUES
+
+
+class TestParserActions(TestCase):
+
+    def setUp(self):
+        self.action = parseractions.KeyValueAction("", "fizz")
+        self.mock_parser = mock.MagicMock()
+        self.test_values = KEYVALUEACTION_VALUES
+
+        self.mock_namespace = mock.MagicMock()
+        self.mock_namespace.fizz = None
+
+        super(TestParserActions, self).setUp()
+
+    def test_keyvalueaction_valid(self):
+
+        self.action(
+            self.mock_parser,
+            self.mock_namespace,
+            self.test_values['valid'])
+
+        self.assertIn('fizz', dir(self.mock_namespace))
+        self.assertDictEqual({'foo': 'bar'}, self.mock_namespace.fizz)
+        self.tearDown()
+
+    def test_keyvalueaction_invalid_no_eq_sign(self):
+
+        self.assertRaises(
+            argparse.ArgumentTypeError,
+            self.action,
+            self.mock_parser,
+            self.mock_namespace,
+            self.test_values['invalid_noeq']
+        )
+
+        self.assertIn('fizz', dir(self.mock_namespace))
+        self.assertDictEqual({}, self.mock_namespace.fizz)
+        self.tearDown()
+
+    def test_keyvalueaction_invalid_invalid_multieq(self):
+
+        self.assertRaises(
+            argparse.ArgumentTypeError,
+            self.action,
+            self.mock_parser,
+            self.mock_namespace,
+            self.test_values['invalid_multieq']
+        )
+
+        self.assertIn('fizz', dir(self.mock_namespace))
+        self.assertDictEqual({}, self.mock_namespace.fizz)
+        self.tearDown()
+
+    def test_keyvalueaction_invalid_invalid_nokey(self):
+
+        self.assertRaises(
+            argparse.ArgumentTypeError,
+            self.action,
+            self.mock_parser,
+            self.mock_namespace,
+            self.test_values['invalid_nokey']
+        )
+
+        self.assertIn('fizz', dir(self.mock_namespace))
+        self.assertDictEqual({}, self.mock_namespace.fizz)
+        self.tearDown()
+
+    def test_keyvalueaction_invalid_invalid_multikey(self):
+
+        self.assertRaises(
+            argparse.ArgumentTypeError,
+            self.action,
+            self.mock_parser,
+            self.mock_namespace,
+            self.test_values['invalid_multikey']
+        )
+
+        self.assertIn('fizz', dir(self.mock_namespace))
+        self.assertDictEqual({}, self.mock_namespace.fizz)
+        self.tearDown()