Skip to content

Commit

Permalink
Merge pull request #6289 from oliver-sanders/jinja2-assert--
Browse files Browse the repository at this point in the history
jinja2: removed excessive context from Jinja2 raise/assert statements
  • Loading branch information
oliver-sanders authored Aug 27, 2024
2 parents cadf1ef + 9c88c5c commit b0fb246
Show file tree
Hide file tree
Showing 9 changed files with 102 additions and 91 deletions.
1 change: 1 addition & 0 deletions changes.d/6289.feat.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Made the errors resulting from Jinja2 `raise` and `assert` statements more straight forward.
26 changes: 22 additions & 4 deletions cylc/flow/parsec/jinja2support.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
TemplateSyntaxError)

from cylc.flow import LOG
from cylc.flow.exceptions import InputError
import cylc.flow.flags
from cylc.flow.parsec.exceptions import Jinja2Error
from cylc.flow.parsec.fileparse import get_cylc_env_vars

Expand Down Expand Up @@ -91,16 +93,19 @@ def root_render_func(context, *args, **kwargs):
return templ


def raise_helper(message, error_type='Error'):
class Jinja2AssertionError(Exception):
"""Exception raised by the Jinja2 "raise" and "assert" functions."""


def raise_helper(message):
"""Provides a Jinja2 function for raising exceptions."""
# TODO - this more nicely
raise Exception('Jinja2 %s: %s' % (error_type, message))
raise Jinja2AssertionError(message)


def assert_helper(logical, message):
"""Provides a Jinja2 function for asserting logical expressions."""
if not logical:
raise_helper(message, 'Assertion Error')
raise_helper(message)
return '' # Prevent None return value polluting output.


Expand Down Expand Up @@ -298,6 +303,19 @@ def jinja2process(
lines=get_error_lines(fpath, flines),
filename=filename
) from None
except Jinja2AssertionError as exc:
if cylc.flow.flags.verbosity < 1:
# raise a user-friently representation of this assertion error
raise InputError(
f'{str(exc)}'
'\n(add --verbose for more context)'
) from None

# raise the full form of the error in verbose mode
raise Jinja2Error(
exc,
lines=get_error_lines(fpath, flines),
) from None
except Exception as exc:
raise Jinja2Error(
exc,
Expand Down
23 changes: 1 addition & 22 deletions tests/functional/jinja2/03-bad.t
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
# an include statement

. "$(dirname "$0")/test_header"
set_test_number 12
set_test_number 10

sub_tabs() {
sed -i 's/\t/ /g' "${TEST_NAME}.stderr"
Expand Down Expand Up @@ -51,27 +51,6 @@ __HERE__

purge_workflow

#------------------------------------------------------------------------------
# Test generic exception in an include file
install_workflow "${TEST_NAME_BASE}" include-exception

TEST_NAME="${TEST_NAME_BASE}-include-exception"
run_fail "${TEST_NAME}" cylc validate "${WORKFLOW_NAME}"

sub_tabs
cmp_ok "${TEST_NAME}.stderr" <<__HERE__
Jinja2Error: Jinja2 Error: some error
File ${WORKFLOW_RUN_DIR}/foo.cylc
# line before error
{{ raise('some error') }} <-- Exception
File ${WORKFLOW_RUN_DIR}/flow.cylc
#!Jinja2
# line before
{% include "foo.cylc" %} <-- Exception
__HERE__

purge_workflow

#------------------------------------------------------------------------------
# Test syntax error in an include file
install_workflow "${TEST_NAME_BASE}" include-badsyntax
Expand Down
38 changes: 0 additions & 38 deletions tests/functional/jinja2/10-builtin-functions.t

This file was deleted.

13 changes: 0 additions & 13 deletions tests/functional/jinja2/10-builtin-functions/flow.cylc

This file was deleted.

4 changes: 0 additions & 4 deletions tests/functional/jinja2/include-exception/flow.cylc

This file was deleted.

3 changes: 0 additions & 3 deletions tests/functional/jinja2/include-exception/foo.cylc

This file was deleted.

76 changes: 76 additions & 0 deletions tests/integration/validate/test_jinja2.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
# THIS FILE IS PART OF THE CYLC WORKFLOW ENGINE.
# Copyright (C) NIWA & British Crown (Met Office) & Contributors.
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.

import re
from textwrap import dedent

from cylc.flow.exceptions import InputError
from cylc.flow.parsec.exceptions import Jinja2Error

import pytest


@pytest.fixture
def flow_cylc(tmp_path):
"""Write a flow.cylc file containing the provided text."""
def _inner(text):
nonlocal tmp_path
(tmp_path / 'flow.cylc').write_text(dedent(text).strip())
return tmp_path

return _inner


@pytest.mark.parametrize(
'line',
[
pytest.param("raise('some error message')", id='raise'),
pytest.param("assert(False, 'some error message')", id='assert'),
],
)
def test_raise_helper(flow_cylc, validate, line, monkeypatch):
"""It should raise an error from within Jinja2."""
# it should raise a minimal InputError
# (because assert/raise are used to validate inputs)
# - intended for users of the workflow
src_dir = flow_cylc(f'''
#!Jinja2
{{{{ {line} }}}}
''')
with pytest.raises(
InputError,
match=(
r'^some error message'
r'\n\(add --verbose for more context\)$'
),
):
validate(src_dir)

# in verbose mode, it should raise the full error
# (this includes the Jinja2 context including file/line info)
# - intended for developers of the workflow
monkeypatch.setattr('cylc.flow.flags.verbosity', 1)
with pytest.raises(
Jinja2Error,
match=(
r'^some error message'
r'\nFile /.*/pytest-.*/test_.*/flow.cylc'
r'\n #!Jinja2'
r'\n \{\{ ' + re.escape(line) + r' \}\}'
r'\t<-- Jinja2AssertionError$'
)
):
validate(src_dir)
9 changes: 2 additions & 7 deletions tests/unit/parsec/test_jinja2support.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,10 @@
def test_raise_helper():
message = 'Ops'
error_type = 'CRITICAL'
with pytest.raises(Exception) as cm:
with pytest.raises(Jinja2AssertionError) as cm:
raise_helper(message=message)

assert str(cm.value) == "Jinja2 Error: Ops"

with pytest.raises(Exception) as cm:
raise_helper(message=message, error_type=error_type)

assert str(cm.value) == "Jinja2 CRITICAL: Ops"
assert str(cm.value) == "Ops"


def test_assert_helper():
Expand Down

0 comments on commit b0fb246

Please sign in to comment.