From 113660af777740338b0c58264397afdee738d52a Mon Sep 17 00:00:00 2001
From: John Sharples <john.sharples@bom.gov.au>
Date: Wed, 4 Oct 2023 05:04:03 +0000
Subject: [PATCH 1/5] feature 2253: command_builder tests
---
.../command_builder/test_command_builder.py | 201 +++++++++++++++++-
metplus/wrappers/command_builder.py | 2 +-
2 files changed, 200 insertions(+), 3 deletions(-)
diff --git a/internal/tests/pytests/wrappers/command_builder/test_command_builder.py b/internal/tests/pytests/wrappers/command_builder/test_command_builder.py
index 8822e165d..37ad13a48 100644
--- a/internal/tests/pytests/wrappers/command_builder/test_command_builder.py
+++ b/internal/tests/pytests/wrappers/command_builder/test_command_builder.py
@@ -1,10 +1,11 @@
#!/usr/bin/env python3
import pytest
+from unittest import mock
import os
import datetime
-
+import metplus.wrappers.command_builder as cb_wrapper
from metplus.wrappers.command_builder import CommandBuilder
from metplus.util import ti_calculate, add_field_info_to_time_info
@@ -1001,4 +1002,200 @@ def test_get_env_copy(metplus_config, shell, expected):
actual = cb.get_env_copy({'MET_TMP_DIR', 'OMP_NUM_THREADS'})
assert expected in actual
-
\ No newline at end of file
+
+
+def _in_last_err(msg, mock_logger):
+ last_msg = mock_logger.error.call_args_list[-1][0][0]
+ return msg in last_msg
+
+@pytest.mark.wrapper
+def test_get_command(metplus_config):
+ config = metplus_config
+ #config.set('user_env_vars','CMD_BUILD_USER_TEST', )
+
+ cb = CommandBuilder(config)
+ cb.app_path = '/jabberwocky/'
+ cb.infiles = ['O','frabjous','day']
+ cb.outfile = 'callooh'
+ cb.param = 'callay'
+
+ with mock.patch.object(os.path, 'dirname', return_value='callooh'):
+ with mock.patch.object(cb_wrapper, 'mkdir_p'):
+ actual = cb.get_command()
+ assert actual == '/jabberwocky/ -v 2 O frabjous day callooh callay'
+
+ with mock.patch.object(os.path, 'dirname', return_value=None):
+ actual = cb.get_command()
+ assert actual is None
+ assert _in_last_err('Must specify path to output file', cb.logger)
+
+ cb.outfile = None
+ actual = cb.get_command()
+ assert actual is None
+ assert _in_last_err('No output filename specified', cb.logger)
+
+ cb.infiles = None
+ actual = cb.get_command()
+ assert actual is None
+
+
+ assert _in_last_err('No input filenames specified', cb.logger)
+
+ cb.app_path = None
+ actual = cb.get_command()
+ assert actual is None
+ assert _in_last_err('No app path specified.', cb.logger)
+
+
+@pytest.mark.parametrize(
+ 'd_type, curly, values, expected', [
+ ('fcst',
+ True,
+ [['0.2','1.0'],'A24','Z0','extra'],
+ ['{ name="A24"; level="Z0"; cat_thresh=[ 0.2,1.0 ]; extra; }']
+ ),
+ ('fcst',
+ False,
+ [['20'],'apcp','3000',None],
+ ['\'name="apcp"; level="3000"; cat_thresh=[ 20 ];\'']
+ ),
+ ('obs',
+ True,
+ [['0.2','1.0'],'A24','Z0',None],
+ ['{ name="A24"; level="Z0"; cat_thresh=[ 0.2,1.0 ]; }']
+ ),
+ ]
+)
+@pytest.mark.wrapper
+def test_format_field_info(metplus_config,
+ d_type,
+ curly,
+ values,
+ expected):
+ var_keys = [
+ f'{d_type}_thresh',
+ f'{d_type}_name',
+ f'{d_type}_level',
+ f'{d_type}_extra',
+ ]
+ var_info = dict(zip(var_keys, values))
+
+ cb = CommandBuilder(metplus_config)
+ actual = cb.format_field_info(var_info, d_type, curly)
+ assert actual == expected
+
+
+@pytest.mark.wrapper
+def test_format_field_info_error(metplus_config):
+ cb = CommandBuilder(metplus_config)
+ with mock.patch.object(cb_wrapper, 'format_field_info', return_value='bar'):
+ actual = cb.format_field_info({},'foo')
+
+ assert actual is None
+ assert _in_last_err('bar', cb.logger)
+
+
+@pytest.mark.wrapper
+def test_get_field_info_error(metplus_config):
+ cb = CommandBuilder(metplus_config)
+ with mock.patch.object(cb_wrapper, 'get_field_info', return_value='bar'):
+ actual = cb.get_field_info({},'foo')
+
+ assert actual is None
+ assert _in_last_err('bar', cb.logger)
+
+
+@pytest.mark.parametrize(
+ 'log_metplus', [
+ (True),(False)
+ ]
+)
+@pytest.mark.wrapper
+def test_run_command_error(metplus_config, log_metplus):
+ config = metplus_config
+ if log_metplus:
+ config.set('config', 'LOG_METPLUS', '/fake/file.log')
+ else:
+ config.set('config', 'LOG_METPLUS', '')
+
+ cb = CommandBuilder(metplus_config)
+ with mock.patch.object(cb.cmdrunner, 'run_cmd', return_value=('ERR',None)):
+ actual = cb.run_command('foo')
+ assert not actual
+ assert _in_last_err('Command returned a non-zero return code: foo', cb.logger)
+
+
+
+@pytest.mark.wrapper
+def test_errors_and_defaults(metplus_config):
+ """A test to check various errors and default return"""
+ config = metplus_config
+ app_name = 'command_builder'
+ config.set('config', f'{app_name.upper()}_OUTPUT_PREFIX', 'prefix')
+ cb = CommandBuilder(metplus_config)
+ cb.app_name = app_name
+
+ # smoke test run_all_times
+ cb.run_all_times()
+ assert cb.isOK
+
+ # test get_output_prefix without time_info
+ actual = cb.get_output_prefix(time_info=None)
+ assert actual == 'prefix'
+
+ # test handle_climo_dict errors counted correctly
+ starting_errs = cb.errors
+ with mock.patch.object(cb_wrapper, 'handle_climo_dict', return_value=False):
+ for x in range(2):
+ cb.handle_climo_dict()
+ assert starting_errs + 2 == cb.errors
+
+ # test missing FLAGS returns none
+ actual = cb.handle_flags('foo')
+ assert actual is None
+
+ # test get_env_var_value empty and list
+ actual = cb.get_env_var_value('foo',{'foo':''},'list')
+ assert actual == '[]'
+
+ # test add_met_config_dict not OK
+ assert cb.isOK
+ with mock.patch.object(cb_wrapper, 'add_met_config_dict', return_value=False):
+ actual = cb.add_met_config_dict('foo', 'bar')
+ assert actual is False
+ assert not cb.isOK
+
+ # test build when no cmd
+ with mock.patch.object(cb, 'get_command', return_value=None):
+ actual = cb.build()
+ assert actual == False
+ assert _in_last_err('Could not generate command', cb.logger)
+
+ # test python embedding error
+ with mock.patch.object(cb_wrapper, 'is_python_script', return_value=True):
+ actual = cb.check_for_python_embedding('FCST',{'fcst_name':'pyEmbed'})
+ assert actual == None
+ assert _in_last_err('must be set to a valid Python Embedding type', cb.logger)
+
+ cb.c_dict['FCST_INPUT_DATATYPE'] = 'PYTHON_XARRAY'
+ with mock.patch.object(cb_wrapper, 'is_python_script', return_value=True):
+ actual = cb.check_for_python_embedding('FCST',{'fcst_name':'pyEmbed'})
+ assert actual == 'python_embedding'
+
+ # test field_info not set
+ cb.c_dict['CURRENT_VAR_INFO'] = None
+ actual = cb.set_current_field_config()
+ assert actual is None
+
+ # test check_gempaktocf
+ cb.isOK = True
+ cb.check_gempaktocf(False)
+ assert cb.isOK == False
+ assert _in_last_err('[exe] GEMPAKTOCF_JAR was not set in configuration file.', cb.logger)
+
+ # test expected ensemble mismatch
+ cb.c_dict['N_MEMBERS'] = 1
+ actual = cb._check_expected_ensembles(['file1', 'file2'])
+ assert actual is False
+ assert _in_last_err('Found more files than expected!', cb.logger)
+
diff --git a/metplus/wrappers/command_builder.py b/metplus/wrappers/command_builder.py
index 058a9e93f..3a70fe99e 100755
--- a/metplus/wrappers/command_builder.py
+++ b/metplus/wrappers/command_builder.py
@@ -1075,7 +1075,7 @@ def check_for_gempak(self):
def check_gempaktocf(self, gempaktocf_jar):
if not gempaktocf_jar:
- self.log_error("[exe] GEMPAKTOCF_JAR was not set if configuration file. "
+ self.log_error("[exe] GEMPAKTOCF_JAR was not set in configuration file. "
"This is required to process Gempak data.")
self.logger.info("Refer to the GempakToCF use case documentation for information "
"on how to obtain the tool: parm/use_cases/met_tool_wrapper/GempakToCF/GempakToCF.py")
From 7ba0ab415616e4791e333d07fc9dc7b1c241db44 Mon Sep 17 00:00:00 2001
From: John Sharples <john.sharples@bom.gov.au>
Date: Thu, 5 Oct 2023 04:00:05 +0000
Subject: [PATCH 2/5] feature 2253: command_builder tests
---
.../command_builder/test_command_builder.py | 46 +++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/internal/tests/pytests/wrappers/command_builder/test_command_builder.py b/internal/tests/pytests/wrappers/command_builder/test_command_builder.py
index 37ad13a48..cef47053e 100644
--- a/internal/tests/pytests/wrappers/command_builder/test_command_builder.py
+++ b/internal/tests/pytests/wrappers/command_builder/test_command_builder.py
@@ -1126,6 +1126,52 @@ def test_run_command_error(metplus_config, log_metplus):
+@pytest.mark.wrapper
+def test_find_input_files_ensemble(metplus_config):
+ config = metplus_config
+ cb = CommandBuilder(metplus_config)
+
+ time_info = ti_calculate({
+ 'valid': datetime.datetime.strptime("201802010000", '%Y%m%d%H%M'),
+ 'lead': 0,
+ })
+
+ # can't write file list
+ with mock.patch.object(cb, 'write_list_file', return_value=None):
+ with mock.patch.object(cb, 'find_model', return_value=['file']):
+ actual = cb.find_input_files_ensemble(time_info, False)
+ assert actual is False
+ assert _in_last_err('Could not write filelist file', cb.logger)
+
+ # not _check_expected_ensembles
+ with mock.patch.object(cb, '_check_expected_ensembles', return_value=None):
+ with mock.patch.object(cb, 'find_model', return_value=['file']):
+ actual = cb.find_input_files_ensemble(time_info)
+ assert actual is False
+
+ # no input files
+ with mock.patch.object(cb, 'find_model', return_value=[]):
+ actual = cb.find_input_files_ensemble(time_info)
+ assert actual is False
+ assert _in_last_err('Could not find any input files', cb.logger)
+
+ # file list does/doesn't exist
+ cb.c_dict['FCST_INPUT_FILE_LIST'] = 'fcst_file_list'
+ actual = cb.find_input_files_ensemble(time_info)
+ assert actual is False
+ assert _in_last_err('Could not find file list file', cb.logger)
+
+ with mock.patch.object(cb_wrapper.os.path, 'exists', return_value=True):
+ actual = cb.find_input_files_ensemble(time_info)
+ assert actual is True
+ assert cb.infiles[-1] == 'fcst_file_list'
+
+ # ctrl file not found
+ cb.c_dict['CTRL_INPUT_TEMPLATE'] = 'ctrl_file'
+ with mock.patch.object(cb, 'find_data', return_value=None):
+ actual = cb.find_input_files_ensemble(time_info)
+ assert actual is False
+
@pytest.mark.wrapper
def test_errors_and_defaults(metplus_config):
"""A test to check various errors and default return"""
From 70164bf60069cf03e7083fc3fb4a5b723956b54a Mon Sep 17 00:00:00 2001
From: John Sharples <john.sharples@bom.gov.au>
Date: Thu, 5 Oct 2023 04:08:16 +0000
Subject: [PATCH 3/5] feature 2253: tidy up
---
.../command_builder/test_command_builder.py | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/internal/tests/pytests/wrappers/command_builder/test_command_builder.py b/internal/tests/pytests/wrappers/command_builder/test_command_builder.py
index cef47053e..68427fa9d 100644
--- a/internal/tests/pytests/wrappers/command_builder/test_command_builder.py
+++ b/internal/tests/pytests/wrappers/command_builder/test_command_builder.py
@@ -1008,10 +1008,10 @@ def _in_last_err(msg, mock_logger):
last_msg = mock_logger.error.call_args_list[-1][0][0]
return msg in last_msg
+
@pytest.mark.wrapper
def test_get_command(metplus_config):
config = metplus_config
- #config.set('user_env_vars','CMD_BUILD_USER_TEST', )
cb = CommandBuilder(config)
cb.app_path = '/jabberwocky/'
@@ -1037,8 +1037,6 @@ def test_get_command(metplus_config):
cb.infiles = None
actual = cb.get_command()
assert actual is None
-
-
assert _in_last_err('No input filenames specified', cb.logger)
cb.app_path = None
@@ -1090,7 +1088,6 @@ def test_format_field_info_error(metplus_config):
cb = CommandBuilder(metplus_config)
with mock.patch.object(cb_wrapper, 'format_field_info', return_value='bar'):
actual = cb.format_field_info({},'foo')
-
assert actual is None
assert _in_last_err('bar', cb.logger)
@@ -1100,7 +1097,6 @@ def test_get_field_info_error(metplus_config):
cb = CommandBuilder(metplus_config)
with mock.patch.object(cb_wrapper, 'get_field_info', return_value='bar'):
actual = cb.get_field_info({},'foo')
-
assert actual is None
assert _in_last_err('bar', cb.logger)
@@ -1124,7 +1120,6 @@ def test_run_command_error(metplus_config, log_metplus):
assert not actual
assert _in_last_err('Command returned a non-zero return code: foo', cb.logger)
-
@pytest.mark.wrapper
def test_find_input_files_ensemble(metplus_config):
@@ -1172,9 +1167,13 @@ def test_find_input_files_ensemble(metplus_config):
actual = cb.find_input_files_ensemble(time_info)
assert actual is False
+
@pytest.mark.wrapper
def test_errors_and_defaults(metplus_config):
- """A test to check various errors and default return"""
+ """
+ A test to check various functions produce expected log messages
+ and return values on error or unexpected input.
+ """
config = metplus_config
app_name = 'command_builder'
config.set('config', f'{app_name.upper()}_OUTPUT_PREFIX', 'prefix')
From afecc6eddb463a9a27db4a84c30b64474a410042 Mon Sep 17 00:00:00 2001
From: John Sharples <john.sharples@bom.gov.au>
Date: Thu, 5 Oct 2023 05:39:18 +0000
Subject: [PATCH 4/5] feature 2253: rearrange some tests
---
.../command_builder/test_command_builder.py | 30 ++++++++-----------
1 file changed, 13 insertions(+), 17 deletions(-)
diff --git a/internal/tests/pytests/wrappers/command_builder/test_command_builder.py b/internal/tests/pytests/wrappers/command_builder/test_command_builder.py
index 68427fa9d..493d86d00 100644
--- a/internal/tests/pytests/wrappers/command_builder/test_command_builder.py
+++ b/internal/tests/pytests/wrappers/command_builder/test_command_builder.py
@@ -1083,22 +1083,6 @@ def test_format_field_info(metplus_config,
assert actual == expected
-@pytest.mark.wrapper
-def test_format_field_info_error(metplus_config):
- cb = CommandBuilder(metplus_config)
- with mock.patch.object(cb_wrapper, 'format_field_info', return_value='bar'):
- actual = cb.format_field_info({},'foo')
- assert actual is None
- assert _in_last_err('bar', cb.logger)
-
-
-@pytest.mark.wrapper
-def test_get_field_info_error(metplus_config):
- cb = CommandBuilder(metplus_config)
- with mock.patch.object(cb_wrapper, 'get_field_info', return_value='bar'):
- actual = cb.get_field_info({},'foo')
- assert actual is None
- assert _in_last_err('bar', cb.logger)
@pytest.mark.parametrize(
@@ -1243,4 +1227,16 @@ def test_errors_and_defaults(metplus_config):
actual = cb._check_expected_ensembles(['file1', 'file2'])
assert actual is False
assert _in_last_err('Found more files than expected!', cb.logger)
-
+
+ # format field info
+ with mock.patch.object(cb_wrapper, 'format_field_info', return_value='bar'):
+ actual = cb.format_field_info({},'foo')
+ assert actual is None
+ assert _in_last_err('bar', cb.logger)
+
+ # check get_field_info
+ with mock.patch.object(cb_wrapper, 'get_field_info', return_value='bar'):
+ actual = cb.get_field_info({},'foo')
+ assert actual is None
+ assert _in_last_err('bar', cb.logger)
+
From 9d42c0e03ca1a8b96c48b896b27a549fb95f427e Mon Sep 17 00:00:00 2001
From: John Sharples <john.sharples@bom.gov.au>
Date: Thu, 5 Oct 2023 06:15:52 +0000
Subject: [PATCH 5/5] feature 2253: cleanup whitespace
---
.../pytests/wrappers/command_builder/test_command_builder.py | 2 --
1 file changed, 2 deletions(-)
diff --git a/internal/tests/pytests/wrappers/command_builder/test_command_builder.py b/internal/tests/pytests/wrappers/command_builder/test_command_builder.py
index 493d86d00..93a3bcb80 100644
--- a/internal/tests/pytests/wrappers/command_builder/test_command_builder.py
+++ b/internal/tests/pytests/wrappers/command_builder/test_command_builder.py
@@ -1082,8 +1082,6 @@ def test_format_field_info(metplus_config,
actual = cb.format_field_info(var_info, d_type, curly)
assert actual == expected
-
-
@pytest.mark.parametrize(
'log_metplus', [