From 42d8740ae42a0f89f4f72252ef6ba014dc55eeb5 Mon Sep 17 00:00:00 2001 From: Jorge Alvarez Jarreta Date: Mon, 7 Oct 2024 17:25:21 +0100 Subject: [PATCH 1/2] update database.factory.main() function to test all its contents --- pyproject.toml | 1 - .../ensembl/io/genomio/database/factory.py | 36 +++++-- src/python/tests/database/test_factory.py | 95 +++++++++++++++++-- 3 files changed, 115 insertions(+), 17 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 3c9d24488..888f9f84e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -228,7 +228,6 @@ exclude_lines = [ "raise NotImplementedError", # Do not complain if non-runnable code is not run "if __name__ == .__main__.:", - "def main", # Do not complain about abstract methods, they are not run "@(abc\\.)?abstractmethod", ] diff --git a/src/python/ensembl/io/genomio/database/factory.py b/src/python/ensembl/io/genomio/database/factory.py index fa114a8f5..33b6ca0ec 100644 --- a/src/python/ensembl/io/genomio/database/factory.py +++ b/src/python/ensembl/io/genomio/database/factory.py @@ -16,10 +16,11 @@ __all__ = ["format_db_data", "get_core_dbs_metadata"] +import argparse import json -from pathlib import Path -from typing import Dict, List, Optional import logging +from pathlib import Path +import sys from sqlalchemy.engine import URL @@ -29,7 +30,7 @@ from .dbconnection_lite import DBConnectionLite -def format_db_data(server_url: URL, dbs: List[str], brc_mode: bool = False) -> List[Dict]: +def format_db_data(server_url: URL, dbs: list[str], brc_mode: bool = False) -> list[dict]: """Returns a metadata list from the given databases on a server. Args: @@ -87,13 +88,14 @@ def format_db_data(server_url: URL, dbs: List[str], brc_mode: bool = False) -> L def get_core_dbs_metadata( server_url: URL, + *, prefix: str = "", - build: Optional[int] = None, - version: Optional[int] = None, + build: int | None = None, + version: int | None = None, db_regex: str = "", - db_list: Optional[Path] = None, + db_list: Path | None = None, brc_mode: bool = False, -) -> List[Dict]: +) -> list[dict]: """Returns all the metadata fetched for the selected core databases. Args: @@ -123,8 +125,12 @@ def get_core_dbs_metadata( return format_db_data(server_url, databases, brc_mode) -def main() -> None: - """Main script entry-point.""" +def parse_args(arg_list: list[str] | None) -> argparse.Namespace: + """TODO + + Args: + arg_list: TODO + """ parser = ArgumentParser(description=__doc__) parser.add_server_arguments() # Add filter arguments @@ -140,7 +146,17 @@ def main() -> None: help="Enable BRC mode, i.e. use organism_abbrev for species, component for division", ) parser.add_log_arguments() - args = parser.parse_args() + return parser.parse_args(arg_list) + + +def main(arg_list: list[str] | None = None) -> None: + """Main script entry-point. + + Args: + arg_list: TODO + + """ + args = parse_args(arg_list) init_logging_with_args(args) databases_data = get_core_dbs_metadata( diff --git a/src/python/tests/database/test_factory.py b/src/python/tests/database/test_factory.py index 7ba8c8e7f..900d2fb1e 100644 --- a/src/python/tests/database/test_factory.py +++ b/src/python/tests/database/test_factory.py @@ -20,13 +20,13 @@ """ from pathlib import Path -from typing import Dict, List, Optional +from typing import Generator from unittest.mock import call, Mock, patch from deepdiff import DeepDiff import pytest from pytest import param -from sqlalchemy.engine import URL +from sqlalchemy.engine import make_url, URL from ensembl.io.genomio.database import factory @@ -115,7 +115,7 @@ ], ) def test_format_db_data( - mock_dbconn: Mock, server_url: URL, dbs: List[str], brc_mode: bool, skip_keys: bool, output: List[Dict] + mock_dbconn: Mock, server_url: URL, dbs: list[str], brc_mode: bool, skip_keys: bool, output: list[dict] ) -> None: """Tests the `factory.format_db_data()` function. @@ -128,7 +128,7 @@ def test_format_db_data( output: Expected list of dictionaries with metadata per database. """ - def _get_meta_value(meta_key: str) -> Optional[str]: + def _get_meta_value(meta_key: str) -> str | None: """Return empty string if "species.division" is requested in BRC mode, "Metazoa" otherwise.""" if (meta_key == "species.division") and brc_mode: return "" @@ -169,7 +169,7 @@ def test_get_core_dbs_metadata( mock_format_db_data: Mock, data_dir: Path, use_db_file: bool, - output: List[Dict], + output: list[dict], ) -> None: """Tests the `factory.get_core_dbs_metadata()` function. @@ -181,7 +181,7 @@ def test_get_core_dbs_metadata( output: Expected list of dictionaries with some metadata for each selected database. """ - def _format_db_data(server_url: URL, dbs: List[str], brc_mode: bool = False) -> List[Dict]: + def _format_db_data(server_url: URL, dbs: list[str], brc_mode: bool = False) -> list[dict]: """Returns metadata from a list of databases.""" _ = (server_url, brc_mode) # Unused by mock return [{"database": db, "species": "dog"} for db in dbs] @@ -196,3 +196,86 @@ def _format_db_data(server_url: URL, dbs: List[str], brc_mode: bool = False) -> db_list = data_dir / "databases.txt" if use_db_file else None result = factory.get_core_dbs_metadata(URL.create("mysql"), db_list=db_list) assert not DeepDiff(result, output) + + +@pytest.mark.parametrize( + "arg_list, expected", + [ + param( + ["--host", "localhost", "--port", "42", "--user", "me"], + { + "host": "localhost", + "port": 42, + "user": "me", + "password": None, + "url": make_url("mysql://me@localhost:42"), + "prefix": "", + "build": None, + "version": None, + "db_regex": "", + "db_list": None, + "brc_mode": False, + "log_level": "WARNING", + }, + id="Default args", + ), + param( + [ + "--host", "localhost", "--port", "42", "--user", "me", "--password", "secret", + "--prefix", "pre", "--build", "70", "--version", "114", "--db_regex", "%_core_%", + "--db_list", __file__, "--brc_mode", + ], + { + "host": "localhost", + "port": 42, + "user": "me", + "password": "secret", + "url": make_url("mysql://me:secret@localhost:42"), + "prefix": "pre", + "build": 70, + "version": 114, + "db_regex": "%_core_%", + "db_list": __file__, + "brc_mode": True, + "log_level": "WARNING", + }, + id="New arg values", + ), + ], +) +def test_parse_args(arg_list: list[str], expected: dict) -> None: + """Tests the `factory.parse_args()` function.""" + args = factory.parse_args(arg_list) + if args.db_list: + # DeepDiff is not able to compare two objects of Path type, so convert it to string + setattr(args, "db_list", str(args.db_list)) + assert not DeepDiff(vars(args), expected) + + +@pytest.mark.parametrize( + "arg_list, server_url, stdout", + [ + ( + ["--host", "localhost", "--port", "42", "--user", "me"], + make_url("mysql://me@localhost:42"), + '{\n "test": "output"\n}\n', + ), + ], +) +@patch("ensembl.io.genomio.database.factory.get_core_dbs_metadata") +def test_main( + mock_get_core_dbs_metadata: Mock, capsys: Generator, arg_list: list[str], server_url: URL, stdout: str +) -> None: + """Tests the `factory.main()` function (entry point). + + Fixtures: capsys + """ + mock_get_core_dbs_metadata.return_value = {"test": "output"} + factory.main(arg_list) + # Check that we have called the mocked function once with the expected parameters + mock_get_core_dbs_metadata.assert_called_once_with( + server_url=server_url, prefix="", build=None, version=None, db_regex="", db_list=None, brc_mode=False + ) + # Check that the stdout is as expected + captured = capsys.readouterr() + assert captured.out == stdout From 6ad94c292f9f01afe40abca27cfc3272fc76d08a Mon Sep 17 00:00:00 2001 From: Jorge Alvarez Jarreta Date: Tue, 8 Oct 2024 11:32:32 +0100 Subject: [PATCH 2/2] make black, pylint and mypy happy --- .../ensembl/io/genomio/database/factory.py | 1 - src/python/tests/database/test_factory.py | 30 +++++++++++++++---- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/python/ensembl/io/genomio/database/factory.py b/src/python/ensembl/io/genomio/database/factory.py index 33b6ca0ec..9d47501f2 100644 --- a/src/python/ensembl/io/genomio/database/factory.py +++ b/src/python/ensembl/io/genomio/database/factory.py @@ -20,7 +20,6 @@ import json import logging from pathlib import Path -import sys from sqlalchemy.engine import URL diff --git a/src/python/tests/database/test_factory.py b/src/python/tests/database/test_factory.py index 900d2fb1e..f9adbf0d1 100644 --- a/src/python/tests/database/test_factory.py +++ b/src/python/tests/database/test_factory.py @@ -20,12 +20,12 @@ """ from pathlib import Path -from typing import Generator from unittest.mock import call, Mock, patch from deepdiff import DeepDiff import pytest from pytest import param +from _pytest.capture import CaptureFixture from sqlalchemy.engine import make_url, URL from ensembl.io.genomio.database import factory @@ -221,9 +221,25 @@ def _format_db_data(server_url: URL, dbs: list[str], brc_mode: bool = False) -> ), param( [ - "--host", "localhost", "--port", "42", "--user", "me", "--password", "secret", - "--prefix", "pre", "--build", "70", "--version", "114", "--db_regex", "%_core_%", - "--db_list", __file__, "--brc_mode", + "--host", + "localhost", + "--port", + "42", + "--user", + "me", + "--password", + "secret", + "--prefix", + "pre", + "--build", + "70", + "--version", + "114", + "--db_regex", + "%_core_%", + "--db_list", + __file__, + "--brc_mode", ], { "host": "localhost", @@ -264,7 +280,11 @@ def test_parse_args(arg_list: list[str], expected: dict) -> None: ) @patch("ensembl.io.genomio.database.factory.get_core_dbs_metadata") def test_main( - mock_get_core_dbs_metadata: Mock, capsys: Generator, arg_list: list[str], server_url: URL, stdout: str + mock_get_core_dbs_metadata: Mock, + capsys: CaptureFixture[str], + arg_list: list[str], + server_url: URL, + stdout: str, ) -> None: """Tests the `factory.main()` function (entry point).