WIP: Adjust unit tests and add missing unit tests

After #87 has been merged into `main`, this dev branch for BAR-125
has been rebased on top of `main`.

Given the improvements made to the unit test suite through #87,
we are now adjusting the unit tests of this dev branch:

* `test_server_operation.py` for `ConfigSwitchOperation` is now
  following the new design, which uses `pytest` and splits complex
  tests into several smaller ones;
* add tests for "config switch operation" on unit test modules that
  where introduced by #87 (`test_main.py`, `test_run.py`, and
  `test_utility_controller.py`)

Note: we are still missing the final design of the `barman config switch`
implementation in Barman, so we keep this branch as a WIP.

References: BAR-125.

Signed-off-by: Israel Barth Rubio <[email protected]>

pg_backup_api/pg_backup_api/tests/test_main.py

@@ -27,10 +27,10 @@
 
 _HELP_OUTPUT = {
     "pg-backup-api --help": dedent("""\
-        usage: pg-backup-api [-h] {serve,status,recovery} ...
+        usage: pg-backup-api [-h] {serve,status,recovery,config-switch} ...
 
         positional arguments:
-          {serve,status,recovery}
+          {serve,status,recovery,config-switch}
 
         options:
           -h, --help            show this help message and exit
@@ -69,6 +69,19 @@
                                 Name of the Barman server to be recovered.
           --operation-id OPERATION_ID
                                 ID of the operation in the 'pg-backup-api'.
+\
+    """),  # noqa: E501
+    "pg-backup-api config-switch --help": dedent("""\
+        usage: pg-backup-api config-switch [-h] --server-name SERVER_NAME --operation-id OPERATION_ID
+
+        Perform a 'barman config switch' through the 'pg-backup-api'. Can only be run if a config switch operation has been previously registered.
+
+        options:
+          -h, --help            show this help message and exit
+          --server-name SERVER_NAME
+                                Name of the Barman server which config should be switched.
+          --operation-id OPERATION_ID
+                                ID of the operation in the 'pg-backup-api'.
 \
     """),  # noqa: E501
 }
@@ -77,6 +90,7 @@
     "pg-backup-api serve": "serve",
     "pg-backup-api status": "status",
     "pg-backup-api recovery --server-name SOME_SERVER --operation-id SOME_OP_ID": "recovery_operation",  # noqa: E501
+    "pg-backup-api config-switch --server-name SOME_SERVER --operation-id SOME_OP_ID": "config_switch_operation",  # noqa: E501
 }
 
 

pg_backup_api/pg_backup_api/tests/test_run.py

@@ -24,7 +24,8 @@
 
 import pytest
 
-from pg_backup_api.run import serve, status, recovery_operation
+from pg_backup_api.run import (serve, status, recovery_operation,
+                               config_switch_operation)
 
 
 @pytest.mark.parametrize("port", [7480, 7481])
@@ -121,3 +122,41 @@ def test_recovery_operation(mock_rec_op, server_name, operation_id, rc):
     ])
 
     mock_write_output.assert_called_once_with(mock_read_job.return_value)
+
+
+@pytest.mark.parametrize("server_name", ["SERVER_1", "SERVER_2"])
+@pytest.mark.parametrize("operation_id", ["OPERATION_1", "OPERATION_2"])
+@pytest.mark.parametrize("rc", [0, 1])
+@patch("pg_backup_api.run.ConfigSwitchOperation")
+def test_config_switch_operation(mock_cs_op, server_name, operation_id, rc):
+    """Test :func:`config_switch_operation`.
+
+    Ensure the operation is created and executed, and that the expected values
+    are returned depending on the return code.
+    """
+    args = argparse.Namespace(server_name=server_name,
+                              operation_id=operation_id)
+
+    mock_cs_op.return_value.run.return_value = ("SOME_OUTPUT", rc)
+    mock_write_output = mock_cs_op.return_value.write_output_file
+    mock_time_event = mock_cs_op.return_value.time_event_now
+    mock_read_job = mock_cs_op.return_value.read_job_file
+
+    assert config_switch_operation(args) == (mock_write_output.return_value,
+                                             rc == 0)
+
+    mock_cs_op.assert_called_once_with(server_name, operation_id)
+    mock_cs_op.return_value.run.assert_called_once_with()
+    mock_time_event.assert_called_once_with()
+    mock_read_job.assert_called_once_with()
+
+    # Make sure the expected content was added to `read_job_file` output before
+    # writing it to the output file.
+    assert len(mock_read_job.return_value.__setitem__.mock_calls) == 3
+    mock_read_job.return_value.__setitem__.assert_has_calls([
+        call('success', rc == 0),
+        call('end_time', mock_time_event.return_value),
+        call('output', "SOME_OUTPUT"),
+    ])
+
+    mock_write_output.assert_called_once_with(mock_read_job.return_value)

