From 933b2730f790b12f2c81d923f46aa4f99188abc5 Mon Sep 17 00:00:00 2001
From: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>
Date: Tue, 20 Feb 2024 08:27:51 +0000
Subject: [PATCH] Improve xtrigger validation (#60)
* Improve xtrigger validation
* wall_clock: use placeholder function for signature validation & autodocs
* Fix docstring for autodoc [skip ci]
---
cylc/flow/config.py | 13 +--
cylc/flow/exceptions.py | 7 +-
cylc/flow/xtrigger_mgr.py | 88 +++++++++++-------
cylc/flow/xtriggers/echo.py | 18 ++--
cylc/flow/xtriggers/wall_clock.py | 61 ++++++-------
cylc/flow/xtriggers/xrandom.py | 57 ++++--------
tests/functional/runahead/04-no-final-cycle.t | 2 +-
tests/functional/runahead/no_final/flow.cylc | 6 +-
.../spawn-on-demand/15-stop-flow-3/flow.cylc | 4 +-
.../35-pass-special-tasks-non-word-names.t | 40 ---------
.../validate/69-bare-clock-xtrigger.t | 32 -------
tests/integration/test_config.py | 90 +++++++++++++------
tests/integration/test_xtrigger_mgr.py | 1 +
tests/unit/test_xtrigger_mgr.py | 13 +--
tests/unit/xtriggers/test_echo.py | 19 ++--
tests/unit/xtriggers/test_wall_clock.py | 34 ++++---
tests/unit/xtriggers/test_xrandom.py | 18 ++--
17 files changed, 236 insertions(+), 267 deletions(-)
delete mode 100755 tests/functional/validate/35-pass-special-tasks-non-word-names.t
delete mode 100644 tests/functional/validate/69-bare-clock-xtrigger.t
diff --git a/cylc/flow/config.py b/cylc/flow/config.py
index 1b0da424a42..f871531adeb 100644
--- a/cylc/flow/config.py
+++ b/cylc/flow/config.py
@@ -83,7 +83,6 @@
from cylc.flow.print_tree import print_tree
from cylc.flow.simulation import configure_sim_modes
from cylc.flow.subprocctx import SubFuncContext
-from cylc.flow.subprocpool import get_xtrig_func
from cylc.flow.task_events_mgr import (
EventData,
get_event_handler_data
@@ -1735,14 +1734,6 @@ def generate_triggers(self, lexpression, left_nodes, right, seq,
# Generic xtrigger validation.
XtriggerManager.check_xtrigger(label, xtrig, self.fdir)
- # Specific xtrigger.validate(), if available.
- with suppress(AttributeError, ImportError):
- get_xtrig_func(xtrig.func_name, "validate", self.fdir)(
- xtrig.func_args,
- xtrig.func_kwargs,
- xtrig.get_signature()
- )
-
if self.xtrigger_mgr:
# (not available during validation)
self.xtrigger_mgr.add_trig(label, xtrig, self.fdir)
@@ -2434,8 +2425,8 @@ def upgrade_clock_triggers(self):
# Derive an xtrigger label.
label = '_'.join(('_cylc', 'wall_clock', task_name))
# Define the xtrigger function.
- xtrig = SubFuncContext(label, 'wall_clock', [], {})
- xtrig.func_kwargs["offset"] = offset
+ args = [] if offset is None else [offset]
+ xtrig = SubFuncContext(label, 'wall_clock', args, {})
if self.xtrigger_mgr is None:
XtriggerManager.check_xtrigger(label, xtrig, self.fdir)
else:
diff --git a/cylc/flow/exceptions.py b/cylc/flow/exceptions.py
index 43591e7be9c..3ad8c093757 100644
--- a/cylc/flow/exceptions.py
+++ b/cylc/flow/exceptions.py
@@ -241,13 +241,12 @@ class XtriggerConfigError(WorkflowConfigError):
"""
- def __init__(self, label: str, trigger: str, message: str):
+ def __init__(self, label: str, message: str):
self.label: str = label
- self.trigger: str = trigger
self.message: str = message
- def __str__(self):
- return f'[{self.label}] {self.message}'
+ def __str__(self) -> str:
+ return f'[@{self.label}] {self.message}'
class ClientError(CylcError):
diff --git a/cylc/flow/xtrigger_mgr.py b/cylc/flow/xtrigger_mgr.py
index d42e9a87710..b891165dbd3 100644
--- a/cylc/flow/xtrigger_mgr.py
+++ b/cylc/flow/xtrigger_mgr.py
@@ -28,9 +28,10 @@
import cylc.flow.flags
from cylc.flow.hostuserutil import get_user
from cylc.flow.subprocpool import get_xtrig_func
-from cylc.flow.xtriggers.wall_clock import wall_clock
+from cylc.flow.xtriggers.wall_clock import _wall_clock
if TYPE_CHECKING:
+ from inspect import BoundArguments
from cylc.flow.broadcast_mgr import BroadcastMgr
from cylc.flow.data_store_mgr import DataStoreMgr
from cylc.flow.subprocctx import SubFuncContext
@@ -246,9 +247,11 @@ def check_xtrigger(
fctx: 'SubFuncContext',
fdir: str,
) -> None:
- """Generic xtrigger validation: check existence and string templates.
+ """Generic xtrigger validation: check existence, string templates and
+ function signature.
- Xtrigger modules may also supply a specific "validate" function.
+ Xtrigger modules may also supply a specific `validate` function
+ which will be run here.
Args:
label: xtrigger label
@@ -262,6 +265,7 @@ def check_xtrigger(
* If the function is not callable.
* If any string template in the function context
arguments are not present in the expected template values.
+ * If the arguments do not match the function signature.
"""
fname: str = fctx.func_name
@@ -270,32 +274,30 @@ def check_xtrigger(
func = get_xtrig_func(fname, fname, fdir)
except ImportError:
raise XtriggerConfigError(
- label,
- fname,
- f"xtrigger module '{fname}' not found",
+ label, f"xtrigger module '{fname}' not found",
)
except AttributeError:
raise XtriggerConfigError(
- label,
- fname,
- f"'{fname}' not found in xtrigger module '{fname}'",
+ label, f"'{fname}' not found in xtrigger module '{fname}'",
)
if not callable(func):
raise XtriggerConfigError(
- label,
- fname,
- f"'{fname}' not callable in xtrigger module '{fname}'",
+ label, f"'{fname}' not callable in xtrigger module '{fname}'",
)
- if func is not wall_clock:
- # Validate args and kwargs against the function signature
- # (but not for wall_clock because it's a special case).
- try:
- signature(func).bind(*fctx.func_args, **fctx.func_kwargs)
- except TypeError as exc:
- raise XtriggerConfigError(
- label, fname, f"{fctx.get_signature()}: {exc}"
- )
+
+ # Validate args and kwargs against the function signature
+ sig_str = fctx.get_signature()
+ try:
+ bound_args = signature(func).bind(
+ *fctx.func_args, **fctx.func_kwargs
+ )
+ except TypeError as exc:
+ raise XtriggerConfigError(label, f"{sig_str}: {exc}")
+ # Specific xtrigger.validate(), if available.
+ XtriggerManager.try_xtrig_validate_func(
+ label, fname, fdir, bound_args, sig_str
+ )
# Check any string templates in the function arg values (note this
# won't catch bad task-specific values - which are added dynamically).
@@ -311,9 +313,7 @@ def check_xtrigger(
template_vars.add(TemplateVariables(match))
except ValueError:
raise XtriggerConfigError(
- label,
- fname,
- f"Illegal template in xtrigger: {match}",
+ label, f"Illegal template in xtrigger: {match}",
)
# check for deprecated template variables
@@ -329,6 +329,30 @@ def check_xtrigger(
f' {", ".join(t.value for t in deprecated_variables)}'
)
+ @staticmethod
+ def try_xtrig_validate_func(
+ label: str,
+ fname: str,
+ fdir: str,
+ bound_args: 'BoundArguments',
+ signature_str: str,
+ ):
+ """Call an xtrigger's `validate()` function if it is implemented.
+
+ Raise XtriggerConfigError if validation fails.
+ """
+ try:
+ xtrig_validate_func = get_xtrig_func(fname, 'validate', fdir)
+ except (AttributeError, ImportError):
+ return
+ bound_args.apply_defaults()
+ try:
+ xtrig_validate_func(bound_args.arguments)
+ except Exception as exc: # Note: catch all errors
+ raise XtriggerConfigError(
+ label, f"{signature_str} validation failed: {exc}"
+ )
+
def add_trig(self, label: str, fctx: 'SubFuncContext', fdir: str) -> None:
"""Add a new xtrigger function.
@@ -410,20 +434,18 @@ def get_xtrig_ctx(
args = []
kwargs = {}
if ctx.func_name == "wall_clock":
- if "trigger_time" in ctx.func_kwargs:
+ if "trigger_time" in ctx.func_kwargs: # noqa: SIM401 (readabilty)
# Internal (retry timer): trigger_time already set.
kwargs["trigger_time"] = ctx.func_kwargs["trigger_time"]
- elif "offset" in ctx.func_kwargs: # noqa: SIM106
+ else:
# External (clock xtrigger): convert offset to trigger_time.
# Datetime cycling only.
kwargs["trigger_time"] = itask.get_clock_trigger_time(
itask.point,
- ctx.func_kwargs["offset"]
- )
- else:
- # Should not happen!
- raise ValueError(
- "wall_clock function kwargs needs trigger time or offset"
+ ctx.func_kwargs.get(
+ "offset",
+ ctx.func_args[0] if ctx.func_args else None
+ )
)
else:
# Other xtrig functions: substitute template values.
@@ -455,7 +477,7 @@ def call_xtriggers_async(self, itask: 'TaskProxy'):
if sig in self.sat_xtrig:
# Already satisfied, just update the task
itask.state.xtriggers[label] = True
- elif wall_clock(*ctx.func_args, **ctx.func_kwargs):
+ elif _wall_clock(*ctx.func_args, **ctx.func_kwargs):
# Newly satisfied
itask.state.xtriggers[label] = True
self.sat_xtrig[sig] = {}
diff --git a/cylc/flow/xtriggers/echo.py b/cylc/flow/xtriggers/echo.py
index 0297545e935..d200e2389bb 100644
--- a/cylc/flow/xtriggers/echo.py
+++ b/cylc/flow/xtriggers/echo.py
@@ -16,10 +16,9 @@
"""A Cylc xtrigger function."""
+from typing import Any, Dict, Tuple
from cylc.flow.exceptions import WorkflowConfigError
-from typing import Tuple
-
def echo(*args, **kwargs) -> Tuple:
"""Print arguments to stdout, return kwargs['succeed'] and kwargs.
@@ -48,15 +47,18 @@ def echo(*args, **kwargs) -> Tuple:
return kwargs["succeed"], kwargs
-def validate(f_args, f_kwargs, f_signature):
-
+def validate(all_args: Dict[str, Any]):
"""
Validate the xtrigger function arguments parsed from the workflow config.
This is separate from the xtrigger to allow parse-time validation.
"""
- if "succeed" not in f_kwargs or not isinstance(f_kwargs["succeed"], bool):
- raise WorkflowConfigError(
- f"Requires 'succeed=True/False' arg: {f_signature}"
- )
+ # NOTE: with (*args, **kwargs) pattern, all_args looks like:
+ # {
+ # 'args': (arg1, arg2, ...),
+ # 'kwargs': {kwarg1: val, kwarg2: val, ...}
+ # }
+ succeed = all_args['kwargs'].get("succeed")
+ if not isinstance(succeed, bool):
+ raise WorkflowConfigError("Requires 'succeed=True/False' arg")
diff --git a/cylc/flow/xtriggers/wall_clock.py b/cylc/flow/xtriggers/wall_clock.py
index c18ea2e5d2b..d5d1a009154 100644
--- a/cylc/flow/xtriggers/wall_clock.py
+++ b/cylc/flow/xtriggers/wall_clock.py
@@ -17,56 +17,53 @@
"""xtrigger function to trigger off of a wall clock time."""
from time import time
+from typing import Any, Dict
from cylc.flow.cycling.iso8601 import interval_parse
from cylc.flow.exceptions import WorkflowConfigError
-def wall_clock(trigger_time=None):
- """Return True after the desired wall clock time, False.
+def wall_clock(offset: str = 'PT0S'):
+ """Trigger at a specific real "wall clock" time relative to the cycle point
+ in the graph.
+
+ Clock triggers, unlike other trigger functions, are executed synchronously
+ in the main process.
+
+ Args:
+ offset:
+ ISO 8601 interval to wait after the cycle point is reached in real
+ time before triggering. May be negative, in which case it will
+ trigger before the real time reaches the cycle point.
+ """
+ # NOTE: This is just a placeholder for the actual implementation.
+ # This is only used for validating the signature and for autodocs.
+ ...
+
+
+def _wall_clock(trigger_time: int) -> bool:
+ """Actual implementation of wall_clock.
+
+ Return True after the desired wall clock time, or False before.
Args:
- trigger_time (int):
+ trigger_time:
Trigger time as seconds since Unix epoch.
"""
return time() > trigger_time
-def validate(f_args, f_kwargs, f_signature):
+def validate(args: Dict[str, Any]):
"""Validate and manipulate args parsed from the workflow config.
+ NOTE: the xtrigger signature is different to the function signature above
+
wall_clock() # infer zero interval
wall_clock(PT1H)
wall_clock(offset=PT1H)
The offset must be a valid ISO 8601 interval.
-
- If f_args used, convert to f_kwargs for clarity.
-
"""
-
- n_args = len(f_args)
- n_kwargs = len(f_kwargs)
-
- if n_args + n_kwargs > 1:
- raise WorkflowConfigError(f"Too many args: {f_signature}")
-
- if n_kwargs:
- # sole kwarg must be "offset"
- kw = next(iter(f_kwargs))
- if kw != "offset":
- raise WorkflowConfigError(f"Illegal arg '{kw}': {f_signature}")
-
- elif n_args:
- # convert to kwarg
- f_kwargs["offset"] = f_args[0]
- del f_args[0]
-
- else:
- # no args, infer zero interval
- f_kwargs["offset"] = "P0Y"
-
- # must be a valid interval
try:
- interval_parse(f_kwargs["offset"])
+ interval_parse(args["offset"])
except (ValueError, AttributeError):
- raise WorkflowConfigError(f"Invalid offset: {f_signature}")
+ raise WorkflowConfigError(f"Invalid offset: {args['offset']}")
diff --git a/cylc/flow/xtriggers/xrandom.py b/cylc/flow/xtriggers/xrandom.py
index 2ceaed35a9f..867035d46f8 100644
--- a/cylc/flow/xtriggers/xrandom.py
+++ b/cylc/flow/xtriggers/xrandom.py
@@ -16,7 +16,7 @@
from random import random, randint
from time import sleep
-from typing import Any, Dict, List, Tuple
+from typing import Any, Dict, Tuple
from cylc.flow.exceptions import WorkflowConfigError
@@ -26,8 +26,8 @@
def xrandom(
- percent: float, secs: int = 0, _: 'Any' = None
-) -> 'Tuple[bool, Dict]':
+ percent: float, secs: int = 0, _: Any = None
+) -> Tuple[bool, Dict[str, str]]:
"""Random xtrigger, with configurable sleep and percent success.
Sleep for ``sec`` seconds, and report satisfied with ``percent``
@@ -75,7 +75,7 @@ def xrandom(
(True, {'COLOR': 'orange', 'SIZE': 'small'})
Returns:
- Tuple, containing:
+ tuple: (satisfied, results)
satisfied:
True if ``satisfied`` else ``False``.
@@ -90,52 +90,31 @@ def xrandom(
"""
sleep(float(secs))
results = {}
- satisfied = random() < float(percent) / 100 # nosec
+ satisfied = random() < float(percent) / 100 # nosec: B311
if satisfied:
results = {
- 'COLOR': COLORS[randint(0, len(COLORS) - 1)], # nosec
- 'SIZE': SIZES[randint(0, len(SIZES) - 1)] # nosec
+ 'COLOR': COLORS[randint(0, len(COLORS) - 1)], # nosec: B311
+ 'SIZE': SIZES[randint(0, len(SIZES) - 1)] # nosec: B311
}
return satisfied, results
-def validate(f_args: List[str], f_kwargs: Dict[str, Any], f_signature: str):
+def validate(args: Dict[str, Any]):
"""Validate and manipulate args parsed from the workflow config.
The rules for args are:
* percent: Must be 0 ≤ x ≤ 100
* secs: Must be an integer.
"""
- # convert to kwarg for clarity
- f_kwargs["percent"] = f_args[0]
- del f_args[0]
-
- should_raise = False
-
- try:
- percent = f_kwargs['percent']
- percent = float(percent)
- except ValueError:
- should_raise = True
- else:
- if (
- not isinstance(percent, (float, int))
- or percent < 0
- or percent > 100
- ):
- should_raise = True
- if should_raise:
+ percent = args['percent']
+ if (
+ not isinstance(percent, (float, int))
+ or not (0 <= percent <= 100)
+ ):
raise WorkflowConfigError(
- f"'percent' should be a float between 0 and 100: {f_signature}")
+ "'percent' should be a float between 0 and 100"
+ )
- try:
- secs = f_kwargs.get('secs', 0)
- except ValueError:
- should_raise = True
- else:
- if not isinstance(secs, int):
- should_raise = True
-
- if should_raise:
- raise WorkflowConfigError(
- f"'secs' should be an integer: {f_signature}")
+ secs = args['secs']
+ if not isinstance(secs, int):
+ raise WorkflowConfigError("'secs' should be an integer")
diff --git a/tests/functional/runahead/04-no-final-cycle.t b/tests/functional/runahead/04-no-final-cycle.t
index 1c9609033df..ae1ecebc500 100644
--- a/tests/functional/runahead/04-no-final-cycle.t
+++ b/tests/functional/runahead/04-no-final-cycle.t
@@ -32,7 +32,7 @@ TEST_NAME=${TEST_NAME_BASE}-check-fail
DB="$RUN_DIR/${WORKFLOW_NAME}/log/db"
TASKS=$(sqlite3 "${DB}" 'select count(*) from task_states where status=="failed"')
# manual comparison for the test
-if ((TASKS==4)); then
+if ((TASKS==2)); then
ok "${TEST_NAME}"
else
fail "${TEST_NAME}"
diff --git a/tests/functional/runahead/no_final/flow.cylc b/tests/functional/runahead/no_final/flow.cylc
index 89e23f7af1c..3dd1d7e9734 100644
--- a/tests/functional/runahead/no_final/flow.cylc
+++ b/tests/functional/runahead/no_final/flow.cylc
@@ -1,11 +1,13 @@
-# Should shutdown stalled with 4 failed tasks.
+# Should shutdown stalled with 2 failed tasks due to P1 runahead limit
[scheduler]
cycle point time zone = Z
[[events]]
+ abort on stall timeout = True
+ stall timeout = PT0S
abort on inactivity timeout = True
inactivity timeout = PT20S
[scheduling]
- runahead limit = P3
+ runahead limit = P1
initial cycle point = 20100101T00
[[xtriggers]]
never = wall_clock(P100Y)
diff --git a/tests/functional/spawn-on-demand/15-stop-flow-3/flow.cylc b/tests/functional/spawn-on-demand/15-stop-flow-3/flow.cylc
index fb796f784f5..4731fee49ea 100644
--- a/tests/functional/spawn-on-demand/15-stop-flow-3/flow.cylc
+++ b/tests/functional/spawn-on-demand/15-stop-flow-3/flow.cylc
@@ -12,11 +12,11 @@
abort on inactivity timeout = True
[scheduling]
[[xtriggers]]
- x = xrandom("0", "10") # never succeeds
+ x = xrandom(0, 10) # never succeeds
[[graph]]
R1 = """
@x => never
- c => never_again
+ c => never_again
"""
[runtime]
[[c]]
diff --git a/tests/functional/validate/35-pass-special-tasks-non-word-names.t b/tests/functional/validate/35-pass-special-tasks-non-word-names.t
deleted file mode 100755
index c523ea25b78..00000000000
--- a/tests/functional/validate/35-pass-special-tasks-non-word-names.t
+++ /dev/null
@@ -1,40 +0,0 @@
-#!/usr/bin/env bash
-# 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 .
-#-------------------------------------------------------------------------------
-# Test validation of special tasks names with non-word characters
-. "$(dirname "$0")/test_header"
-set_test_number 1
-cat >'flow.cylc' <<'__FLOW_CONFIG__'
-[scheduling]
- initial cycle point = 20200202
- final cycle point = 20300303
- [[special tasks]]
- clock-trigger = t-1, t+1, t%1, t@1
- [[graph]]
- P1D = """
- t-1
- t+1
- t%1
- t@1
- """
-
-[runtime]
- [[t-1, t+1, t%1, t@1]]
- script = true
-__FLOW_CONFIG__
-run_ok "${TEST_NAME_BASE}" cylc validate "${PWD}"
-exit
diff --git a/tests/functional/validate/69-bare-clock-xtrigger.t b/tests/functional/validate/69-bare-clock-xtrigger.t
deleted file mode 100644
index 4408a064dba..00000000000
--- a/tests/functional/validate/69-bare-clock-xtrigger.t
+++ /dev/null
@@ -1,32 +0,0 @@
-#!/usr/bin/env bash
-# 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 .
-
-# Test that undeclared zero-offset clock xtriggers are allowed.
-. "$(dirname "$0")/test_header"
-
-set_test_number 1
-
-cat >'flow.cylc' <<'__FLOW_CONFIG__'
-[scheduler]
- allow implicit tasks = True
-[scheduling]
- initial cycle point = now
- [[graph]]
- T00 = "@wall_clock => foo"
-__FLOW_CONFIG__
-
-run_ok "${TEST_NAME_BASE}-val" cylc validate .
diff --git a/tests/integration/test_config.py b/tests/integration/test_config.py
index 11d7bd483e1..581ec4d83fb 100644
--- a/tests/integration/test_config.py
+++ b/tests/integration/test_config.py
@@ -362,17 +362,31 @@ def test_xtrig_validation_wall_clock(
'scheduler': {'allow implicit tasks': True},
'scheduling': {
'initial cycle point': '1012',
- 'xtriggers': {'myxt': 'wall_clock(offset=PT755MH)'},
+ 'xtriggers': {'myxt': 'wall_clock(offset=PT7MH)'},
'graph': {'R1': '@myxt => foo'},
}
})
- with pytest.raises(
- WorkflowConfigError,
- match=r'Invalid offset: wall_clock\(offset=PT755MH\)'
- ):
+ with pytest.raises(WorkflowConfigError, match=(
+ r'\[@myxt\] wall_clock\(offset=PT7MH\) validation failed: '
+ r'Invalid offset: PT7MH'
+ )):
validate(id_)
+def test_xtrig_implicit_wall_clock(flow: Fixture, validate: Fixture):
+ """Test @wall_clock is allowed in graph without explicit
+ xtrigger definition.
+ """
+ wid = flow({
+ 'scheduler': {'allow implicit tasks': True},
+ 'scheduling': {
+ 'initial cycle point': '2024',
+ 'graph': {'R1': '@wall_clock => foo'},
+ }
+ })
+ validate(wid)
+
+
def test_xtrig_validation_echo(
flow: 'Fixture',
validate: 'Fixture',
@@ -390,7 +404,7 @@ def test_xtrig_validation_echo(
})
with pytest.raises(
WorkflowConfigError,
- match=r'Requires \'succeed=True/False\' arg: echo()'
+ match=r'echo.* Requires \'succeed=True/False\' arg'
):
validate(id_)
@@ -411,8 +425,8 @@ def test_xtrig_validation_xrandom(
}
})
with pytest.raises(
- WorkflowConfigError,
- match=r"'percent' should be a float between 0 and 100:"
+ XtriggerConfigError,
+ match=r"'percent' should be a float between 0 and 100"
):
validate(id_)
@@ -432,23 +446,16 @@ def test_xtrig_validation_custom(
# mock our own exception, xtrigger and xtrigger
# validation functions and inject these into the
# appropriate locations:
- GreenExc = type('Green', (Exception,), {})
-
def kustom_xt(feature):
return True, {}
- def kustom_validate(args, kwargs, sig):
- raise GreenExc('This is only a test.')
+ def kustom_validate(args):
+ raise Exception('This is only a test.')
- # Patch xtrigger func
+ # Patch xtrigger func & its validate func
monkeypatch.setattr(
'cylc.flow.xtrigger_mgr.get_xtrig_func',
- lambda *args: kustom_xt,
- )
- # Patch xtrigger's validate func
- monkeypatch.setattr(
- 'cylc.flow.config.get_xtrig_func',
- lambda *args: kustom_validate if "validate" in args else ''
+ lambda *args: kustom_validate if "validate" in args else kustom_xt
)
id_ = flow({
@@ -461,24 +468,53 @@ def kustom_validate(args, kwargs, sig):
})
Path(id_)
- with pytest.raises(GreenExc, match=r'This is only a test.'):
+ with pytest.raises(XtriggerConfigError, match=r'This is only a test.'):
validate(id_)
+@pytest.mark.parametrize('xtrig_call, expected_msg', [
+ pytest.param(
+ 'xrandom()',
+ r"xrandom.* missing a required argument: 'percent'",
+ id="missing-arg"
+ ),
+ pytest.param(
+ 'wall_clock(alan_grant=1)',
+ r"wall_clock.* unexpected keyword argument 'alan_grant'",
+ id="unexpected-arg"
+ ),
+])
def test_xtrig_signature_validation(
- flow: 'Fixture',
- validate: 'Fixture',
+ flow: 'Fixture', validate: 'Fixture',
+ xtrig_call: str, expected_msg: str
):
"""Test automatic xtrigger function signature validation."""
id_ = flow({
'scheduler': {'allow implicit tasks': True},
'scheduling': {
- 'xtriggers': {'myxt': 'xrandom()'},
+ 'initial cycle point': '2024',
+ 'xtriggers': {'myxt': xtrig_call},
'graph': {'R1': '@myxt => foo'},
}
})
- with pytest.raises(
- XtriggerConfigError,
- match=r"xrandom\(\): missing a required argument: 'percent'"
- ):
+ with pytest.raises(XtriggerConfigError, match=expected_msg):
validate(id_)
+
+
+def test_special_task_non_word_names(flow: Fixture, validate: Fixture):
+ """Test validation of special tasks names with non-word characters"""
+ wid = flow({
+ 'scheduling': {
+ 'initial cycle point': '2020',
+ 'special tasks': {
+ 'clock-trigger': 't-1, t+1, t%1, t@1',
+ },
+ 'graph': {
+ 'P1D': 't-1 & t+1 & t%1 & t@1',
+ },
+ },
+ 'runtime': {
+ 't-1, t+1, t%1, t@1': {'script': True},
+ },
+ })
+ validate(wid)
diff --git a/tests/integration/test_xtrigger_mgr.py b/tests/integration/test_xtrigger_mgr.py
index c116e6968a5..612049163cc 100644
--- a/tests/integration/test_xtrigger_mgr.py
+++ b/tests/integration/test_xtrigger_mgr.py
@@ -21,6 +21,7 @@
from cylc.flow.pathutil import get_workflow_run_dir
+
async def test_2_xtriggers(flow, start, scheduler, monkeypatch):
"""Test that if an itask has 2 wall_clock triggers with different
offsets that xtrigger manager gets both of them.
diff --git a/tests/unit/test_xtrigger_mgr.py b/tests/unit/test_xtrigger_mgr.py
index 9b497d8e5ab..78f8fe6d969 100644
--- a/tests/unit/test_xtrigger_mgr.py
+++ b/tests/unit/test_xtrigger_mgr.py
@@ -44,7 +44,7 @@ def test_extract_templates():
)
-def test_check_xtrigger(xtrigger_mgr):
+def test_add_xtrigger(xtrigger_mgr):
"""Test for adding an xtrigger."""
xtrig = SubFuncContext(
label="echo",
@@ -84,9 +84,12 @@ def test_check_xtrigger_with_unknown_params(xtrigger_mgr):
label="echo",
func_name="echo",
func_args=[1, "name", "%(what_is_this)s"],
- func_kwargs={"location": "soweto"}
+ func_kwargs={"succeed": True}
)
- with pytest.raises(XtriggerConfigError):
+ with pytest.raises(
+ XtriggerConfigError,
+ match="Illegal template in xtrigger: what_is_this"
+ ):
xtrigger_mgr.check_xtrigger("xtrig", xtrig, 'fdir')
@@ -96,7 +99,7 @@ def test_check_xtrigger_with_deprecated_params(xtrigger_mgr, caplog):
label="echo",
func_name="echo",
func_args=[1, "name", "%(suite_name)s"],
- func_kwargs={"location": "soweto"}
+ func_kwargs={"succeed": True}
)
caplog.set_level(logging.WARNING, CYLC_LOG)
xtrigger_mgr.check_xtrigger("xtrig", xtrig, 'fdir')
@@ -139,7 +142,6 @@ def test_housekeeping_nothing_satisfied(xtrigger_mgr):
def test_housekeeping_with_xtrigger_satisfied(xtrigger_mgr):
"""The housekeeping method makes sure only satisfied xtrigger function
are kept."""
- xtrigger_mgr.check_xtrigger = lambda *a, **k: True # Ignore validation
xtrig = SubFuncContext(
label="get_name",
func_name="get_name",
@@ -171,7 +173,6 @@ def test_housekeeping_with_xtrigger_satisfied(xtrigger_mgr):
def test__call_xtriggers_async(xtrigger_mgr):
"""Test _call_xtriggers_async"""
- xtrigger_mgr.check_xtrigger = lambda *a, **k: True # Ignore validation
# the echo1 xtrig (not satisfied)
echo1_xtrig = SubFuncContext(
label="echo1",
diff --git a/tests/unit/xtriggers/test_echo.py b/tests/unit/xtriggers/test_echo.py
index b4bc104adac..7b1f4d140ee 100644
--- a/tests/unit/xtriggers/test_echo.py
+++ b/tests/unit/xtriggers/test_echo.py
@@ -20,18 +20,19 @@
from pytest import param
-def test_validate_good_path():
- assert validate(
- [], {'succeed': False, 'egg': 'fried', 'potato': 'baked'}, 'succeed'
- ) is None
+def test_validate_good():
+ validate({
+ 'args': (),
+ 'kwargs': {'succeed': False, 'egg': 'fried', 'potato': 'baked'}
+ })
@pytest.mark.parametrize(
- 'args, kwargs', (
- param([False], {}, id='no-kwarg'),
- param([], {'spud': 'mashed'}, id='no-succeed-kwarg'),
+ 'all_args', (
+ param({'args': (False,), 'kwargs': {}}, id='no-kwarg'),
+ param({'args': (), 'kwargs': {'spud': 'mashed'}}, id='no-succeed-kwarg'),
)
)
-def test_validate_exceptions(args, kwargs):
+def test_validate_exceptions(all_args):
with pytest.raises(WorkflowConfigError, match='^Requires'):
- validate(args, kwargs, 'blah')
\ No newline at end of file
+ validate(all_args)
diff --git a/tests/unit/xtriggers/test_wall_clock.py b/tests/unit/xtriggers/test_wall_clock.py
index 6dc1cb2b6f8..44bba2619fc 100644
--- a/tests/unit/xtriggers/test_wall_clock.py
+++ b/tests/unit/xtriggers/test_wall_clock.py
@@ -14,13 +14,25 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see .
-from cylc.flow.exceptions import WorkflowConfigError
-from cylc.flow.xtriggers.wall_clock import validate
+from cylc.flow.xtriggers.wall_clock import _wall_clock, validate
from metomi.isodatetime.parsers import DurationParser
import pytest
from pytest import param
+@pytest.mark.parametrize('trigger_time, expected', [
+ (499, True),
+ (500, False),
+])
+def test_wall_clock(
+ monkeypatch: pytest.MonkeyPatch, trigger_time: int, expected: bool
+):
+ monkeypatch.setattr(
+ 'cylc.flow.xtriggers.wall_clock.time', lambda: 500
+ )
+ assert _wall_clock(trigger_time) == expected
+
+
@pytest.fixture
def monkeypatch_interval_parser(monkeypatch):
"""Interval parse only works normally if a WorkflowSpecifics
@@ -32,20 +44,18 @@ def monkeypatch_interval_parser(monkeypatch):
)
-def test_validate_good_path(monkeypatch_interval_parser):
- assert validate([], {}, 'Alles Gut') is None
+def test_validate_good(monkeypatch_interval_parser):
+ validate({'offset': 'PT1H'})
@pytest.mark.parametrize(
- 'args, kwargs, err', (
- param([1, 2], {}, "^Too", id='too-many-args'),
- param([], {'egg': 12}, "^Illegal", id='illegal-arg'),
- param([1], {}, "^Invalid", id='invalid-offset-int'),
- param([], {'offset': 'Zaphod'}, "^Invalid", id='invalid-offset-str'),
+ 'args, err', (
+ param({'offset': 1}, "^Invalid", id='invalid-offset-int'),
+ param({'offset': 'Zaphod'}, "^Invalid", id='invalid-offset-str'),
)
)
def test_validate_exceptions(
- monkeypatch_interval_parser, args, kwargs, err
+ monkeypatch_interval_parser, args, err
):
- with pytest.raises(WorkflowConfigError, match=err):
- validate(args, kwargs, 'Alles Gut')
+ with pytest.raises(Exception, match=err):
+ validate(args)
diff --git a/tests/unit/xtriggers/test_xrandom.py b/tests/unit/xtriggers/test_xrandom.py
index 23fcd1b97fd..21a9fdb776d 100644
--- a/tests/unit/xtriggers/test_xrandom.py
+++ b/tests/unit/xtriggers/test_xrandom.py
@@ -21,19 +21,19 @@
from cylc.flow.exceptions import WorkflowConfigError
-def test_validate_good_path():
- assert validate([1], {'secs': 0, '_': 'HelloWorld'}, 'good_path') is None
+def test_validate_good():
+ validate({'percent': 1, 'secs': 0, '_': 'HelloWorld'})
@pytest.mark.parametrize(
- 'args, kwargs, err', (
- param(['foo'], {}, r"'percent", id='percent-not-numeric'),
- param([101], {}, r"'percent", id='percent>100'),
- param([-1], {}, r"'percent", id='percent<0'),
- param([100], {'secs': 1.1}, r"'secs'", id='secs-not-int'),
+ 'args, err', (
+ param({'percent': 'foo'}, r"'percent", id='percent-not-numeric'),
+ param({'percent': 101}, r"'percent", id='percent>100'),
+ param({'percent': -1}, r"'percent", id='percent<0'),
+ param({'percent': 100, 'secs': 1.1}, r"'secs'", id='secs-not-int'),
)
)
-def test_validate_exceptions(args, kwargs, err):
+def test_validate_exceptions(args, err):
"""Illegal args and kwargs cause a WorkflowConfigError raised."""
with pytest.raises(WorkflowConfigError, match=f'^{err}'):
- validate(args, kwargs, 'blah')
+ validate(args)