Improve unit tests suite (#87)

This PR improves the unit tests suite in a couple terms:

* There used to be only tests for the `pg_backup_api.server_operation` module. Some of the tests had several implicit subtests in each function, and they were using `unittest` module. To improve readability and make it easier to write tests, we are now using `pytest`, and the tests were split into several smaller ones;
* As tests for all other modules of `pg-backup-api` were missing, this PR adds them to the suite. Again, using `pytest` and parametrizing tests with focus on readability.

As part of writing the unit tests for the Flask endpoints, a couple issues were found in the API and also fixed through this PR:
* `load_barman_config` was being called at module level instead of only when used
* POST request to `/servers/*server_name*/operations` was returning a bogus message when `backup_id` was not valid

References: BAR-132.

pg_backup_api/pg_backup_api/logic/utility_controller.py

@@ -189,15 +189,15 @@ def servers_operations_post(server_name: str,
 
     if op_type == OperationType.RECOVERY:
         try:
-            backup_id = request_body["backup_id"]
+            msg_backup_id = request_body["backup_id"]
         except KeyError:
             msg_400 = "Request body is missing ``backup_id``"
             abort(400, description=msg_400)
 
-        backup_id = parse_backup_id(Server(server), backup_id)
+        backup_id = parse_backup_id(Server(server), msg_backup_id)
 
         if not backup_id:
-            msg_404 = f"Backup '{backup_id}' does not exist"
+            msg_404 = f"Backup '{msg_backup_id}' does not exist"
             abort(404, description=msg_404)
 
         operation = RecoveryOperation(server_name)

pg_backup_api/pg_backup_api/server_operation.py

@@ -41,8 +41,6 @@
 if TYPE_CHECKING:  # pragma: no cover
     from barman.config import Config as BarmanConfig
 
-load_barman_config()
-
 logging.basicConfig(stream=sys.stdout, level=logging.DEBUG)
 log = logging.getLogger()
 
@@ -117,6 +115,8 @@ def __init__(self, name: str) -> None:
                 f"No barman config found for '{name}'."
             )
 
+        load_barman_config()
+
         if TYPE_CHECKING:  # pragma: no cover
             assert isinstance(barman.__config__, BarmanConfig)
 

pg_backup_api/pg_backup_api/tests/test_main.py

@@ -0,0 +1,156 @@
+# -*- coding: utf-8 -*-
+# © Copyright EnterpriseDB UK Limited 2021-2023
+#
+# This file is part of Postgres Backup API.
+#
+# Postgres Backup API is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Postgres Backup API is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Postgres Backup API.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Unit tests for the CLI."""
+from textwrap import dedent
+from unittest.mock import MagicMock, patch
+
+import pytest
+
+from pg_backup_api.__main__ import main
+
+
+_HELP_OUTPUT = {
+    "pg-backup-api --help": dedent("""\
+        usage: pg-backup-api [-h] {serve,status,recovery} ...
+
+        positional arguments:
+          {serve,status,recovery}
+
+        options:
+          -h, --help            show this help message and exit
+
+        Postgres Backup API by EnterpriseDB (www.enterprisedb.com)
+\
+    """),
+    "pg-backup-api serve --help": dedent("""\
+        usage: pg-backup-api serve [-h] [--port PORT]
+
+        Start the REST API server. Listen for requests on '127.0.0.1', on the given port.
+
+        options:
+          -h, --help   show this help message and exit
+          --port PORT  Port to bind to.
+\
+    """),  # noqa: E501
+    "pg-backup-api status --help": dedent("""\
+        usage: pg-backup-api status [-h] [--port PORT]
+
+        Check if the REST API server is up and running
+
+        options:
+          -h, --help   show this help message and exit
+          --port PORT  Port to be checked.
+\
+    """),  # noqa: E501
+    "pg-backup-api recovery --help": dedent("""\
+        usage: pg-backup-api recovery [-h] --server-name SERVER_NAME --operation-id OPERATION_ID
+
+        Perform a 'barman recover' through the 'pg-backup-api'. Can only be run if a recover operation has been previously registered.
+
+        options:
+          -h, --help            show this help message and exit
+          --server-name SERVER_NAME
+                                Name of the Barman server to be recovered.
+          --operation-id OPERATION_ID
+                                ID of the operation in the 'pg-backup-api'.
+\
+    """),  # noqa: E501
+}
+
+_COMMAND_FUNC = {
+    "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
+}
+
+
+@pytest.mark.parametrize("command", _HELP_OUTPUT.keys())
+def test_main_helper(command, capsys):
+    """Test :func:`main`.
+
+    Ensure all the ``--help`` calls print the expected content to the console.
+    """
+    with patch("sys.argv", command.split()), pytest.raises(SystemExit) as exc:
+        main()
+
+    assert str(exc.value) == "0"
+
+    assert capsys.readouterr().out == _HELP_OUTPUT[command]
+
+
+@pytest.mark.parametrize("command", _COMMAND_FUNC.keys())
+@pytest.mark.parametrize("output", [None, "SOME_OUTPUT"])
+@pytest.mark.parametrize("success", [False, True])
+def test_main_funcs(command, output, success, capsys):
+    """Test :func:`main`.
+
+    Ensure :func:`main` executes the expected functions, print the expected
+    messages, and exits with the expected codes.
+    """
+    mock_controller = patch(f"pg_backup_api.__main__.{_COMMAND_FUNC[command]}")
+    mock_func = mock_controller.start()
+
+    mock_func.return_value = (output, success)
+
+    with patch("sys.argv", command.split()), pytest.raises(SystemExit) as exc:
+        main()
+
+    mock_controller.stop()
+
+    assert capsys.readouterr().out == (f"{output}\n" if output else "")
+    assert str(exc.value) == ("0" if success else "-1")
+
+
+@patch("argparse.ArgumentParser.parse_args")
+def test_main_with_func(mock_parse_args, capsys):
+    """Test :func:`main`.
+
+    Ensure :func:`main` calls the function with the expected arguments, if a
+    command has a function associated with it.
+    """
+    mock_parse_args.return_value.func = MagicMock()
+    mock_func = mock_parse_args.return_value.func
+    mock_func.return_value = ("SOME_OUTPUT", True)
+
+    with pytest.raises(SystemExit) as exc:
+        main()
+
+    capsys.readouterr()  # so we don't write to stdout during unit tests
+
+    mock_func.assert_called_once_with(mock_parse_args.return_value)
+    assert str(exc.value) == "0"
+
+
+@patch("argparse.ArgumentParser.print_help")
+@patch("argparse.ArgumentParser.parse_args")
+def test_main_without_func(mock_parse_args, mock_print_help, capsys):
+    """Test :func:`main`.
+
+    Ensure :func:`main` prints a helper if a command has no function associated
+    with it.
+    """
+    delattr(mock_parse_args.return_value, "func")
+
+    with pytest.raises(SystemExit) as exc:
+        main()
+
+    capsys.readouterr()  # so we don't write to stdout during unit tests
+
+    mock_print_help.assert_called_once_with()
+    assert str(exc.value) == "0"

pg_backup_api/pg_backup_api/tests/test_run.py

@@ -0,0 +1,123 @@
+# -*- coding: utf-8 -*-
+# © Copyright EnterpriseDB UK Limited 2021-2023
+#
+# This file is part of Postgres Backup API.
+#
+# Postgres Backup API is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Postgres Backup API is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Postgres Backup API.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Unit tests for functions used by the CLI."""
+
+import argparse
+from requests.exceptions import ConnectionError
+from unittest.mock import MagicMock, patch, call
+
+import pytest
+
+from pg_backup_api.run import serve, status, recovery_operation
+
+
+@pytest.mark.parametrize("port", [7480, 7481])
+@patch("pg_backup_api.run.output")
+@patch("pg_backup_api.run.load_barman_config")
+@patch("pg_backup_api.run.app")
+def test_serve(mock_app, mock_load_config, mock_output, port):
+    """Test :func:`serve`.
+
+    Ensure :func:`serve` performs the expected calls and return the expected
+    values.
+    """
+    mock_output.AVAILABLE_WRITERS.__getitem__.return_value = MagicMock()
+    expected = mock_output.AVAILABLE_WRITERS.__getitem__.return_value
+    expected.return_value = MagicMock()
+
+    args = argparse.Namespace(port=port)
+
+    assert serve(args) == (mock_app.run.return_value, True)
+
+    mock_load_config.assert_called_once_with()
+    mock_output.set_output_writer.assert_called_once_with(
+        expected.return_value,
+    )
+    mock_app.run.assert_called_once_with(host="127.0.0.1", port=port)
+
+
+@pytest.mark.parametrize("port", [7480, 7481])
+@patch("requests.get")
+def test_status_ok(mock_request, port):
+    """Test :func:`status`.
+
+    Ensure the expected ``GET`` request is performed, and that :func:`status`
+    returns `OK` when the API is available.
+    """
+    args = argparse.Namespace(port=port)
+
+    assert status(args) == ("OK", True)
+
+    mock_request.assert_called_once_with(f"http://127.0.0.1:{port}/status")
+
+
+@pytest.mark.parametrize("port", [7480, 7481])
+@patch("requests.get")
+def test_status_failed(mock_request, port):
+    """Test :func:`status`.
+
+    Ensure the expected ``GET`` request is performed, and that :func:`status`
+    returns an error message when the API is not available.
+    """
+    args = argparse.Namespace(port=port)
+
+    mock_request.side_effect = ConnectionError("Some Error")
+
+    message = "The Postgres Backup API does not appear to be available."
+    assert status(args) == (message, False)
+
+    mock_request.assert_called_once_with(f"http://127.0.0.1:{port}/status")
+
+
+@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.RecoveryOperation")
+def test_recovery_operation(mock_rec_op, server_name, operation_id, rc):
+    """Test :func:`recovery_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_rec_op.return_value.run.return_value = ("SOME_OUTPUT", rc)
+    mock_write_output = mock_rec_op.return_value.write_output_file
+    mock_time_event = mock_rec_op.return_value.time_event_now
+    mock_read_job = mock_rec_op.return_value.read_job_file
+
+    assert recovery_operation(args) == (mock_write_output.return_value,
+                                        rc == 0)
+
+    mock_rec_op.assert_called_once_with(server_name, operation_id)
+    mock_rec_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)