pg_backup_api/pg_backup_api/tests/test_server_operation.py

@@ -873,54 +873,71 @@ def test__run_logic(self, mock_get_args, mock_run_subprocess, operation):
 
 
 @patch("pg_backup_api.server_operation.OperationServer", MagicMock())
-class TestConfigSwitchOperation(TestCase):
+class TestConfigSwitchOperation:
+    """Run tests for :class:`ConfigSwitchOperation`."""
 
+    @pytest.fixture
     @patch("pg_backup_api.server_operation.OperationServer", MagicMock())
-    def setUp(self):
-        self.operation = ConfigSwitchOperation(_BARMAN_SERVER)
-
-    def test__validate_job_content(self):
-        content = {}
-        # Ensure exception is raised if content is missing keys -- test 1.
-        with self.assertRaises(MalformedContent) as exc:
-            self.operation._validate_job_content(content)
-
-        # TODO: adjust the arguments in this test
-        self.assertEqual(
-            str(exc.exception),
-            "Missing required arguments: be, defined, to",
-        )
-
-        # Ensure exception is raised if content is missing keys -- test 2.
-        content["to"] = "TO_VALUE"
+    def operation(self):
+        """Create a :class:`ConfigSwitchOperation` instance for testing.
 
-        with self.assertRaises(MalformedContent) as exc:
-            self.operation._validate_job_content(content)
+        :return: a new instance of :class:`ConfigSwitchOperation` for testing.
+        """
+        return ConfigSwitchOperation(_BARMAN_SERVER)
 
-        self.assertEqual(
-            str(exc.exception),
-            "Missing required arguments: be, defined",
-        )
+    # TODO: adjust the parameter names once the final design is defined
+    @pytest.mark.parametrize("content,missing_keys", [
+        ({}, "be, defined, to",),
+        ({"to": "SOME_TO"},
+         "be, defined"),
+        ({"be": "SOME_BE"},
+         "defined, to",),
+        ({"defined": "SOME_DEFINED"},
+         "be, to",),
+        ({"to": "SOME_TO",
+          "be": "SOME_BE"},
+         "defined"),
+        ({"to": "SOME_TO",
+          "defined": "SOME_DEFINED"},
+         "be"),
+        ({"be": "SOME_BE",
+          "defined": "SOME_DEFINED"},
+         "to"),
+    ])
+    def test__validate_job_content_content_missing_keys(self, content,
+                                                        missing_keys,
+                                                        operation):
+        """Test :meth:`ConfigSwitchOperation._validate_job_content`.
 
-        # Ensure exception is raised if content is missing keys -- test 3.
-        content["be"] = "BE_VALUE"
+        Ensure and exception is raised if the content is missing keys.
+        """
+        with pytest.raises(MalformedContent) as exc:
+            operation._validate_job_content(content)
 
-        with self.assertRaises(MalformedContent) as exc:
-            self.operation._validate_job_content(content)
+        assert str(exc.value) == f"Missing required arguments: {missing_keys}"
 
