From 0027b03425bc8e68cc8dfd9c9bf6b8ceb94fc535 Mon Sep 17 00:00:00 2001 From: tttuan Date: Thu, 31 Oct 2019 15:52:16 -0700 Subject: [PATCH 1/5] cli fix: wait for parser change --- dtran/main.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/dtran/main.py b/dtran/main.py index 1fd0a54..f24edf3 100644 --- a/dtran/main.py +++ b/dtran/main.py @@ -25,13 +25,9 @@ def create_pipeline(ctx, config=None): # Accept user-specified inputs: expect format of --key=value user_inputs = {} for arg in ctx.args: - try: - key, value = arg[2:].split("=") - func_name, attr_name = key.split(".") - user_inputs[(func_name, attr_name)] = value - except ValueError: - print(f"user input: '{arg}' should have format '--FuncName.Attr=value'") - return + key, value = arg[2:].split("=") + func_name, attr_name = key.split(".") + user_inputs[(func_name, attr_name)] = value parser = ConfigParser(user_inputs) parsed_pipeline, parsed_inputs = parser.parse(config) From 5e600639e4b95f1608bbad36fb04a95f94ccc2b9 Mon Sep 17 00:00:00 2001 From: tttuan Date: Mon, 11 Nov 2019 14:40:20 -0800 Subject: [PATCH 2/5] handle user inputs exception --- dtran/main.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/dtran/main.py b/dtran/main.py index f24edf3..1fd0a54 100644 --- a/dtran/main.py +++ b/dtran/main.py @@ -25,9 +25,13 @@ def create_pipeline(ctx, config=None): # Accept user-specified inputs: expect format of --key=value user_inputs = {} for arg in ctx.args: - key, value = arg[2:].split("=") - func_name, attr_name = key.split(".") - user_inputs[(func_name, attr_name)] = value + try: + key, value = arg[2:].split("=") + func_name, attr_name = key.split(".") + user_inputs[(func_name, attr_name)] = value + except ValueError: + print(f"user input: '{arg}' should have format '--FuncName.Attr=value'") + return parser = ConfigParser(user_inputs) parsed_pipeline, parsed_inputs = parser.parse(config) From 5ce1efe854245c6eabe2d5a1609df23533736eb0 Mon Sep 17 00:00:00 2001 From: tttuan Date: Thu, 24 Oct 2019 14:18:33 -0700 Subject: [PATCH 3/5] unit test for cli parametrized check cli unit test for cli using Python Mock ready to rebase --- dtran/main.py | 12 +++++-- test/README.md | 6 ++++ test/cli_unit_test.py | 73 +++++++++++++++++++++++++++++++++++++++++ test/sample_config.json | 33 +++++++++++++++++++ test/sample_config.yml | 24 ++++++++++++++ 5 files changed, 146 insertions(+), 2 deletions(-) create mode 100644 test/README.md create mode 100644 test/cli_unit_test.py create mode 100644 test/sample_config.json create mode 100644 test/sample_config.yml diff --git a/dtran/main.py b/dtran/main.py index 1fd0a54..44a3511 100644 --- a/dtran/main.py +++ b/dtran/main.py @@ -12,9 +12,13 @@ def cli(): ignore_unknown_options=True, allow_extra_args=True, )) -@click.option("--config", help="full path to config") +@click.option("--config", help="Full path to the config.") +@click.option( + "--dryrun", is_flag=True, + help="Only check parsed inputs without actual execution." +) @click.pass_context -def create_pipeline(ctx, config=None): +def create_pipeline(ctx, config=None, dryrun=False): """ Creates a pipeline and execute it based on given config and input(optional). To specify the input to pipeline, use (listed in ascending priority): @@ -36,6 +40,10 @@ def create_pipeline(ctx, config=None): parser = ConfigParser(user_inputs) parsed_pipeline, parsed_inputs = parser.parse(config) + if dryrun: + print(parsed_inputs) + return + # Execute the pipeline parsed_pipeline.exec(parsed_inputs) diff --git a/test/README.md b/test/README.md new file mode 100644 index 0000000..e5e6a48 --- /dev/null +++ b/test/README.md @@ -0,0 +1,6 @@ +## Prereq +Install Pytest in the current container/env + +## Run +Run the following scripts in /ws: +`python -m pytest test/cli_unit_test.py` \ No newline at end of file diff --git a/test/cli_unit_test.py b/test/cli_unit_test.py new file mode 100644 index 0000000..e5568e6 --- /dev/null +++ b/test/cli_unit_test.py @@ -0,0 +1,73 @@ +import pytest +from click.testing import CliRunner +from unittest.mock import patch, call + +from dtran.main import cli + + +def _mock_pipeline_parser(pipeline_mock, parse_mock): + pipeline_mock.exec.side_effect = lambda _: True + mock_parsed_inputs = { + "keep_attr": "keep this value", + "substitute_attr": "substitute this value" + } + + mock_mappings = { + "my_custom_func1": "keep_", + "my_custom_func2": "substitute_" + } + + parse_mock.return_value = ( + pipeline_mock, + mock_parsed_inputs, + mock_mappings + ) + + return pipeline_mock, parse_mock + + +# https://click.palletsprojects.com/en/7.x/testing/ +@pytest.mark.parametrize( + "func_name, attr_name, arg_value, upserted_inputs", + [ + ("", "", "", { + "keep_attr": "keep this value", + "substitute_attr": "substitute this value", + }), ("my_custom_func2", "attr", "some value", { + "keep_attr": "keep this value", + "substitute_attr": "some value" + }), ("my_custom_func2", "other_attr", "some value", { + "keep_attr": "keep this value", + "substitute_attr": "substitute this value", + "substitute_other_attr": "some value" + }), ("my_custom_func4", "attr", "some value", { + "keep_attr": "keep this value", + "substitute_attr": "substitute this value", + }), + ] +) +@patch('dtran.config_parser.ConfigParser.parse') +@patch('dtran.pipeline.Pipeline') +def test_cli(pipeline_mock, parse_mock, func_name, attr_name, arg_value, upserted_inputs): + """ + This function tests cli given 4 scenarios: + 1) User does not specify any inputs + 2) User updates existing inputs + 3) User inserts new valid inputs + 4) User trys to insert invalid inputs + """ + # TODO: how to combine mock context and pytest fixture? + pipeline_mock, parse_mock = _mock_pipeline_parser(pipeline_mock, parse_mock) + runner = CliRunner() + if not func_name or not attr_name: + result = runner.invoke(cli, [ + 'create_pipeline', '--config', 'config/path/config.yml' + ]) + else: + result = runner.invoke(cli, [ + 'create_pipeline', '--config', 'config/path/config.yml', + f'--{func_name}.{attr_name}={arg_value}', + ]) + + assert result.exit_code == 0 + assert pipeline_mock.exec.mock_calls[-1] == call(upserted_inputs) diff --git a/test/sample_config.json b/test/sample_config.json new file mode 100644 index 0000000..0165142 --- /dev/null +++ b/test/sample_config.json @@ -0,0 +1,33 @@ +{ + "version": "1", + "adapters": { + "MyCustomName1": { + "comment": "My First Adapter", + "adapter": "funcs.ReadFunc", + "inputs": { + "repr_file": "./examples/demo/s01_ethiopia_commodity_price.yml", + "resources": "./examples/demo/s01_ethiopia_commodity_price.csv" + } + }, + "MyCustomName2": { + "comment": "My Second Adapter", + "adapter": "funcs.UnitTransFunc", + "inputs": { + "graph": "$.MyCustomName1.data", + "unit_value": "rdf:value", + "unit_label": "eg:unit", + "unit_desired": "$/liter" + } + }, + "MyCustomName3": { + "comment": "My Third Adapter", + "adapter": "funcs.GraphWriteFunc", + "inputs": { + "graph": "$.MyCustomName2.graph", + "main_class": "qb:Observation", + "output_file": "./examples/demo/s01_ethiopia_commodity_price_write.csv", + "mapped_columns": {} + } + } + } +} \ No newline at end of file diff --git a/test/sample_config.yml b/test/sample_config.yml new file mode 100644 index 0000000..7ddcfb2 --- /dev/null +++ b/test/sample_config.yml @@ -0,0 +1,24 @@ +version: "1" +adapters: + MyCustomName1: + comment: My First Adapter + adapter: funcs.ReadFunc + inputs: + repr_file: ./examples/demo/s01_ethiopia_commodity_price.yml + resources: ./examples/demo/s01_ethiopia_commodity_price.csv + MyCustomName2: + comment: My Second Adapter + adapter: funcs.UnitTransFunc + inputs: + graph: $.MyCustomName1.data + unit_value: rdf:value + unit_label: eg:unit + unit_desired: $/liter + MyCustomName3: + comment: My Third Adapter + adapter: funcs.GraphWriteFunc + inputs: + graph: $.MyCustomName2.graph + main_class: qb:Observation + output_file: ./examples/demo/s01_ethiopia_commodity_price_write.csv + mapped_columns: {} \ No newline at end of file From 10d01a1041782ce4eee44ee939aeceb932a1292a Mon Sep 17 00:00:00 2001 From: tttuan Date: Mon, 11 Nov 2019 14:33:32 -0800 Subject: [PATCH 4/5] test cli valid args using mock --- test/cli_unit_test.py | 82 ++++++++++++++++++++----------------------- 1 file changed, 38 insertions(+), 44 deletions(-) diff --git a/test/cli_unit_test.py b/test/cli_unit_test.py index e5568e6..4398d04 100644 --- a/test/cli_unit_test.py +++ b/test/cli_unit_test.py @@ -1,54 +1,20 @@ import pytest from click.testing import CliRunner -from unittest.mock import patch, call +from unittest.mock import patch, call, MagicMock from dtran.main import cli -def _mock_pipeline_parser(pipeline_mock, parse_mock): - pipeline_mock.exec.side_effect = lambda _: True - mock_parsed_inputs = { - "keep_attr": "keep this value", - "substitute_attr": "substitute this value" - } - - mock_mappings = { - "my_custom_func1": "keep_", - "my_custom_func2": "substitute_" - } - - parse_mock.return_value = ( - pipeline_mock, - mock_parsed_inputs, - mock_mappings - ) - - return pipeline_mock, parse_mock - - # https://click.palletsprojects.com/en/7.x/testing/ @pytest.mark.parametrize( - "func_name, attr_name, arg_value, upserted_inputs", + "func_name, attr_name, arg_value", [ - ("", "", "", { - "keep_attr": "keep this value", - "substitute_attr": "substitute this value", - }), ("my_custom_func2", "attr", "some value", { - "keep_attr": "keep this value", - "substitute_attr": "some value" - }), ("my_custom_func2", "other_attr", "some value", { - "keep_attr": "keep this value", - "substitute_attr": "substitute this value", - "substitute_other_attr": "some value" - }), ("my_custom_func4", "attr", "some value", { - "keep_attr": "keep this value", - "substitute_attr": "substitute this value", - }), + ("", "", ""), + ("substitute", "attr", "some value"), ] ) -@patch('dtran.config_parser.ConfigParser.parse') -@patch('dtran.pipeline.Pipeline') -def test_cli(pipeline_mock, parse_mock, func_name, attr_name, arg_value, upserted_inputs): +@patch('dtran.main.ConfigParser', autospec=True) +def test_cli_valid(parser_mock, func_name, attr_name, arg_value): """ This function tests cli given 4 scenarios: 1) User does not specify any inputs @@ -57,17 +23,45 @@ def test_cli(pipeline_mock, parse_mock, func_name, attr_name, arg_value, upserte 4) User trys to insert invalid inputs """ # TODO: how to combine mock context and pytest fixture? - pipeline_mock, parse_mock = _mock_pipeline_parser(pipeline_mock, parse_mock) + pipeline_mock = MagicMock() + mock_parsed_inputs = { + "keep_attr": "keep this value", + "substitute_attr": "substitute this value" + } + + parser_mock.return_value.parse.return_value = ( + pipeline_mock, + mock_parsed_inputs + ) + + mock_user_inputs = {} + runner = CliRunner() if not func_name or not attr_name: result = runner.invoke(cli, [ - 'create_pipeline', '--config', 'config/path/config.yml' + 'create_pipeline', '--config', 'config/path' ]) else: + mock_user_inputs = { + (func_name, attr_name): arg_value + } result = runner.invoke(cli, [ - 'create_pipeline', '--config', 'config/path/config.yml', + 'create_pipeline', '--config', 'config/path', f'--{func_name}.{attr_name}={arg_value}', ]) assert result.exit_code == 0 - assert pipeline_mock.exec.mock_calls[-1] == call(upserted_inputs) + assert parser_mock.mock_calls == [call(mock_user_inputs), call().parse('config/path')] + assert pipeline_mock.exec.mock_calls[-1] == call(mock_parsed_inputs) + + +@pytest.mark.parametrize( + "func_name, attr_name, arg_value", + [ + ("", "", ""), + ("substitute", "attr", "some value"), + ] +) +@patch('dtran.main.ConfigParser', autospec=True) +def test_cli_invalid(parser_mock, func_name, attr_name, arg_value): + pass From ab8558aacf2ed4c3fd13f95b1bdfbbee1e8e9d9f Mon Sep 17 00:00:00 2001 From: tttuan Date: Mon, 11 Nov 2019 16:16:31 -0800 Subject: [PATCH 5/5] parser & cli unit tests --- test/cli_unit_test.py | 48 +++++++++++++++++++++++++++++++++++----- test/parser_unit_test.py | 37 +++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 6 deletions(-) create mode 100644 test/parser_unit_test.py diff --git a/test/cli_unit_test.py b/test/cli_unit_test.py index 4398d04..ee7e488 100644 --- a/test/cli_unit_test.py +++ b/test/cli_unit_test.py @@ -16,13 +16,11 @@ @patch('dtran.main.ConfigParser', autospec=True) def test_cli_valid(parser_mock, func_name, attr_name, arg_value): """ - This function tests cli given 4 scenarios: + This function tests 3 valid scenarios: 1) User does not specify any inputs 2) User updates existing inputs 3) User inserts new valid inputs - 4) User trys to insert invalid inputs """ - # TODO: how to combine mock context and pytest fixture? pipeline_mock = MagicMock() mock_parsed_inputs = { "keep_attr": "keep this value", @@ -58,10 +56,48 @@ def test_cli_valid(parser_mock, func_name, attr_name, arg_value): @pytest.mark.parametrize( "func_name, attr_name, arg_value", [ - ("", "", ""), + ("substitute", "attr", "some value"), + ] +) +def test_cli_invalid(func_name, attr_name, arg_value): + """ + This function tests invalid cli input scenario. + """ + runner = CliRunner() + arg = f'--{func_name}//{attr_name}={arg_value}' + result = runner.invoke(cli, [ + 'create_pipeline', '--config', 'config/path', arg + ]) + + assert result.exit_code == 0 + assert f"user input: '{arg}' should have format '--FuncName.Attr=value'" in result.output + + +@pytest.mark.parametrize( + "func_name, attr_name, arg_value", + [ ("substitute", "attr", "some value"), ] ) @patch('dtran.main.ConfigParser', autospec=True) -def test_cli_invalid(parser_mock, func_name, attr_name, arg_value): - pass +def test_cli_dryrun(parser_mock, func_name, attr_name, arg_value): + pipeline_mock = MagicMock() + mock_parsed_inputs = { + "keep_attr": "keep this value", + "substitute_attr": "substitute this value" + } + + parser_mock.return_value.parse.return_value = ( + pipeline_mock, + mock_parsed_inputs + ) + + runner = CliRunner() + result = runner.invoke(cli, [ + 'create_pipeline', '--config', 'config/path', + f'--{func_name}.{attr_name}={arg_value}', '--dryrun' + ]) + + for parsed_key, parsed_value in mock_parsed_inputs.items(): + assert parsed_key in result.output + assert parsed_value in result.output diff --git a/test/parser_unit_test.py b/test/parser_unit_test.py new file mode 100644 index 0000000..b5206a2 --- /dev/null +++ b/test/parser_unit_test.py @@ -0,0 +1,37 @@ +import pytest +from dtran.config_parser import ConfigParser +from unittest.mock import patch, call, MagicMock +from funcs import ReadFunc, UnitTransFunc, GraphWriteFunc + + +@pytest.mark.parametrize("config_path", [ + "test/sample_config.json", "test/sample_config.yml" +]) +@patch('dtran.config_parser.Pipeline') +def test_config_parser_no_user_inputs(pipeline_mock, config_path): + pipeline_mock.return_value = MagicMock() + + parser = ConfigParser({}) + parsed_pipeline, parsed_inputs = parser.parse(config_path) + assert "$/liter" in parsed_inputs.values() + assert pipeline_mock.mock_calls[-1] == call( + [ReadFunc, UnitTransFunc, GraphWriteFunc], + [(['unit_trans', 1, 'graph'], ['read_func', 1, 'data']), (['graph_write_func', 1, 'graph'], ['unit_trans', 1, 'graph'])] + ) + + +@pytest.mark.parametrize("config_path", [ + "test/sample_config.json", "test/sample_config.yml" +]) +@patch('dtran.config_parser.Pipeline') +def test_config_parser_with_user_inputs(pipeline_mock, config_path): + pipeline_mock.return_value = MagicMock() + + parser = ConfigParser({("MyCustomName2", "unit_desired"): "$/oz"}) + parsed_pipeline, parsed_inputs = parser.parse(config_path) + assert "$/oz" in parsed_inputs.values() + assert pipeline_mock.mock_calls[-1] == call( + [ReadFunc, UnitTransFunc, GraphWriteFunc], + [(['unit_trans', 1, 'graph'], ['read_func', 1, 'data']), + (['graph_write_func', 1, 'graph'], ['unit_trans', 1, 'graph'])] + )