From 1b1e00279c22b31069e3096a467ed5e8b224f514 Mon Sep 17 00:00:00 2001 From: Mackenzie Grimes Date: Thu, 22 Jun 2023 11:57:38 -0600 Subject: [PATCH 1/5] replace mocker patches with pytest.monkeypatch fixture --- python/idsse_common/test/test_config.py | 26 ++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/python/idsse_common/test/test_config.py b/python/idsse_common/test/test_config.py index e354ef14..68242440 100644 --- a/python/idsse_common/test/test_config.py +++ b/python/idsse_common/test/test_config.py @@ -10,6 +10,8 @@ # -------------------------------------------------------------------------------- import json import pytest +from pytest import MonkeyPatch +from unittest.mock import Mock, mock_open from idsse.common.config import Config @@ -96,58 +98,60 @@ def __init__(self, config: dict) -> None: assert config.a_key == 'value for a' -def test_load_from_file(mocker): +def test_load_from_file(monkeypatch: MonkeyPatch): class WithoutKeyConfig(Config): """Config class that doesn't use a key to find config data""" def __init__(self, config: dict) -> None: self.y_this_is_a_key = None super().__init__(config, '') - mocker.patch('glob.glob', return_value=['filename']) + monkeypatch.setattr('glob.glob', Mock(return_value=['filename'])) read_data = json.dumps({"y_this_is_a_key": "value found in file"}) - mocker.patch('builtins.open', mocker.mock_open(read_data=read_data)) + monkeypatch.setattr('builtins.open', mock_open(read_data=read_data)) config = WithoutKeyConfig('path/to/file') assert config.y_this_is_a_key == "value found in file" -def test_load_from_files_with_out_key(mocker): +def test_load_from_files_with_out_key(monkeypatch: MonkeyPatch): class WithoutKeyConfig(Config): """Config class that doesn't use a key to find config data""" def __init__(self, config: dict) -> None: self.y_this_is_a_key = None super().__init__(config, []) - mocker.patch('glob.glob', return_value=['filename1', 'filename2']) + monkeypatch.setattr('glob.glob', Mock(return_value=['filename1', 'filename2'])) read_data = [json.dumps({"y_this_is_a_key": "value found in file1"}), json.dumps({"y_this_is_a_key": "value found in file2"})] - mock_files = mocker.patch('builtins.open', mocker.mock_open(read_data=read_data[0])) - mock_files.side_effect = (mocker.mock_open(read_data=data).return_value for data in read_data) + mock_files = Mock(side_effect=(mock_open(read_data=data).return_value for data in read_data)) + monkeypatch.setattr('builtins.open', mock_files) config = WithoutKeyConfig('path/to/dir') assert config.y_this_is_a_key == "value found in file1" assert config.next.y_this_is_a_key == "value found in file2" + assert mock_files.call_count == 2 -def test_load_from_files_with_key(mocker): +def test_load_from_files_with_key(monkeypatch: MonkeyPatch): class WithoutKeyConfig(Config): """Config class that doesn't use a key to find config data""" def __init__(self, config: dict) -> None: self.y_this_is_a_key = None super().__init__(config, 'config_key') - mocker.patch('glob.glob', return_value=['filename1', 'filename2']) + monkeypatch.setattr('glob.glob', Mock(return_value=['filename1', 'filename2'])) read_data = [json.dumps({"config_key": {"y_this_is_a_key": "value found in file1"}}), json.dumps({"config_key": {"y_this_is_a_key": "value found in file2"}})] - mock_files = mocker.patch('builtins.open', mocker.mock_open(read_data=read_data[0])) - mock_files.side_effect = (mocker.mock_open(read_data=data).return_value for data in read_data) + mock_files = Mock(side_effect=(mock_open(read_data=data).return_value for data in read_data)) + monkeypatch.setattr('builtins.open', mock_files) config = WithoutKeyConfig('path/to/dir') assert config.y_this_is_a_key == "value found in file1" assert config.next.y_this_is_a_key == "value found in file2" + assert mock_files.call_count == 2 From 23053a00a94e85614643fb84bc7cf8329f3184ef Mon Sep 17 00:00:00 2001 From: Mackenzie Grimes Date: Thu, 22 Jun 2023 13:05:26 -0600 Subject: [PATCH 2/5] add unit tests for config.py --- python/idsse_common/idsse/common/config.py | 9 ++-- python/idsse_common/test/test_config.py | 55 ++++++++++++++++++++-- 2 files changed, 54 insertions(+), 10 deletions(-) diff --git a/python/idsse_common/idsse/common/config.py b/python/idsse_common/idsse/common/config.py index 1c2de7fc..cafbade2 100644 --- a/python/idsse_common/idsse/common/config.py +++ b/python/idsse_common/idsse/common/config.py @@ -22,7 +22,7 @@ class Config: """Configuration data class""" def __init__(self, - config: Union[dict, str], + config: Union[dict, list[dict], str], keys: Union[list, str] = None, recursive: bool = False, ignore_missing: bool = False) -> None: @@ -116,7 +116,7 @@ def _from_config_dict(self, config_dict: dict, keys: str) -> Self: # update the instance dictionary to hold all configuration attributes self.__dict__.update(config_dict) - def _from_config_dicts(self, config_dicts, keys: str) -> Self: + def _from_config_dicts(self, config_dicts: list[dict], keys: str) -> Self: self._from_config_dict(config_dicts[0], keys) for config_dict in config_dicts[1:]: # if inherited class takes only one argument @@ -127,7 +127,7 @@ def _from_config_dicts(self, config_dicts, keys: str) -> Self: self._next._previous = self # pylint: disable=protected-access -def _example(): +def _example(): # pragma: no cover class NameAsKeyConfig(Config): """Testing config class the uses class name as key""" def __init__(self, config: Union[dict, str]) -> None: @@ -197,7 +197,6 @@ def get_config_dict(key: Union[list, str]) -> dict: logging.info('Metaphor:', config.metaphor) -if __name__ == '__main__': +if __name__ == '__main__': # pragma: no cover import random - _example() diff --git a/python/idsse_common/test/test_config.py b/python/idsse_common/test/test_config.py index 68242440..208c05d5 100644 --- a/python/idsse_common/test/test_config.py +++ b/python/idsse_common/test/test_config.py @@ -21,6 +21,7 @@ def test_load_from_dict_without_key(): class WithoutKeyConfig(Config): """Config class that doesn't use a key to find config data""" + def __init__(self, config: dict) -> None: self.a_key = None super().__init__(config, '') @@ -32,6 +33,7 @@ def __init__(self, config: dict) -> None: def test_load_from_dict_as_string_without_key(): class WithoutKeyConfig(Config): """Config class that doesn't use a key to find config data""" + def __init__(self, config: dict) -> None: self.some_key = None super().__init__(config, '') @@ -43,6 +45,7 @@ def __init__(self, config: dict) -> None: def test_load_from_dict_with_name_key(): class NameAsKeyConfig(Config): """Config class that class name as the key to find config data""" + def __init__(self, config: dict) -> None: self.best_key = None super().__init__(config, None) @@ -54,17 +57,20 @@ def __init__(self, config: dict) -> None: def test_load_from_dict_require_string_key(): class RequireKeyConfig(Config): """Config class that doesn't use a key to find config data""" + def __init__(self, config: dict, key: str) -> None: self.some_key = None super().__init__(config, key) - config = RequireKeyConfig('{"custom_key": {"some_key": "value found"}}', 'custom_key') + config = RequireKeyConfig( + '{"custom_key": {"some_key": "value found"}}', 'custom_key') assert config.some_key == 'value found' def test_load_from_dict_require_list_key(): class RequireKeyConfig(Config): """Config class that doesn't use a key to find config data""" + def __init__(self, config: dict, key: list) -> None: self.some_key = None super().__init__(config, key) @@ -78,6 +84,7 @@ def __init__(self, config: dict, key: list) -> None: def test_load_with_missing_attribute_should_fail(): class WithoutKeyConfig(Config): """Config class that doesn't use a key to find config data""" + def __init__(self, config: dict) -> None: self.a_key = None super().__init__(config, '') @@ -86,9 +93,40 @@ def __init__(self, config: dict) -> None: WithoutKeyConfig({'diff_key': 'value found'}) +def test_config_str_with_no_files_raises_error(monkeypatch: MonkeyPatch): + class WithoutKeyConfig(Config): + """Config class that doesn't use a key to find config data""" + + def __init__(self, config: str) -> None: + self.a_key = None + super().__init__(config, '') + + monkeypatch.setattr('glob.glob', Mock(return_value=[])) + + with pytest.raises(FileNotFoundError): + WithoutKeyConfig('wont_be_found') + + +def test_config_list_of_dicts_succeeds(): + class WithoutKeyConfig(Config): + """Config class that doesn't use a key to find config data""" + + def __init__(self, config: dict) -> None: + self.a_key = None + self.b_key = None + super().__init__(config, '', ignore_missing=True) + + config = WithoutKeyConfig( + [{'a_key': 'value for a'}, {'b_key': 'value for b'}]) + + assert config.a_key == 'value for a' + assert config.next.b_key == 'value for b' + + def test_load_with_ignore_missing_attribute(): class WithoutKeyConfig(Config): """Config class that doesn't use a key to find config data""" + def __init__(self, config: dict) -> None: self.a_key = None self.b_key = None @@ -101,6 +139,7 @@ def __init__(self, config: dict) -> None: def test_load_from_file(monkeypatch: MonkeyPatch): class WithoutKeyConfig(Config): """Config class that doesn't use a key to find config data""" + def __init__(self, config: dict) -> None: self.y_this_is_a_key = None super().__init__(config, '') @@ -118,15 +157,18 @@ def __init__(self, config: dict) -> None: def test_load_from_files_with_out_key(monkeypatch: MonkeyPatch): class WithoutKeyConfig(Config): """Config class that doesn't use a key to find config data""" + def __init__(self, config: dict) -> None: self.y_this_is_a_key = None super().__init__(config, []) - monkeypatch.setattr('glob.glob', Mock(return_value=['filename1', 'filename2'])) + monkeypatch.setattr('glob.glob', Mock( + return_value=['filename1', 'filename2'])) read_data = [json.dumps({"y_this_is_a_key": "value found in file1"}), json.dumps({"y_this_is_a_key": "value found in file2"})] - mock_files = Mock(side_effect=(mock_open(read_data=data).return_value for data in read_data)) + mock_files = Mock(side_effect=( + mock_open(read_data=data).return_value for data in read_data)) monkeypatch.setattr('builtins.open', mock_files) config = WithoutKeyConfig('path/to/dir') @@ -139,15 +181,18 @@ def __init__(self, config: dict) -> None: def test_load_from_files_with_key(monkeypatch: MonkeyPatch): class WithoutKeyConfig(Config): """Config class that doesn't use a key to find config data""" + def __init__(self, config: dict) -> None: self.y_this_is_a_key = None super().__init__(config, 'config_key') - monkeypatch.setattr('glob.glob', Mock(return_value=['filename1', 'filename2'])) + monkeypatch.setattr('glob.glob', Mock( + return_value=['filename1', 'filename2'])) read_data = [json.dumps({"config_key": {"y_this_is_a_key": "value found in file1"}}), json.dumps({"config_key": {"y_this_is_a_key": "value found in file2"}})] - mock_files = Mock(side_effect=(mock_open(read_data=data).return_value for data in read_data)) + mock_files = Mock(side_effect=( + mock_open(read_data=data).return_value for data in read_data)) monkeypatch.setattr('builtins.open', mock_files) config = WithoutKeyConfig('path/to/dir') From 271d8aa51a55a097776ae8994028d5132caf03e1 Mon Sep 17 00:00:00 2001 From: Mackenzie Grimes Date: Thu, 22 Jun 2023 15:03:18 -0600 Subject: [PATCH 3/5] correct list type hints to List --- python/idsse_common/idsse/common/config.py | 81 +--------------------- 1 file changed, 3 insertions(+), 78 deletions(-) diff --git a/python/idsse_common/idsse/common/config.py b/python/idsse_common/idsse/common/config.py index cafbade2..a0a4f2a5 100644 --- a/python/idsse_common/idsse/common/config.py +++ b/python/idsse_common/idsse/common/config.py @@ -13,7 +13,7 @@ import json import logging from inspect import signature -from typing import Self, Union +from typing import Self, Union, List logger = logging.getLogger(__name__) @@ -22,7 +22,7 @@ class Config: """Configuration data class""" def __init__(self, - config: Union[dict, list[dict], str], + config: Union[dict, List[dict], str], keys: Union[list, str] = None, recursive: bool = False, ignore_missing: bool = False) -> None: @@ -116,7 +116,7 @@ def _from_config_dict(self, config_dict: dict, keys: str) -> Self: # update the instance dictionary to hold all configuration attributes self.__dict__.update(config_dict) - def _from_config_dicts(self, config_dicts: list[dict], keys: str) -> Self: + def _from_config_dicts(self, config_dicts: List[dict], keys: str) -> Self: self._from_config_dict(config_dicts[0], keys) for config_dict in config_dicts[1:]: # if inherited class takes only one argument @@ -125,78 +125,3 @@ def _from_config_dicts(self, config_dicts: list[dict], keys: str) -> Self: else: self._next = type(self)(config_dict, keys) self._next._previous = self # pylint: disable=protected-access - - -def _example(): # pragma: no cover - class NameAsKeyConfig(Config): - """Testing config class the uses class name as key""" - def __init__(self, config: Union[dict, str]) -> None: - """To create a config that uses it's class name when look for nested config, - pass None to the super.__init()""" - self.idiom = None - self.metaphor = None - super().__init__(config, None) - - class WithoutKeyConfig(Config): - """Testing config class the uses no key""" - def __init__(self, config: Union[dict, str]) -> None: - """To create a config that does NOT look for config nested under a key, - pass an empty string to the super.__init()""" - self.idiom = None - self.metaphor = None - super().__init__(config, '') - - class RequiresKeyConfig(Config): - """Testing config class that requires key to be provided""" - def __init__(self, config: Union[dict, str], key: Union[list, str]) -> None: - """To create a config that requires a user provided name when look for nested config, - pass a key as string or list of string to the super.__init()""" - self.idiom = None - self.metaphor = None - super().__init__(config, key) - - def get_config_dict(key: Union[list, str]) -> dict: - idioms = ['Out of hand', - 'See eye to eye', - 'Under the weather', - 'Cut the mustard'] - metaphors = ['blanket of stars', - 'weighing on my mind', - 'were a glaring light', - 'floated down the river'] - if key is None: - return {'idiom': random.choice(idioms), - 'metaphor': random.choice(metaphors)} - if isinstance(key, str): - key = [key] - config_dict = {'idiom': random.choice(idioms), - 'metaphor': random.choice(metaphors)} - for k in reversed(key): - config_dict = {k: config_dict} - return config_dict - - # example of config that uses class name to identify relevant block of data - config_dict = get_config_dict('NameAsKeyConfig') - logging.info(config_dict) - config = NameAsKeyConfig(config_dict) - logging.info('Idiom:', config.idiom) - logging.info('Metaphor:', config.metaphor) - - # example of config with block of data at top level - config_dict = get_config_dict(None) - logging.info(config_dict) - config = WithoutKeyConfig(config_dict) - logging.info('Idiom:', config.idiom) - logging.info('Metaphor:', config.metaphor) - - # example of config with relevant block of data nested - config_dict = get_config_dict('NestKey') - logging.info(config_dict) - config = RequiresKeyConfig(config_dict, 'NestKey') - logging.info('Idiom:', config.idiom) - logging.info('Metaphor:', config.metaphor) - - -if __name__ == '__main__': # pragma: no cover - import random - _example() From a72478637ded7771857bd0126acbaed6c7ff51ad Mon Sep 17 00:00:00 2001 From: Mackenzie Grimes Date: Thu, 22 Jun 2023 15:19:41 -0600 Subject: [PATCH 4/5] rename couchdb/test_driver.py and rabbitmq/test_driver.py to avoid pytest conflict errors --- couchdb/test/run_test.sh | 2 +- couchdb/test/{test_driver.py => test_couchdb_driver.py} | 0 python/idsse_common/README.md | 4 ++-- rabbitmq/test/run_test.sh | 2 +- rabbitmq/test/{test_driver.py => test_rabbitmq_driver.py} | 0 5 files changed, 4 insertions(+), 4 deletions(-) rename couchdb/test/{test_driver.py => test_couchdb_driver.py} (100%) rename rabbitmq/test/{test_driver.py => test_rabbitmq_driver.py} (100%) diff --git a/couchdb/test/run_test.sh b/couchdb/test/run_test.sh index f1a9cd4e..20ccedfd 100755 --- a/couchdb/test/run_test.sh +++ b/couchdb/test/run_test.sh @@ -4,4 +4,4 @@ # [1] = couchdb server host name # [2] = couchdb client/server username # [3] = couchdb client/server password -python ./test_driver.py "couchtest" "idss" "idss" +python ./test_couchdb_driver.py "couchtest" "idss" "idss" diff --git a/couchdb/test/test_driver.py b/couchdb/test/test_couchdb_driver.py similarity index 100% rename from couchdb/test/test_driver.py rename to couchdb/test/test_couchdb_driver.py diff --git a/python/idsse_common/README.md b/python/idsse_common/README.md index 06f1c666..87c1d95d 100644 --- a/python/idsse_common/README.md +++ b/python/idsse_common/README.md @@ -72,7 +72,7 @@ Once installed elements from the package can be imported directly into code. For ### Python After installing the project's dependencies, make sure you have the [pytest-cov](https://pytest-cov.readthedocs.io/en/latest/config.html?highlight=missing#reference) plugin installed. -Run pytest coverage with the following CLI command. Note: the path argument can be removed to run all tests in the project. +Run pytest coverage with the following CLI command. Note: the path argument `=python` can be removed to run all tests in the project. ``` -pytest --cov=python/idsse_common python/idsse_common/test --cov-report=term-missing +pytest --cov=python --cov-report=term-missing ``` diff --git a/rabbitmq/test/run_test.sh b/rabbitmq/test/run_test.sh index b155cce5..5723dbfe 100755 --- a/rabbitmq/test/run_test.sh +++ b/rabbitmq/test/run_test.sh @@ -4,4 +4,4 @@ # [1] = rmq server host name # [2] = rmq client/server username # [3] = rmq client/server password -python ./test_driver.py "rmqtest" "idss" "password" +python ./test_rabbitmq_driver.py "rmqtest" "idss" "password" diff --git a/rabbitmq/test/test_driver.py b/rabbitmq/test/test_rabbitmq_driver.py similarity index 100% rename from rabbitmq/test/test_driver.py rename to rabbitmq/test/test_rabbitmq_driver.py From 7357d35d9e2e5f22c604b0a44e14d349073b254f Mon Sep 17 00:00:00 2001 From: Mackenzie Grimes Date: Fri, 23 Jun 2023 16:37:08 -0600 Subject: [PATCH 5/5] add python library install steps to README, add develop dependencies to setup.py --- .gitignore | 5 +++- python/idsse_common/README.md | 49 +++++++++++++++++++++++++---------- python/idsse_common/setup.py | 7 +++++ 3 files changed, 47 insertions(+), 14 deletions(-) diff --git a/.gitignore b/.gitignore index 52b88a8e..70333773 100644 --- a/.gitignore +++ b/.gitignore @@ -40,4 +40,7 @@ local.properties __pycache__ .venv .pytest_cache -.coverage* \ No newline at end of file +.coverage* +build/ +dist/ +*.egg* \ No newline at end of file diff --git a/python/idsse_common/README.md b/python/idsse_common/README.md index 87c1d95d..d3606044 100644 --- a/python/idsse_common/README.md +++ b/python/idsse_common/README.md @@ -53,26 +53,49 @@ this. The initial packaging for the project uses [setuptools](https://setuptools ## Build and Install -To manually install the package on your local instance pull the idss-engine-common from the repository and navigate into idsse_common +For any of the steps below, first clone this idss-engine-commons repository locally from GitHub. -From the directory `/idss-engine-common/python/idsse_common`: +### Building this library -`$ python3 setup.py install` +1. `cd` into `/python/idsse_common` +1. Build the project + ``` + $ python3 setup.py install + ``` -**NOTE** Python 3.11+ is required to install and use this package, it won't work on earlier versions of python +**NOTE** Python 3.11+ is required to install and use this package. Install should fail for earlier versions -## Using the package +### Importing this library into other projects -Once installed elements from the package can be imported directly into code. For example: +1. `cd` into the project's directory (where you want to use this library) +1. Make sure you're command line session has a `virtualenv` created and activated + 1. If you haven't done this, run `python3 -m venv .venv` + 1. Activate the virtualenv with `source .venv/bin/activate` +1. Use `pip -e` to install the library using the local path to the cloned repo's `setup.py`. E.g. + ``` + pip install -e /Users/my_user_name/idss-engine-commons/python/idsse_common + ``` ---- -> from idsse.common.path_builder import PathBuilder +On success, you should see a message from pip like `Successfully built idsse-1.x` -## Running tests -### Python -After installing the project's dependencies, make sure you have the [pytest-cov](https://pytest-cov.readthedocs.io/en/latest/config.html?highlight=missing#reference) plugin installed. -Run pytest coverage with the following CLI command. Note: the path argument `=python` can be removed to run all tests in the project. +## Using the package + +Once installed, elements from the package can be imported directly into code. For example: + ``` -pytest --cov=python --cov-report=term-missing +from idsse.common.path_builder import PathBuilder +my_path_builder = PathBuilder() ``` + +## Running tests + +1. Install this library's dependencies as detailed above in [Building this library](#building-this-library) +1. Install [pytest](https://docs.pytest.org/en/latest/index.html) and the [pytest-cov](https://pytest-cov.readthedocs.io/en/latest/config.html?highlight=missing#reference) plugin if you don't have it + ``` + pip install pytest pytest-cov + ``` +1. Generate a pytest coverage report with the following command + ``` + pytest --cov --cov-report=term-missing + ``` diff --git a/python/idsse_common/setup.py b/python/idsse_common/setup.py index 6e561913..58cb1c87 100644 --- a/python/idsse_common/setup.py +++ b/python/idsse_common/setup.py @@ -8,6 +8,7 @@ author='WIDS', author_email='@noaa.gov', license='MIT', + python_requires=">3.11", packages=['idsse.common'], # packages=['idsse', 'idsse.common'], # packages=find_packages(exclude=['contrib', 'docs', 'tests*']), @@ -16,4 +17,10 @@ 'pint', 'importlib_metadata', ], + extras_require={ + 'develop': [ + 'pytest', + 'pytest-cov', + ] + }, zip_safe=False)