-        self.assertEqual(
-            str(exc.exception),
-            "Missing required arguments: defined",
-        )
+    def test__validate_job_content_ok(self, operation):
+        """Test :meth:`ConfigSwitchOperation._validate_job_content`.
 
-        # Ensure execution is fine if everything is filled.
-        content["defined"] = "DEFINED_VALUE"
-        self.operation._validate_job_content(content)
+        Ensure execution is fine if everything is filled as expected.
+        """
+        # TODO: adjust the parameter names once the final design is defined
+        content = {
+            "to": "SOME_TO",
+            "be": "SOME_BE",
+            "defined": "SOME_DEFINED",
+        }
+        operation._validate_job_content(content)
 
     @patch("pg_backup_api.server_operation.Operation.time_event_now")
     @patch("pg_backup_api.server_operation.Operation.write_job_file")
-    def test_write_job_file(self, mock_write_job_file, mock_time_event_now):
-        # Ensure underlying methods are called as expected.
+    def test_write_job_file(self, mock_write_job_file, mock_time_event_now,
+                            operation):
+        """Test :meth:`ConfigSwitchOperation.write_job_file`.
+
+        Ensure the underlying methods are called as expected.
+        """
+        # TODO: adjust the parameter names once the final design is defined
         content = {
             "SOME": "CONTENT",
         }
@@ -930,48 +947,50 @@ def test_write_job_file(self, mock_write_job_file, mock_time_event_now):
             "start_time": "SOME_TIMESTAMP",
         }
 
-        with patch.object(self.operation, "_validate_job_content") as mock:
+        with patch.object(operation, "_validate_job_content") as mock:
             mock_time_event_now.return_value = "SOME_TIMESTAMP"
 
-            self.operation.write_job_file(content)
+            operation.write_job_file(content)
 
             mock_time_event_now.assert_called_once()
             mock.assert_called_once_with(extended_content)
             mock_write_job_file.assert_called_once_with(extended_content)
 
-    def test__get_args(self):
-        # Ensure it returns the correct arguments for 'barman recover'.
-        with patch.object(self.operation, "read_job_file") as mock:
-            # TODO: adjust the arguments in the test
+    def test__get_args(self, operation):
+        """Test :meth:`ConfigSwitchOperation._get_args`.
+
+        Ensure it returns the correct arguments for ``barman recover``.
+        """
+        # TODO: adjust the parameter names once the final design is defined
+        with patch.object(operation, "read_job_file") as mock:
             mock.return_value = {
-                "to": "TO_VALUE",
-                "be": "BE_VALUE",
-                "defined": "DEFINED_VALUE",
+                "to": "SOME_TO",
+                "be": "SOME_BE",
+                "defined": "SOME_DEFINED",
             }
 
-            self.assertEqual(
-                self.operation._get_args(),
-                [
-                    self.operation.server.name,
-                    "TO_VALUE",
-                    "BE_VALUE",
-                    "DEFINED_VALUE",
-                ]
-            )
+            expected = [
+                operation.server.name,
+                "SOME_TO",
+                "SOME_BE",
+                "SOME_DEFINED",
+            ]
+            assert operation._get_args() == expected
 
     @patch("pg_backup_api.server_operation.Operation._run_subprocess")
     @patch("pg_backup_api.server_operation.ConfigSwitchOperation._get_args")
-    def test__run_logic(self, mock_get_args, mock_run_subprocess):
+    def test__run_logic(self, mock_get_args, mock_run_subprocess, operation):
+        """Test :meth:`ConfigSwitchOperation._run_logic`.
+
+        Ensure the underlying calls occur as expected.
+        """
         arguments = ["SOME", "ARGUMENTS"]
         output = ("SOME OUTPUT", 0)
 
         mock_get_args.return_value = arguments
         mock_run_subprocess.return_value = output
 
-        self.assertEqual(
-            self.operation._run_logic(),
-            output,
-        )
+        assert operation._run_logic() == output
 
         mock_get_args.assert_called_once()
         mock_run_subprocess.assert_called_once_with(