From d3212bd367f7520116173ac2e149e97dcbebad70 Mon Sep 17 00:00:00 2001 From: "Ian H. Pittwood" Date: Fri, 21 Jun 2019 09:19:48 -0500 Subject: [PATCH] Use data objects for document generation This change removes the intermediate step of converting the SiteDocumentData object returned by the parser to a dictionary. This change depends on [0], a related change that updates the Jinja2 templates and site processor to use data objects instead of a dictionary. [0] https://review.opendev.org/#/c/661092/ Depends-On: I66ebfeaf5d6ca76b6dee5a2285a74bad8b06b720 Change-Id: I991c9f6590752b36050dc5239d9ea3f3dfe8c5bc --- spyglass_plugin_xls/cli.py | 5 +- .../shared/templates/site-definition.yaml.j2 | 4 +- tests/unit/test_cli.py | 115 +++++------------- tox.ini | 4 +- 4 files changed, 37 insertions(+), 91 deletions(-) diff --git a/spyglass_plugin_xls/cli.py b/spyglass_plugin_xls/cli.py index 7076ce9..9c74fa9 100644 --- a/spyglass_plugin_xls/cli.py +++ b/spyglass_plugin_xls/cli.py @@ -150,7 +150,7 @@ def generate_manifests_and_intermediary(*args, **kwargs): process_input_ob = \ spyglass.cli.intermediary_processor('excel', **kwargs) LOG.info("Generate intermediary yaml") - intermediary_yaml = process_input_ob.generate_intermediary_yaml() + site_data = process_input_ob.generate_intermediary_yaml() if kwargs["generate_intermediary"]: LOG.debug("Dumping intermediary yaml") process_input_ob.dump_intermediary_file(kwargs['intermediary_dir']) @@ -159,6 +159,5 @@ def generate_manifests_and_intermediary(*args, **kwargs): LOG.info("Generating site Manifests") processor_engine = SiteProcessor( - intermediary_yaml.dict_from_class(), kwargs['manifest_dir'], - kwargs['force']) + site_data, kwargs['manifest_dir'], kwargs['force']) processor_engine.render_template(kwargs['template_dir']) diff --git a/tests/shared/templates/site-definition.yaml.j2 b/tests/shared/templates/site-definition.yaml.j2 index d446262..312e8e7 100644 --- a/tests/shared/templates/site-definition.yaml.j2 +++ b/tests/shared/templates/site-definition.yaml.j2 @@ -5,9 +5,9 @@ metadata: layeringDefinition: abstract: false layer: site - name: {{ data['region_name'] }} + name: {{ data.site_info.region_name }} storagePolicy: cleartext data: - site_type: {{ data['site_info']['sitetype'] }} + site_type: {{ data.site_info.sitetype }} ... diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index da03e0f..3dec5e3 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -39,14 +39,8 @@ def test_generate_intermediary(mock_excel_plugin, mock_intermediary_processor): """Test generate_intermediary (intermediary) normal execution""" runner = CliRunner() args = [ - '-x', - EXCEL_FILE_PATH, - '-e', - EXCEL_SPEC_PATH, - '-c', - SITE_CONFIG_PATH, - '-s', - 'airship-seaworthy' + '-x', EXCEL_FILE_PATH, '-e', EXCEL_SPEC_PATH, '-c', SITE_CONFIG_PATH, + '-s', 'airship-seaworthy' ] result = runner.invoke(generate_intermediary, args) assert result.exit_code == 0 @@ -66,16 +60,8 @@ def test_generate_intermediary_intermediary_dir( test_dir_name = 'intermediary_test_dir' test_dir = tmpdir.mkdir(test_dir_name) args = [ - '-x', - EXCEL_FILE_PATH, - '-e', - EXCEL_SPEC_PATH, - '-c', - SITE_CONFIG_PATH, - '-s', - 'airship-seaworthy', - '--intermediary-dir', - test_dir + '-x', EXCEL_FILE_PATH, '-e', EXCEL_SPEC_PATH, '-c', SITE_CONFIG_PATH, + '-s', 'airship-seaworthy', '--intermediary-dir', test_dir ] result = runner.invoke(generate_intermediary, args) assert result.exit_code == 0 @@ -86,28 +72,20 @@ def test_generate_intermediary_intermediary_dir( .assert_called_once_with(test_dir) +@mock.patch.object(SiteProcessor, '__init__', return_value=None) @mock.patch('spyglass.cli.intermediary_processor', autospec=True) @mock.patch('spyglass_plugin_xls.excel.ExcelPlugin', autospec=False) def test_generate_manifests_and_intermediary( - mock_excel_plugin, mock_intermediary_processor): + mock_excel_plugin, mock_intermediary_processor, + mock_site_processor_init): """Test generate_manifests_and_intermediary (documents) normal execution""" runner = CliRunner() args = [ - '-x', - EXCEL_FILE_PATH, - '-e', - EXCEL_SPEC_PATH, - '-c', - SITE_CONFIG_PATH, - '-s', - 'airship-seaworthy', - '-t', - TEMPLATE_PATH + '-x', EXCEL_FILE_PATH, '-e', EXCEL_SPEC_PATH, '-c', SITE_CONFIG_PATH, + '-s', 'airship-seaworthy', '-t', TEMPLATE_PATH ] - with mock.patch.object( - SiteProcessor, - 'render_template', - autospec=True) as mock_render_template: + with mock.patch.object(SiteProcessor, 'render_template', + autospec=True) as mock_render_template: result = runner.invoke(generate_manifests_and_intermediary, args) assert result.exit_code == 0 mock_intermediary_processor.assert_called_once() @@ -118,29 +96,20 @@ def test_generate_manifests_and_intermediary( mock_render_template.assert_called_once_with(mock.ANY, TEMPLATE_PATH) +@mock.patch.object(SiteProcessor, '__init__', return_value=None) @mock.patch('spyglass.cli.intermediary_processor', autospec=True) @mock.patch('spyglass_plugin_xls.excel.ExcelPlugin', autospec=False) def test_generate_manifests_and_intermediary_generate_intermediary( - mock_excel_plugin, mock_intermediary_processor): + mock_excel_plugin, mock_intermediary_processor, + mock_site_processor_init): """Test generate_intermediary option for documents command""" runner = CliRunner() args = [ - '-x', - EXCEL_FILE_PATH, - '-e', - EXCEL_SPEC_PATH, - '-c', - SITE_CONFIG_PATH, - '-s', - 'airship-seaworthy', - '-t', - TEMPLATE_PATH, - '-i' + '-x', EXCEL_FILE_PATH, '-e', EXCEL_SPEC_PATH, '-c', SITE_CONFIG_PATH, + '-s', 'airship-seaworthy', '-t', TEMPLATE_PATH, '-i' ] - with mock.patch.object( - SiteProcessor, - 'render_template', - autospec=True) as mock_render_template: + with mock.patch.object(SiteProcessor, 'render_template', + autospec=True) as mock_render_template: result = runner.invoke(generate_manifests_and_intermediary, args) assert result.exit_code == 0 mock_intermediary_processor.assert_called_once() @@ -151,33 +120,23 @@ def test_generate_manifests_and_intermediary_generate_intermediary( mock_render_template.assert_called_once_with(mock.ANY, TEMPLATE_PATH) +@mock.patch.object(SiteProcessor, '__init__', return_value=None) @mock.patch('spyglass.cli.intermediary_processor', autospec=True) @mock.patch('spyglass_plugin_xls.excel.ExcelPlugin', autospec=False) def test_generate_manifests_and_intermediary_intermediary_dir( - mock_excel_plugin, mock_intermediary_processor, tmpdir): + mock_excel_plugin, mock_intermediary_processor, + mock_site_processor_init, tmpdir): """Test intermediary_dir option for documents command""" runner = CliRunner() test_dir_name = 'intermediary_test_dir' test_dir = tmpdir.mkdir(test_dir_name) args = [ - '-x', - EXCEL_FILE_PATH, - '-e', - EXCEL_SPEC_PATH, - '-c', - SITE_CONFIG_PATH, - '-s', - 'airship-seaworthy', - '-t', - TEMPLATE_PATH, - '-i', - '--intermediary-dir', - test_dir + '-x', EXCEL_FILE_PATH, '-e', EXCEL_SPEC_PATH, '-c', SITE_CONFIG_PATH, + '-s', 'airship-seaworthy', '-t', TEMPLATE_PATH, '-i', + '--intermediary-dir', test_dir ] - with mock.patch.object( - SiteProcessor, - 'render_template', - autospec=True) as mock_render_template: + with mock.patch.object(SiteProcessor, 'render_template', + autospec=True) as mock_render_template: result = runner.invoke(generate_manifests_and_intermediary, args) assert result.exit_code == 0 mock_intermediary_processor.assert_called_once() @@ -190,8 +149,7 @@ def test_generate_manifests_and_intermediary_intermediary_dir( @mock.patch('spyglass.cli.intermediary_processor', autospec=True) @mock.patch('spyglass_plugin_xls.excel.ExcelPlugin', autospec=False) -@mock.patch.object( - SiteProcessor, '__init__', autospec=True, return_value=None) +@mock.patch.object(SiteProcessor, '__init__', autospec=True, return_value=None) def test_generate_manifests_and_intermediary_manifest_dir( mock_site_processor, mock_excel_plugin, mock_intermediary_processor, tmpdir): @@ -200,23 +158,12 @@ def test_generate_manifests_and_intermediary_manifest_dir( test_dir_name = 'manifest_test_dir' test_dir = tmpdir.mkdir(test_dir_name) args = [ - '-x', - EXCEL_FILE_PATH, - '-e', - EXCEL_SPEC_PATH, - '-c', - SITE_CONFIG_PATH, - '-s', - 'airship-seaworthy', - '-t', - TEMPLATE_PATH, - '--manifest-dir', + '-x', EXCEL_FILE_PATH, '-e', EXCEL_SPEC_PATH, '-c', SITE_CONFIG_PATH, + '-s', 'airship-seaworthy', '-t', TEMPLATE_PATH, '--manifest-dir', test_dir ] - with mock.patch.object( - SiteProcessor, - 'render_template', - autospec=True) as mock_render_template: + with mock.patch.object(SiteProcessor, 'render_template', + autospec=True) as mock_render_template: result = runner.invoke(generate_manifests_and_intermediary, args) assert result.exit_code == 0 mock_intermediary_processor.assert_called_once() diff --git a/tox.ini b/tox.ini index 8d5a889..c2b3aee 100644 --- a/tox.ini +++ b/tox.ini @@ -24,7 +24,7 @@ basepython = python3 deps = -r{toxinidir}/test-requirements.txt commands = - yapf -ir {toxinidir}/spyglass_plugin_xls {toxinidir}/setup.py + yapf -ir {toxinidir}/spyglass_plugin_xls {toxinidir}/setup.py {toxinidir}/tests [testenv:pep8] basepython = python3 @@ -32,7 +32,7 @@ deps = -r{toxinidir}/test-requirements.txt commands = bash -c "{toxinidir}/tools/gate/whitespace-linter.sh" - yapf -dr {toxinidir}/spyglass_plugin_xls {toxinidir}/setup.py + yapf -dr {toxinidir}/spyglass_plugin_xls {toxinidir}/setup.py {toxinidir}/tests flake8 {toxinidir}/spyglass_plugin_xls bandit -r spyglass_plugin_xls -n 5 safety check -r requirements.txt --bare