From d68e821db60ae420c4c50e53c6d08ded731240c2 Mon Sep 17 00:00:00 2001 From: Chris Billows <152496175+chrisbillowsMO@users.noreply.github.com> Date: Thu, 23 May 2024 13:04:55 +0100 Subject: [PATCH 1/7] #3392: Add basic function and basic tests. --- .../app/configure/bin/__init__.py | 0 .../app/configure/bin/configure.py | 44 +++++++++++ .../app/configure/bin/test_configure.py | 77 +++++++++++++++++++ 3 files changed, 121 insertions(+) create mode 100644 esmvaltool/utils/recipe_test_workflow/recipe_test_workflow/app/configure/bin/__init__.py create mode 100644 esmvaltool/utils/recipe_test_workflow/recipe_test_workflow/app/configure/bin/test_configure.py diff --git a/esmvaltool/utils/recipe_test_workflow/recipe_test_workflow/app/configure/bin/__init__.py b/esmvaltool/utils/recipe_test_workflow/recipe_test_workflow/app/configure/bin/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/esmvaltool/utils/recipe_test_workflow/recipe_test_workflow/app/configure/bin/configure.py b/esmvaltool/utils/recipe_test_workflow/recipe_test_workflow/app/configure/bin/configure.py index bdd4f419f0..c01378faf3 100755 --- a/esmvaltool/utils/recipe_test_workflow/recipe_test_workflow/app/configure/bin/configure.py +++ b/esmvaltool/utils/recipe_test_workflow/recipe_test_workflow/app/configure/bin/configure.py @@ -4,6 +4,7 @@ import pprint import yaml +from esmvalcore.config._config_validators import ValidationError, _validators def main(): @@ -17,6 +18,9 @@ def main(): # 'configure' task. config_values = get_config_values_from_task_env() + # Validate the user config file content. + validate_user_config_file(config_values) + # Update the configuration from OS environment. user_config_path = os.environ["USER_CONFIG_PATH"] config_values["config_file"] = user_config_path @@ -80,6 +84,46 @@ def get_config_values_from_task_env(): return config_values_from_task_env +def validate_user_config_file(user_config_file_content): + """Validate a user config with ``ESMValCore.config._validators`` functions. + + Parameters + ---------- + user_config_file_content: dict + An ESMValTool user configuration file loaded in memory as a Python + dictionary. + + Raises + ------ + KeyError + If ``user_config_file_content`` includes a key for which there is no + validator listed in ``_validators``, + ValidationError + If any of the called validation functions raise a ValidationError. + """ + errors = [] + for user_config_key, usr_config_value in user_config_file_content.items(): + try: + validatation_function = _validators[user_config_key] + except KeyError as err: + errors.append( + f'Key Error for {user_config_key.upper()}. May not be a valid ' + f'ESMValTool user configuration key\nERROR: {err}\n') + else: + try: + print(f'Validating {user_config_key.upper()} with value ' + f'"{usr_config_value}" using function ' + f'{validatation_function.__name__.upper()}.') + validatation_function(usr_config_value) + except ValidationError as err: + errors.append( + f'Validation error for {user_config_key.upper()} with ' + f'value "{usr_config_value}"\nERROR: {err}\n') + if errors: + print(errors) + raise ValidationError("\n".join(errors)) + + def write_yaml(file_path, contents): """Write ``contents`` to the YAML file ``file_path``. diff --git a/esmvaltool/utils/recipe_test_workflow/recipe_test_workflow/app/configure/bin/test_configure.py b/esmvaltool/utils/recipe_test_workflow/recipe_test_workflow/app/configure/bin/test_configure.py new file mode 100644 index 0000000000..118d8f4815 --- /dev/null +++ b/esmvaltool/utils/recipe_test_workflow/recipe_test_workflow/app/configure/bin/test_configure.py @@ -0,0 +1,77 @@ +import pytest +from bin.configure import validate_user_config_file +from esmvalcore.config._config_validators import ValidationError + +# Run tests with `pytest -c dev/null`. + + +def test_validate_user_config_file(): + mock_valid_config = { + "output_dir": "~/esmvaltool_output", + "auxiliary_data_dir": "~/auxiliary_data", + "search_esgf": "never", + "download_dir": "~/climate_data", + "max_parallel_tasks": None, + "log_level": "info", + "exit_on_warning": True, + "output_file_type": "png", + } + result = validate_user_config_file(mock_valid_config) + assert result is None + + +def test_validate_user_config_file_one_validation_error(): + mock_one_invalid_config = { + "output_dir": "~/esmvaltool_output", + "auxiliary_data_dir": "~/auxiliary_data", + "search_esgf": "never", + "download_dir": "~/climate_data", + "max_parallel_tasks": None, + "log_level": "info", + "exit_on_warning": 100, + "output_file_type": "png", + } + with pytest.raises( + ValidationError, + match='Validation error for EXIT_ON_WARNING with value "100"\n' + 'ERROR: Could not convert `100` to `bool`\n'): + validate_user_config_file(mock_one_invalid_config) + + +def test_validate_user_config_file_two_validation_errors(): + mock_two_invalids_config = { + "output_dir": 111, + "auxiliary_data_dir": "~/auxiliary_data", + "search_esgf": "never", + "download_dir": "~/climate_data", + "max_parallel_tasks": None, + "log_level": "info", + "exit_on_warning": 100, + "output_file_type": "png", + } + with pytest.raises( + ValidationError, + match='Validation error for OUTPUT_DIR with value "111"\nERROR: ' + 'Expected a path, but got 111\n\nValidation error for ' + 'EXIT_ON_WARNING with value "100"\nERROR: Could not convert `100` ' + 'to `bool`\n'): + validate_user_config_file(mock_two_invalids_config) + + +def test_validate_user_config_file_key_error(): + mock_one_key_error = { + "output_dir": "~/esmvaltool_output", + "auxiliary_data_dir": "~/auxiliary_data", + "search_esgf": "never", + "download_dir": "~/climate_data", + "one_rogue_field": 90210, + "max_parallel_tasks": None, + "log_level": "info", + "exit_on_warning": True, + "output_file_type": "png", + } + with pytest.raises( + ValidationError, + match="Key Error for ONE_ROGUE_FIELD. May not be a valid " + "ESMValTool user configuration key\nERROR: 'one_rogue_field'\n"): + validate_user_config_file(mock_one_key_error) From d646d2df8494d0482427f3b41159f1276075e648 Mon Sep 17 00:00:00 2001 From: Chris Billows <152496175+chrisbillowsMO@users.noreply.github.com> Date: Thu, 23 May 2024 15:17:38 +0100 Subject: [PATCH 2/7] #3392: Improve user messages. --- .../recipe_test_workflow/app/configure/bin/configure.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/esmvaltool/utils/recipe_test_workflow/recipe_test_workflow/app/configure/bin/configure.py b/esmvaltool/utils/recipe_test_workflow/recipe_test_workflow/app/configure/bin/configure.py index c01378faf3..862b3d4319 100755 --- a/esmvaltool/utils/recipe_test_workflow/recipe_test_workflow/app/configure/bin/configure.py +++ b/esmvaltool/utils/recipe_test_workflow/recipe_test_workflow/app/configure/bin/configure.py @@ -101,7 +101,10 @@ def validate_user_config_file(user_config_file_content): ValidationError If any of the called validation functions raise a ValidationError. """ - errors = [] + errors = [ + "There were validation errors in your user configuration file. See " + "details below." + ] for user_config_key, usr_config_value in user_config_file_content.items(): try: validatation_function = _validators[user_config_key] @@ -120,8 +123,8 @@ def validate_user_config_file(user_config_file_content): f'Validation error for {user_config_key.upper()} with ' f'value "{usr_config_value}"\nERROR: {err}\n') if errors: - print(errors) raise ValidationError("\n".join(errors)) + print("All validation checks passed.") def write_yaml(file_path, contents): From ac0ceafca1eab88817ca64536ee1c358965bad2e Mon Sep 17 00:00:00 2001 From: Chris Billows <152496175+chrisbillowsMO@users.noreply.github.com> Date: Thu, 23 May 2024 15:52:33 +0100 Subject: [PATCH 3/7] #3392: Add additional line break. --- .../recipe_test_workflow/app/configure/bin/configure.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esmvaltool/utils/recipe_test_workflow/recipe_test_workflow/app/configure/bin/configure.py b/esmvaltool/utils/recipe_test_workflow/recipe_test_workflow/app/configure/bin/configure.py index 862b3d4319..f8f4878251 100755 --- a/esmvaltool/utils/recipe_test_workflow/recipe_test_workflow/app/configure/bin/configure.py +++ b/esmvaltool/utils/recipe_test_workflow/recipe_test_workflow/app/configure/bin/configure.py @@ -103,7 +103,7 @@ def validate_user_config_file(user_config_file_content): """ errors = [ "There were validation errors in your user configuration file. See " - "details below." + "details below.\n" ] for user_config_key, usr_config_value in user_config_file_content.items(): try: From 4ab8237fd46969888f384b62ced9bf30160c3c51 Mon Sep 17 00:00:00 2001 From: Chris Billows <152496175+chrisbillowsMO@users.noreply.github.com> Date: Thu, 23 May 2024 16:09:26 +0100 Subject: [PATCH 4/7] #3392: Fix for error message string in errors list. --- .../recipe_test_workflow/app/configure/bin/configure.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esmvaltool/utils/recipe_test_workflow/recipe_test_workflow/app/configure/bin/configure.py b/esmvaltool/utils/recipe_test_workflow/recipe_test_workflow/app/configure/bin/configure.py index f8f4878251..380dfeb2df 100755 --- a/esmvaltool/utils/recipe_test_workflow/recipe_test_workflow/app/configure/bin/configure.py +++ b/esmvaltool/utils/recipe_test_workflow/recipe_test_workflow/app/configure/bin/configure.py @@ -122,7 +122,7 @@ def validate_user_config_file(user_config_file_content): errors.append( f'Validation error for {user_config_key.upper()} with ' f'value "{usr_config_value}"\nERROR: {err}\n') - if errors: + if len(errors) > 1: raise ValidationError("\n".join(errors)) print("All validation checks passed.") From 18745ab060c99879289836ab4c82268e0c13c42d Mon Sep 17 00:00:00 2001 From: Chris Billows <152496175+chrisbillowsMO@users.noreply.github.com> Date: Thu, 23 May 2024 16:32:07 +0100 Subject: [PATCH 5/7] #3392: Remove assert for codacy/pylint. --- .../app/configure/bin/test_configure.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/esmvaltool/utils/recipe_test_workflow/recipe_test_workflow/app/configure/bin/test_configure.py b/esmvaltool/utils/recipe_test_workflow/recipe_test_workflow/app/configure/bin/test_configure.py index 118d8f4815..68decf7f7d 100644 --- a/esmvaltool/utils/recipe_test_workflow/recipe_test_workflow/app/configure/bin/test_configure.py +++ b/esmvaltool/utils/recipe_test_workflow/recipe_test_workflow/app/configure/bin/test_configure.py @@ -2,8 +2,6 @@ from bin.configure import validate_user_config_file from esmvalcore.config._config_validators import ValidationError -# Run tests with `pytest -c dev/null`. - def test_validate_user_config_file(): mock_valid_config = { @@ -16,8 +14,9 @@ def test_validate_user_config_file(): "exit_on_warning": True, "output_file_type": "png", } - result = validate_user_config_file(mock_valid_config) - assert result is None + # No assert statement is needed. If the function call errors Pytest + # considers the test failed. + validate_user_config_file(mock_valid_config) def test_validate_user_config_file_one_validation_error(): From 01f38971418bea5308256774dbaba10f035a78e8 Mon Sep 17 00:00:00 2001 From: Chris Billows <152496175+chrisbillowsMO@users.noreply.github.com> Date: Thu, 23 May 2024 16:40:15 +0100 Subject: [PATCH 6/7] #3392: Update documentation. --- .../recipe_test_workflow/doc/source/user_guide/workflow.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/esmvaltool/utils/recipe_test_workflow/doc/source/user_guide/workflow.rst b/esmvaltool/utils/recipe_test_workflow/doc/source/user_guide/workflow.rst index ca52d19711..3bb4a9b7e5 100644 --- a/esmvaltool/utils/recipe_test_workflow/doc/source/user_guide/workflow.rst +++ b/esmvaltool/utils/recipe_test_workflow/doc/source/user_guide/workflow.rst @@ -20,7 +20,7 @@ The |RTW| performs the following steps: ``get_esmval`` :Description: - Either clones the latest versions of |ESMValTool| and |ESMValCore| from GitHub, + Either clones the latest versions of |ESMValTool| and |ESMValCore| from GitHub, or gets the latest container image from DockerHub and converts to a singularity image, depending on ``SITE``. :Runs on: @@ -35,13 +35,13 @@ The |RTW| performs the following steps: ``configure`` :Description: - Creates the |ESMValTool| user configuration file + Creates the |ESMValTool| user configuration file and validates it. :Runs on: Localhost :Executes: The ``configure.py`` script from the |Rose| app :Details: - ``configure`` should run at the start of each cycle after + ``configure`` should run at the start of each cycle after ``install_env_file`` has completed. ``process`` From 17e97eecb4ef3b949a9d2e82b6325d476acea9dd Mon Sep 17 00:00:00 2001 From: Chris Billows <152496175+chrisbillowsMO@users.noreply.github.com> Date: Thu, 23 May 2024 17:30:58 +0100 Subject: [PATCH 7/7] #3392: Exclude test_configure.py. --- .codacy.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.codacy.yml b/.codacy.yml index 06a0ea342f..a4d0293fd5 100644 --- a/.codacy.yml +++ b/.codacy.yml @@ -21,5 +21,6 @@ engines: exclude_paths: [ 'doc/sphinx/**', 'esmvaltool/cmor/tables/**', - 'tests/**' + 'tests/**', + 'esmvaltool/utils/recipe_test_workflow/recipe_test_workflow/app/configure/bin/test_configure.py' ]