From ef7b5d6572a6832fd079df7aafcf3d25797271f4 Mon Sep 17 00:00:00 2001 From: Israel Barth Rubio Date: Thu, 18 Jan 2024 08:46:44 -0300 Subject: [PATCH] Add unit tests for remote `barman config-update` and instance operations References: BAR-126. Signed-off-by: Israel Barth Rubio --- .../pg_backup_api/tests/test_main.py | 18 +++- pg_backup_api/pg_backup_api/tests/test_run.py | 39 +++++++- .../tests/test_server_operation.py | 92 +++++++++++++++++++ .../tests/test_utility_controller.py | 61 +++++++++++- 4 files changed, 202 insertions(+), 8 deletions(-) diff --git a/pg_backup_api/pg_backup_api/tests/test_main.py b/pg_backup_api/pg_backup_api/tests/test_main.py index 7394157..af5190f 100644 --- a/pg_backup_api/pg_backup_api/tests/test_main.py +++ b/pg_backup_api/pg_backup_api/tests/test_main.py @@ -28,10 +28,11 @@ _HELP_OUTPUT = { "pg-backup-api --help": dedent("""\ - usage: pg-backup-api [-h] {serve,status,recovery,config-switch} ... + usage: pg-backup-api [-h] + {serve,status,recovery,config-switch,config-update} ... positional arguments: - {serve,status,recovery,config-switch} + {serve,status,recovery,config-switch,config-update} optional arguments: -h, --help show this help message and exit @@ -89,6 +90,18 @@ switched. --operation-id OPERATION_ID ID of the operation in the 'pg-backup-api'. +\ + """), # noqa: E501 + "pg-backup-api config-update --help": dedent("""\ + usage: pg-backup-api config-update [-h] --operation-id OPERATION_ID + + Perform a 'barman config-update' through the 'pg-backup-api'. Can only be run + if a config-update operation has been previously registered. + + optional arguments: + -h, --help show this help message and exit + --operation-id OPERATION_ID + ID of the operation in the 'pg-backup-api'. \ """), # noqa: E501 } @@ -98,6 +111,7 @@ "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 config-update --operation-id SOME_OP_ID": "config_update_operation", # noqa: E501 } diff --git a/pg_backup_api/pg_backup_api/tests/test_run.py b/pg_backup_api/pg_backup_api/tests/test_run.py index f53b0cd..2d07e8b 100644 --- a/pg_backup_api/pg_backup_api/tests/test_run.py +++ b/pg_backup_api/pg_backup_api/tests/test_run.py @@ -25,7 +25,8 @@ import pytest from pg_backup_api.run import (serve, status, recovery_operation, - config_switch_operation) + config_switch_operation, + config_update_operation) @pytest.mark.parametrize("port", [7480, 7481]) @@ -160,3 +161,39 @@ def test_config_switch_operation(mock_cs_op, server_name, operation_id, rc): ]) mock_write_output.assert_called_once_with(mock_read_job.return_value) + + +@pytest.mark.parametrize("operation_id", ["OPERATION_1", "OPERATION_2"]) +@pytest.mark.parametrize("rc", [0, 1]) +@patch("pg_backup_api.run.ConfigUpdateOperation") +def test_config_switch_operation(mock_cu_op, operation_id, rc): + """Test :func:`config_update_operation`. + + Ensure the operation is created and executed, and that the expected values + are returned depending on the return code. + """ + args = argparse.Namespace(operation_id=operation_id) + + mock_cu_op.return_value.run.return_value = ("SOME_OUTPUT", rc) + mock_write_output = mock_cu_op.return_value.write_output_file + mock_time_event = mock_cu_op.return_value.time_event_now + mock_read_job = mock_cu_op.return_value.read_job_file + + assert config_update_operation(args) == (mock_write_output.return_value, + rc == 0) + + mock_cu_op.assert_called_once_with(None, operation_id) + mock_cu_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) diff --git a/pg_backup_api/pg_backup_api/tests/test_server_operation.py b/pg_backup_api/pg_backup_api/tests/test_server_operation.py index d0f0898..38b403e 100644 --- a/pg_backup_api/pg_backup_api/tests/test_server_operation.py +++ b/pg_backup_api/pg_backup_api/tests/test_server_operation.py @@ -31,6 +31,7 @@ Operation, RecoveryOperation, ConfigSwitchOperation, + ConfigUpdateOperation, ) @@ -1052,3 +1053,94 @@ def test__run_logic(self, mock_get_args, mock_run_subprocess, operation): mock_run_subprocess.assert_called_once_with( ["barman", "config-switch"] + arguments, ) + + +@patch("pg_backup_api.server_operation.OperationServer", MagicMock()) +class TestConfigUpdateOperation: + """Run tests for :class:`ConfigUpdateOperation`.""" + + @pytest.fixture + @patch("pg_backup_api.server_operation.OperationServer", MagicMock()) + def operation(self): + """Create a :class:`ConfigUpdateOperation` instance for testing. + + :return: a new instance of :class:`ConfigUpdateOperation` for testing. + """ + return ConfigUpdateOperation(None) + + def test__validate_job_content_content_missing_keys(self, operation): + """Test :meth:`ConfigUpdateOperation._validate_job_content`. + + Ensure an exception is raised if the content is missing the required + keys. + """ + with pytest.raises(MalformedContent) as exc: + operation._validate_job_content({}) + + assert str(exc.value) == ( + "Missing required arguments: changes" + ) + + def test__validate_job_content_ok(self, operation): + """Test :meth:`ConfigUpdateOperation._validate_job_content`. + + Ensure execution is fine if required keys are giving. + """ + operation._validate_job_content({"changes": "SOME_CHANGES"}) + + @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, + operation): + """Test :meth:`ConfigUpdateOperation.write_job_file`. + + Ensure the underlying methods are called as expected. + """ + content = { + "SOME": "CONTENT", + } + extended_content = { + "SOME": "CONTENT", + "operation_type": OperationType.CONFIG_UPDATE.value, + "start_time": "SOME_TIMESTAMP", + } + + with patch.object(operation, "_validate_job_content") as mock: + mock_time_event_now.return_value = "SOME_TIMESTAMP" + + 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, operation): + """Test :meth:`ConfigUpdateOperation._get_args`. + + Ensure it returns the correct arguments for ``barman config-update``. + """ + with patch.object(operation, "read_job_file") as mock: + mock.return_value = {"changes": [{"SOME": "CHANGES"}]} + + expected = ['[{"SOME": "CHANGES"}]'] + assert operation._get_args() == expected + + @patch("pg_backup_api.server_operation.Operation._run_subprocess") + @patch("pg_backup_api.server_operation.ConfigUpdateOperation._get_args") + def test__run_logic(self, mock_get_args, mock_run_subprocess, operation): + """Test :meth:`ConfigUpdateOperation._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 + + assert operation._run_logic() == output + + mock_get_args.assert_called_once() + mock_run_subprocess.assert_called_once_with( + ["barman", "config-update"] + arguments, + ) diff --git a/pg_backup_api/pg_backup_api/tests/test_utility_controller.py b/pg_backup_api/pg_backup_api/tests/test_utility_controller.py index 5327f59..b7cc281 100644 --- a/pg_backup_api/pg_backup_api/tests/test_utility_controller.py +++ b/pg_backup_api/pg_backup_api/tests/test_utility_controller.py @@ -777,21 +777,72 @@ def test_instance_operation_post_empty_json(self, mock_popen, mock_op_type, mock_popen.assert_not_called() @patch("pg_backup_api.logic.utility_controller.OperationServer", Mock()) - def test_server_operation_post_ok(self, client): - """Test ``/operations`` endpoint. + @patch("pg_backup_api.logic.utility_controller.OperationType") + @patch("pg_backup_api.logic.utility_controller.ConfigUpdateOperation") + @patch("subprocess.Popen") + def test_instance_operation_post_cu_op_missing_options(self, mock_popen, + mock_cu_op, + mock_op_type, + client): + """Test ``operations`` endpoint. + + Ensure ``POST`` request returns ``400`` if any option is missing when + requesting a config update operation. + """ + path = "operations" + json_data = { + "type": "config_update", + } + + mock_op_type.return_value = mock_op_type.CONFIG_UPDATE + mock_cu_op.return_value.id = "SOME_OP_ID" + mock_write_job = mock_cu_op.return_value.write_job_file + mock_write_job.side_effect = MalformedContent("SOME_ERROR") + + response = client.post(path, json=json_data) + + mock_op_type.assert_called_once_with("config_update") + mock_cu_op.assert_called_once_with(None) + mock_write_job.assert_called_once_with(json_data) + mock_popen.assert_not_called() + + assert response.status_code == 400 + expected = b"Make sure all options/arguments are met and try again" + assert expected in response.data + + @patch("pg_backup_api.logic.utility_controller.OperationServer", Mock()) + @patch("pg_backup_api.logic.utility_controller.OperationType") + @patch("pg_backup_api.logic.utility_controller.ConfigUpdateOperation") + @patch("subprocess.Popen") + def test_instance_operation_post_cu_ok(self, mock_popen, mock_cu_op, + mock_op_type, client): + """Test ``operations`` endpoint. Ensure ``POST`` request returns ``202`` if everything is ok when - requesting an instance operation. + requesting a config-update operation, and ensure the subprocess is + started. """ path = "/operations" json_data = { - "SOME": "THING", + "type": "config_update", + "changes": "SOME_CHANGES", } + mock_op_type.return_value = mock_op_type.CONFIG_UPDATE + mock_cu_op.return_value.id = "SOME_OP_ID" + response = client.post(path, json=json_data) + mock_write_job = mock_cu_op.return_value.write_job_file + mock_op_type.assert_called_once_with("config_update") + mock_cu_op.assert_called_once_with(None) + mock_write_job.assert_called_once_with(json_data) + mock_popen.assert_called_once_with(["pg-backup-api", "config-update", + "--operation-id", + "SOME_OP_ID"]) + assert response.status_code == 202 - assert response.data == b'{"operation_id":"DUMMY"}\n' + assert response.data == b'{"operation_id":"SOME_OP_ID"}\n' def test_instance_operation_not_allowed(self, client): """Test ``/operations`` endpoint.