From 455f78311278e1ae76d34cb56278952cfe8de41c Mon Sep 17 00:00:00 2001 From: Israel Date: Fri, 24 Nov 2023 08:19:43 -0300 Subject: [PATCH] Add workflow for performing tests on pushes and PRs (#88) This PR adds a `tox.ini` file to the repository. It exposes a few different test environments which can be used by the developer for: * Linting the code * Checking Python modules dependencies * Running unit tests with coverage report * Running a static type checker Instructions on how to use `tox` have been added to the `README.md` file. These facilities exposed by the implemented `tox.ini` file are used by the `tests.yml` workflow which is also introduced by this PR. It is responsible for running all the tests which are exposed by `tox.ini` As part of this PR we also applied a few fixes which were reported by the workflow runs: * Issues reported by `flake8` in `setup.py` * Issues reported by `pyright` in `server_operation.py`, `utils.py`, and `utility_controller.py` * Issues on unit test `test_main_helper`: it was failing because of a difference between the terminal size of runners and our laptops * Issues on unit test `test_server_operation_post_not_json` when ran through GitHub Actions with Python 3.7 + Flask 2.2.5. That combination caused Flask to return `400 Bad Request` instead of `415 Unsupported Media Type`, which is returned by other combinations References: BAR-133. --- .github/workflows/tests.yml | 135 ++++++++++++++++++ README.md | 57 +++++++- .../pg_backup_api/logic/utility_controller.py | 11 +- .../pg_backup_api/server_operation.py | 13 +- .../pg_backup_api/tests/test_main.py | 32 +++-- .../tests/test_utility_controller.py | 19 ++- pg_backup_api/pg_backup_api/utils.py | 18 ++- pg_backup_api/pyrightconfig.json | 9 ++ pg_backup_api/requirements-test.txt | 3 + pg_backup_api/setup.py | 8 +- pg_backup_api/tox.ini | 92 ++++++++++++ 11 files changed, 367 insertions(+), 30 deletions(-) create mode 100644 .github/workflows/tests.yml create mode 100644 pg_backup_api/pyrightconfig.json create mode 100644 pg_backup_api/requirements-test.txt create mode 100644 pg_backup_api/tox.ini diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml new file mode 100644 index 0000000..0d2fd36 --- /dev/null +++ b/.github/workflows/tests.yml @@ -0,0 +1,135 @@ +name: Tests + +on: + pull_request: + + push: + branches: + - main + + workflow_dispatch: + inputs: + source-ref: + description: Source code branch/ref name + default: main + required: true + type: string + +env: + SOURCE_REF: ${{ inputs.source-ref || github.ref }} + WORKING_DIRECTORY: ./pg_backup_api + +jobs: + lint: + runs-on: ubuntu-latest + + steps: + - name: Checkout the source code + uses: actions/checkout@v3 + with: + ref: ${{ env.SOURCE_REF }} + + - name: Set up Python + uses: actions/setup-python@v4 + with: + python-version: 3.x + + - name: Install tox + run: + pip install tox + + - name: Run linter + working-directory: ${{ env.WORKING_DIRECTORY }} + run: + tox -e lint + + dependency_checking: + runs-on: ubuntu-latest + + steps: + - name: Checkout the source code + uses: actions/checkout@v3 + with: + ref: ${{ env.SOURCE_REF }} + + - name: Set up Python + uses: actions/setup-python@v4 + with: + python-version: 3.x + + - name: Install tox + run: + pip install tox + + - name: Run dependency checker + working-directory: ${{ env.WORKING_DIRECTORY }} + run: + tox -e dep + + unit_tests: + runs-on: ubuntu-latest + + strategy: + fail-fast: false + + matrix: + python-version: + - '3.7' + - '3.8' + - '3.9' + - '3.10' + - '3.11' + + steps: + - name: Checkout the source code + uses: actions/checkout@v3 + with: + ref: ${{ env.SOURCE_REF }} + + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v4 + with: + python-version: ${{ matrix.python-version }} + + - name: Install tox + run: + pip install tox + + - name: Run unit tests + working-directory: ${{ env.WORKING_DIRECTORY }} + run: + tox -m test + + static_type_checking: + runs-on: ubuntu-latest + + strategy: + fail-fast: false + + matrix: + python-version: + - '3.7' + - '3.8' + - '3.9' + - '3.10' + - '3.11' + + steps: + - name: Checkout the source code + uses: actions/checkout@v3 + with: + ref: ${{ env.SOURCE_REF }} + + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v4 + with: + python-version: ${{ matrix.python-version }} + + - name: Install tox + run: + pip install tox + + - name: Run static type checks + working-directory: ${{ env.WORKING_DIRECTORY }} + run: + tox -m type diff --git a/README.md b/README.md index c4cbafd..58cc0b8 100644 --- a/README.md +++ b/README.md @@ -117,12 +117,61 @@ The command returns `"OK"` if the app is up and running. ## Testing -You can run unit tests through `pytest`: +The repository contains a `tox.ini` file which declares a set of test +environments that are available. + +In the following sub-sections you will find more information about how to +manually run each of the tests. All of them assume you are inside the +`pg_backup_api` folder which can be found in the repository root directory. + +**Note:** install `tox` Python module if you don't have it yet in your +environment. + +### Lint + +You can run the `flake8` linter over the code by running this command: ```bash -cd pg-backup-api/pg_backup_api -python3 -m pytest +tox -e lint ``` -**Note:** install `pytest` Python module if you don't have it yet in your +It will check the source code, tests, and `setup.py`. + +### Dependency checking + +You can run the dependency checker `pipdeptree` by running this command: + +```bash +tox -e dep +``` + +It will print the tree of Python modules used by `pg-backup-api`, which can be +helpful in solving conflicts.. + +### Unit tests + +You can run unit tests by running this command: + +```bash +tox -m test +``` + +It will run unit tests using `pytest` module and `pytest-cov` plugin for +coverage report. + +**Note:** the command will take care of running the tests using all Python +versions which are supported by `pg-backup-api` and that are available in your environment. + +### Static type checking + +You can run the static type checker `pyright` over the source code by running +this command: + +```bash +tox -m type +``` + +**Note:** the command will take care of running the static type checker using +all Python versions which are supported by `pg-backup-api` and that are +available in your environment. diff --git a/pg_backup_api/pg_backup_api/logic/utility_controller.py b/pg_backup_api/pg_backup_api/logic/utility_controller.py index b84c788..dff02a1 100644 --- a/pg_backup_api/pg_backup_api/logic/utility_controller.py +++ b/pg_backup_api/pg_backup_api/logic/utility_controller.py @@ -40,6 +40,8 @@ if TYPE_CHECKING: # pragma: no cover from flask import Request, Response + from barman.config import Config as BarmanConfig + from pg_backup_api.server_operation import Operation @app.route("/diagnose", methods=["GET"]) @@ -53,11 +55,15 @@ def diagnose() -> 'Response': """ # Reload the barman config so that any changes are picked up load_barman_config() + + if TYPE_CHECKING: # pragma: no cover + assert isinstance(barman.__config__, BarmanConfig) + # Get every server (both inactive and temporarily disabled) servers = barman.__config__.server_names() server_dict = {} - for server in servers: + for server in servers: # pyright: ignore conf = barman.__config__.get_server(server) if conf is None: # Unknown server @@ -214,6 +220,9 @@ def servers_operations_post(server_name: str, ) subprocess.Popen(cmd.split()) + if TYPE_CHECKING: # pragma: no cover + assert isinstance(operation, Operation) + return {"operation_id": operation.id} diff --git a/pg_backup_api/pg_backup_api/server_operation.py b/pg_backup_api/pg_backup_api/server_operation.py index b79c143..52cf028 100644 --- a/pg_backup_api/pg_backup_api/server_operation.py +++ b/pg_backup_api/pg_backup_api/server_operation.py @@ -30,7 +30,7 @@ import os import subprocess import sys -from typing import (Any, Callable, Dict, List, Optional, Set, Tuple, +from typing import (Any, Callable, Dict, List, Optional, Set, Tuple, Union, TYPE_CHECKING) from datetime import datetime @@ -494,7 +494,8 @@ def get_status(self) -> str: return self.server.get_operation_status(self.id) @staticmethod - def _run_subprocess(cmd: List[str]) -> Tuple[Optional[str], int]: + def _run_subprocess(cmd: List[str]) -> \ + Tuple[Union[str, bytearray, memoryview], Union[int, Any]]: """ Run *cmd* as a subprocess. @@ -512,7 +513,8 @@ def _run_subprocess(cmd: List[str]) -> Tuple[Optional[str], int]: return stdout, process.returncode @abstractmethod - def _run_logic(self) -> Tuple[Optional[str], int]: + def _run_logic(self) -> \ + Tuple[Union[str, bytearray, memoryview], Union[int, Any]]: """ Logic to be ran when executing the operation. @@ -526,7 +528,7 @@ def _run_logic(self) -> Tuple[Optional[str], int]: """ pass - def run(self) -> Tuple[Optional[str], int]: + def run(self) -> Tuple[Union[str, bytearray, memoryview], Union[int, Any]]: """ Run the operation. @@ -621,7 +623,8 @@ def _get_args(self) -> List[str]: remote_ssh_command, ] - def _run_logic(self) -> Tuple[Optional[str], int]: + def _run_logic(self) -> \ + Tuple[Union[str, bytearray, memoryview], Union[int, Any]]: """ Logic to be ran when executing the recovery operation. 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 9f072f4..5d548ef 100644 --- a/pg_backup_api/pg_backup_api/tests/test_main.py +++ b/pg_backup_api/pg_backup_api/tests/test_main.py @@ -17,6 +17,7 @@ # along with Postgres Backup API. If not, see . """Unit tests for the CLI.""" +import sys from textwrap import dedent from unittest.mock import MagicMock, patch @@ -32,7 +33,7 @@ positional arguments: {serve,status,recovery} - options: + optional arguments: -h, --help show this help message and exit Postgres Backup API by EnterpriseDB (www.enterprisedb.com) @@ -41,9 +42,10 @@ "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. + Start the REST API server. Listen for requests on '127.0.0.1', on the given + port. - options: + optional arguments: -h, --help show this help message and exit --port PORT Port to bind to. \ @@ -53,17 +55,19 @@ Check if the REST API server is up and running - options: + optional arguments: -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 + 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. + Perform a 'barman recover' through the 'pg-backup-api'. Can only be run if a + recover operation has been previously registered. - options: + optional arguments: -h, --help show this help message and exit --server-name SERVER_NAME Name of the Barman server to be recovered. @@ -81,17 +85,27 @@ @pytest.mark.parametrize("command", _HELP_OUTPUT.keys()) -def test_main_helper(command, capsys): +@patch("shutil.get_terminal_size") +def test_main_helper(mock_term_size, command, capsys): """Test :func:`main`. Ensure all the ``--help`` calls print the expected content to the console. """ + # Get a predictable print size + mock_term_size.return_value.columns = 80 + with patch("sys.argv", command.split()), pytest.raises(SystemExit) as exc: main() assert str(exc.value) == "0" - assert capsys.readouterr().out == _HELP_OUTPUT[command] + expected = _HELP_OUTPUT[command] + version = sys.version_info + + if version.major >= 3 and version.minor >= 10: + expected = expected.replace("optional arguments:", "options:") + + assert capsys.readouterr().out == expected @pytest.mark.parametrize("command", _COMMAND_FUNC.keys()) 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 1aeee3d..ce35357 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 @@ -17,9 +17,12 @@ # along with Postgres Backup API. If not, see . """Unit tests for the REST API endpoints.""" +from distutils.version import StrictVersion import json +import sys from unittest.mock import Mock, MagicMock, patch +import flask import pytest from pg_backup_api.server_operation import (OperationServerConfigError, @@ -252,8 +255,20 @@ def test_server_operation_post_not_json(self, client): response = client.post(path, data={}) - assert response.status_code == 415 - assert b"Unsupported Media Type" in response.data + expected_status_code = 415 + expected_data = b"Unsupported Media Type" + version = sys.version_info + + # This is an issue which was detected while running tests through + # GitHub Actions when using Python 3.7 and Flask 2.2.5. We might want + # to remove this once we remove support for Python 3.7 + if version.major <= 3 and version.minor <= 7 and \ + StrictVersion(flask.__version__) <= StrictVersion("2.2.5"): + expected_status_code = 400 + expected_data = b"Bad Request" + + assert response.status_code == expected_status_code + assert expected_data in response.data @patch("pg_backup_api.logic.utility_controller.OperationServer", Mock()) @patch("pg_backup_api.logic.utility_controller.get_server_by_name") diff --git a/pg_backup_api/pg_backup_api/utils.py b/pg_backup_api/pg_backup_api/utils.py index ce1ec7b..736c3e4 100644 --- a/pg_backup_api/pg_backup_api/utils.py +++ b/pg_backup_api/pg_backup_api/utils.py @@ -34,7 +34,8 @@ if TYPE_CHECKING: # pragma: no cover import flask.app - from barman.config import ServerConfig + from barman.config import Config as BarmanConfig, ServerConfig + import barman.server CONFIG_FILENAME = "/etc/barman.conf" @@ -98,7 +99,10 @@ def get_server_by_name(server_name: str) -> Optional['ServerConfig']: :return: configuration of Barman server *server_name* if that server exists, ``None`` otherwise. """ - for server in barman.__config__.server_names(): + if TYPE_CHECKING: # pragma: no cover + assert isinstance(barman.__config__, BarmanConfig) + + for server in barman.__config__.server_names(): # pyright: ignore conf = barman.__config__.get_server(server) if server == server_name: return conf @@ -120,11 +124,13 @@ def parse_backup_id(server: barman.server.Server, :return: information about the backup, if *backup_id* can be satisfied, ``None`` otherwise. """ + parsed_backup_id = backup_id + if backup_id in ("latest", "last"): - backup_id = server.get_last_backup_id() + parsed_backup_id = server.get_last_backup_id() elif backup_id in ("oldest", "first"): - backup_id = server.get_first_backup_id() + parsed_backup_id = server.get_first_backup_id() elif backup_id in ("last-failed"): - backup_id = server.get_last_backup_id([BackupInfo.FAILED]) + parsed_backup_id = server.get_last_backup_id([BackupInfo.FAILED]) - return server.get_backup(backup_id) + return server.get_backup(parsed_backup_id) diff --git a/pg_backup_api/pyrightconfig.json b/pg_backup_api/pyrightconfig.json new file mode 100644 index 0000000..2bc595f --- /dev/null +++ b/pg_backup_api/pyrightconfig.json @@ -0,0 +1,9 @@ +{ + "include": [ + "pg_backup_api" + ], + + "exclude": [ + "pg_backup_api/tests" + ], +} \ No newline at end of file diff --git a/pg_backup_api/requirements-test.txt b/pg_backup_api/requirements-test.txt new file mode 100644 index 0000000..8c9cb5f --- /dev/null +++ b/pg_backup_api/requirements-test.txt @@ -0,0 +1,3 @@ +pytest +pytest-cov +pytest-html diff --git a/pg_backup_api/setup.py b/pg_backup_api/setup.py index 69b1c4f..74ed781 100644 --- a/pg_backup_api/setup.py +++ b/pg_backup_api/setup.py @@ -16,7 +16,6 @@ # You should have received a copy of the GNU General Public License # along with Postgres Backup API. If not, see . -import sys from setuptools import setup, find_packages NAME = "pg-backup-api" @@ -31,7 +30,8 @@ # prerequisite: setuptools # http://pypi.python.org/pypi/setuptools -REQUIRES = ["barman>=2.19,<4.0.0", "Flask>=0.10.1,<3.0.0", "requests>=2.0.0,<3.0.0"] +REQUIRES = ["barman>=2.19,<4.0.0", "Flask>=0.10.1,<3.0.0", + "requests>=2.0.0,<3.0.0"] setup( name=NAME, @@ -45,7 +45,9 @@ install_requires=REQUIRES, packages=find_packages(exclude=["tests"]), include_package_data=True, - entry_points={"console_scripts": ["pg-backup-api=pg_backup_api.__main__:main"]}, + entry_points={ + "console_scripts": ["pg-backup-api=pg_backup_api.__main__:main"], + }, license="GPL-3.0", long_description="""\ A server that provides an HTTP API to interact with Postgres backups diff --git a/pg_backup_api/tox.ini b/pg_backup_api/tox.ini new file mode 100644 index 0000000..dfced40 --- /dev/null +++ b/pg_backup_api/tox.ini @@ -0,0 +1,92 @@ +[common] +python_matrix = {37,38,39,310,311} +platforms = + lin: linux + mac: darwin + win: win32 + +[tox] +min_version = 4.0 +requires = + tox>4 +env_list = + dep + lint + py{[common]python_matrix}-type-{lin,mac,win} + py{[common]python_matrix}-test-{lin,mac,win} +skipsdist = True +toxworkdir = {env:TOX_WORK_DIR:.tox} +skip_missing_interpreters = True + +[testenv] +setenv = + PYTHONDONTWRITEBYTECODE = 1 + mac: OPEN_CMD = {env:OPEN_CMD:open} + lin: OPEN_CMD = {env:OPEN_CMD:xdg-open} +passenv = + BROWSER + DISPLAY + +[testenv:lint] +description = Lint code with flake8 +commands = flake8 {posargs:pg_backup_api setup.py} +deps = + flake8 + +[testenv:py{37,38,39,310,311}-test-{lin,win,mac}] +description = Run unit tests with pytest +labels = + test +commands_pre = + - {tty:rm -f "{toxworkdir}{/}cov_report_{env_name}_html{/}index.html":true} + - {tty:rm -f "{toxworkdir}{/}pytest_report_{env_name}.html":true} +commands = + pytest \ + -p no:cacheprovider \ + -vv \ + --capture=no \ + --cov=pg_backup_api \ + --cov-report=term-missing \ + --cov-append \ + {tty::--cov-report="xml\:{toxworkdir}{/}cov_report.{env_name}.xml"} \ + {tty:--cov-report="html\:{toxworkdir}{/}cov_report_{env_name}_html":} \ + {tty:--html="{toxworkdir}{/}pytest_report_{env_name}.html":} \ + {posargs:pg_backup_api} +commands_post = + - {tty:{env:OPEN_CMD} "{toxworkdir}{/}cov_report_{env_name}_html{/}index.html":true} + - {tty:{env:OPEN_CMD} "{toxworkdir}{/}pytest_report_{env_name}.html":true} +deps = + -r requirements.txt + -r requirements-test.txt +platform = + {[common]platforms} +allowlist_externals = + rm + true + {env:OPEN_CMD} + +[testenv:dep] +description = Check package dependency problems +commands = pipdeptree -w fail +deps = + -r requirements.txt + pipdeptree + +[testenv:py{37,38,39,310,311}-type-{lin,mac,win}] +description = Run static type checking with pyright +labels = + type +deps = + -r requirements.txt + pyright +commands = pyright --venvpath {toxworkdir}{/}{envname} {posargs:pg_backup_api} +platform = + {[common]platforms} + +[flake8] +max-line-length = 79 + +[coverage:run] +omit = + pg_backup_api/app.py + pg_backup_api/tests/*