From ef61f972907611e75047db4b55757f8bc00d87f3 Mon Sep 17 00:00:00 2001 From: Francis Charette Migneault Date: Thu, 12 Oct 2023 14:57:03 -0400 Subject: [PATCH] fix extra test cases --- tests/test_notify.py | 93 +++++++++++++++++++++++++++++--------------- weaver/notify.py | 15 +++++-- 2 files changed, 73 insertions(+), 35 deletions(-) diff --git a/tests/test_notify.py b/tests/test_notify.py index 0b786aa8e..17c9d89ac 100644 --- a/tests/test_notify.py +++ b/tests/test_notify.py @@ -3,7 +3,9 @@ import pathlib import smtplib import tempfile +import textwrap import uuid +from typing import TYPE_CHECKING import mock import pytest @@ -13,6 +15,9 @@ from weaver.notify import decrypt_email, encrypt_email, notify_job_email, resolve_email_template from weaver.status import Status +if TYPE_CHECKING: + from typing import Dict, Optional, Type, Union + def test_encrypt_decrypt_email_valid(): settings = { @@ -154,29 +159,32 @@ def test_notify_job_email_custom_template(): @pytest.mark.parametrize( - ["settings", "test_process", "test_status", "tmp_default", "expect_result"], + ["settings", "test_process", "test_status", "test_default", "tmp_default", "expect_result"], [ - ({}, None, None, None, 4), + ({}, None, None, False, None, 4), # directory exists, but none of the supported mako variants found under it - ({"weaver.wps_email_notify_template_dir": tempfile.gettempdir()}, None, None, None, IOError), - ({"weaver.wps_email_notify_template_dir": "/DOES_NOT_EXIST"}, None, None, None, 4), - ({"weaver.wps_email_notify_template_dir": ""}, None, None, None, 4), + ({"weaver.wps_email_notify_template_dir": tempfile.gettempdir()}, None, None, False, None, IOError), + ({"weaver.wps_email_notify_template_dir": "/DOES_NOT_EXIST"}, None, None, False, None, IOError), + ({"weaver.wps_email_notify_template_dir": ""}, None, None, False, None, IOError), + ({"weaver.wps_email_notify_template_dir": "", + "weaver.wps_email_notify_template_default": "RANDOM"}, None, None, False, None, IOError), + ({"weaver.wps_email_notify_template_dir": ""}, None, None, False, None, IOError), ({"weaver.wps_email_notify_template_dir": "", - "weaver.wps_email_notify_template_default": "RANDOM"}, None, None, None, 4), - ({"weaver.wps_email_notify_template_dir": ""}, None, None, None, 4), + "weaver.wps_email_notify_template_default": "test-default.mako"}, None, False, None, "random.mako", IOError), + ({"weaver.wps_email_notify_template_dir": ""}, None, None, False, "test-default.mako", IOError), ({"weaver.wps_email_notify_template_dir": "", - "weaver.wps_email_notify_template_default": "test-default.mako"}, None, None, "random.mako", 4), - ({"weaver.wps_email_notify_template_dir": ""}, None, None, "test-default.mako", 4), - ({"weaver.wps_email_notify_template_dir": ""}, None, None, "test-default.mako", 4), + "weaver.wps_email_notify_template_default": "test-default.mako"}, None, None, False, "default.mako", IOError), ({"weaver.wps_email_notify_template_dir": "", - "weaver.wps_email_notify_template_default": "test-default.mako"}, None, None, "default.mako", 3), + "weaver.wps_email_notify_template_default": "test-default.mako"}, None, None, True, "default.mako", IOError), + ({"weaver.wps_email_notify_template_dir": ""}, None, None, True, None, 3), ({"weaver.wps_email_notify_template_dir": "", - "weaver.wps_email_notify_template_default": "test-default.mako"}, None, None, "test-default.mako", 2), + "weaver.wps_email_notify_template_default": "test-default.mako"}, None, None, False, "test-default.mako", 2), ( {"weaver.wps_email_notify_template_dir": "", "weaver.wps_email_notify_template_default": "test-default.mako"}, "random-process", None, + False, "test-default.mako", 2 ), @@ -185,14 +193,16 @@ def test_notify_job_email_custom_template(): "weaver.wps_email_notify_template_default": "test-default.mako"}, "random-process", Status.SUCCEEDED, + False, "test-default.mako", 2 ), ( {"weaver.wps_email_notify_template_dir": "", "weaver.wps_email_notify_template_default": "test-default.mako"}, - "tmp-process", + "random-process", Status.STARTED, + False, "test-default.mako", 2 ), @@ -201,61 +211,82 @@ def test_notify_job_email_custom_template(): "weaver.wps_email_notify_template_default": "test-default.mako"}, "tmp-process", None, + False, "test-default.mako", - 2 + 1 ), ( {"weaver.wps_email_notify_template_dir": "", "weaver.wps_email_notify_template_default": "test-default.mako"}, "tmp-process", Status.SUCCEEDED, + False, "test-default.mako", - 2 + 1 ), ( {"weaver.wps_email_notify_template_dir": "", "weaver.wps_email_notify_template_default": "test-default.mako"}, "tmp-process", Status.STARTED, + False, "test-default.mako", - 1 + 0 ), ] ) -def test_resolve_email_template(settings, test_process, test_status, tmp_default, expect_result): - # process name and job status important to evaluate expected mako file resolution +def test_resolve_email_template(settings, test_process, test_status, test_default, tmp_default, expect_result): + # type: (Dict[str, str], Optional[str], Optional[str], bool, Optional[str], Union[Type[Exception], int]) -> None + + # process name and job status are important to evaluate expected mako file resolution tmp_process = "tmp-process" tmp_status = Status.STARTED with contextlib.ExitStack() as tmp_stack: tmp_dir = pathlib.Path(tmp_stack.enter_context(tempfile.TemporaryDirectory())) if settings.get("weaver.wps_email_notify_template_dir") == "": - settings["weaver.wps_email_notify_template_dir"] = tmp_dir + settings["weaver.wps_email_notify_template_dir"] = str(tmp_dir) tmp_proc_dir = tmp_dir / tmp_process os.makedirs(tmp_proc_dir, exist_ok=True) - tmp_file0 = tmp_proc_dir / f"{tmp_process}.mako" + tmp_file0 = tmp_proc_dir / f"{tmp_status}.mako" tmp_file0.touch() - tmp_file1 = tmp_dir / f"{tmp_status}.mako" + tmp_file1 = tmp_dir / f"{tmp_process}.mako" tmp_file1.touch() - tmp_file2 = tmp_dir / f"{tmp_default}.mako" + tmp_file2 = tmp_dir / str(tmp_default) + if tmp_default: + tmp_file2.touch() tmp_file3 = tmp_dir / "default.mako" + if test_default: + tmp_file3.touch() default_file = os.path.join(WEAVER_MODULE_DIR, "wps_restapi/templates/notification_email_example.mako") ordered_possible_matches = [ - tmp_file0, # {tmp_dir}/{process}/{status}.mako - tmp_file1, # {tmp_dir}/{process}.mako - tmp_file2, # {tmp_dir}/{default}.mako - tmp_file3, # {tmp_dir}/default.mako - default_file, # weaver default mako + str(tmp_file0), # {tmp_dir}/{process}/{status}.mako + str(tmp_file1), # {tmp_dir}/{process}.mako + str(tmp_file2), # {tmp_dir}/{default}.mako + str(tmp_file3), # {tmp_dir}/default.mako + str(default_file), # weaver default mako ] + tmp_dir_files = list(sorted(os.path.join(root, file) for root, _, files in os.walk(tmp_dir) for file in files)) + tmp_dir_msg = "Temporary directory contents:\n{}".format(textwrap.indent("\n".join(tmp_dir_files), " ")) test_job = Job(task_id=uuid.uuid4(), process=test_process, status=test_status) try: found_template = resolve_email_template(test_job, settings) found_template_index = ordered_possible_matches.index(found_template.filename) - assert isinstance(expect_result, int), f"Test expected to raise {expect_result} but did not raise." - assert found_template_index == expect_result + assert isinstance(expect_result, int), ( + f"Test expected to raise {expect_result} but did not raise.\n{tmp_dir_msg}" + ) + assert found_template_index == expect_result, ( + f"Test did not match the expected template file.\n{tmp_dir_msg}" + ) + except AssertionError: + raise except Exception as exc: - assert issubclass(expect_result, Exception), f"Test did not expect an error, but raised {exc!s}." - assert isinstance(exc, expect_result), f"Test expected {expect_result}, but raised {exc!s} instead." + assert not isinstance(expect_result, int), ( + f"Test did not expect an error, but raised {exc!r}.\n{tmp_dir_msg}" + ) + assert isinstance(exc, expect_result), ( + f"Test expected {expect_result}, but raised {exc!r} instead.\n{tmp_dir_msg}" + ) diff --git a/weaver/notify.py b/weaver/notify.py index 4c5df65a6..46d17f266 100644 --- a/weaver/notify.py +++ b/weaver/notify.py @@ -37,14 +37,21 @@ def resolve_email_template(job, settings): """ Finds the most appropriate Mako Template email notification file based on configuration and :term:`Job` context. - .. note:: - The example template is used by default if the template directory is not overridden - (weaver/wps_restapi/templates/notification_email_example.mako). + The example template is used by default *ONLY* if the template directory was not overridden. If overridden, failing + to match any of the template file locations will raise to report the issue instead of silently using the default. + + .. seealso:: + https://github.com/crim-ca/weaver/blob/master/weaver/wps_restapi/templates/notification_email_example.mako + + :raises IOError: + If the template directory was configured explicitly, but cannot be resolved, or if any of the possible + combinations of template file names cannot be resolved under that directory. + :returns: Matched template instance based on resolution order as described in the documentation. """ template_dir = settings.get("weaver.wps_email_notify_template_dir") or "" # find appropriate template according to settings - if not os.path.isdir(template_dir): + if not template_dir and not os.path.isdir(template_dir): LOGGER.warning("No default email template directory configured. Using default template.") template_file = os.path.join(WEAVER_MODULE_DIR, "wps_restapi/templates/notification_email_example.mako") template = Template(filename=template_file) # nosec: B702