From bb7a981728f810f4215cf5cee1b6be9ea08eefc2 Mon Sep 17 00:00:00 2001
From: Hilary James Oliver <hilary.j.oliver@gmail.com>
Date: Sun, 11 Aug 2019 16:09:01 +1200
Subject: [PATCH 01/10] Make xtriggers unique to sequence.

---
 lib/cylc/config.py       |  65 ++++---------------
 lib/cylc/task_pool.py    |  15 ++---
 lib/cylc/task_state.py   |  29 +++++----
 lib/cylc/taskdef.py      |  31 +++++----
 lib/cylc/xtrigger_mgr.py | 135 +++++++++++++++++++--------------------
 5 files changed, 119 insertions(+), 156 deletions(-)

diff --git a/lib/cylc/config.py b/lib/cylc/config.py
index 0baa0774030..0fd666376d3 100644
--- a/lib/cylc/config.py
+++ b/lib/cylc/config.py
@@ -58,7 +58,6 @@
 from cylc.graphnode import GraphNodeParser, GraphNodeError
 from cylc.print_tree import print_tree
 from cylc.subprocctx import SubFuncContext
-from cylc.subprocpool import get_func
 from cylc.suite_srv_files_mgr import SuiteSrvFilesManager
 from cylc.taskdef import TaskDef, TaskDefError
 from cylc.task_id import TaskID
@@ -155,7 +154,6 @@ def __init__(
             self.xtrigger_mgr = XtriggerManager(self.suite)
         else:
             self.xtrigger_mgr = xtrigger_mgr
-        self.xtriggers = {}
         self.suite_polling_tasks = {}
         self._last_graph_raw_id = None
         self._last_graph_raw_edges = []
@@ -1741,11 +1739,19 @@ def generate_triggers(self, lexpression, left_nodes, right, seq,
             dependency = Dependency(expr_list, set(triggers.values()), suicide)
             self.taskdefs[right].add_dependency(dependency, seq)
 
-        # Record xtrigger labels for each task name.
-        if right not in self.xtriggers:
-            self.xtriggers[right] = xtrig_labels
-        else:
-            self.xtriggers[right] = self.xtriggers[right].union(xtrig_labels)
+        for label in xtrig_labels:
+            try:
+                xtrig = self.cfg['scheduling']['xtriggers'][label]
+            except KeyError:
+                if label == 'wall_clock':
+                    # Allow predefined zero-offset wall clock xtrigger.
+                    xtrig = SubFuncContext(
+                        'wall_clock', 'wall_clock', [], {})
+                else:
+                    raise SuiteConfigError(
+                        "ERROR, undefined xtrigger label: %s" % label)
+            self.xtrigger_mgr.add_trig(label, xtrig, self.fdir)
+            self.taskdefs[right].add_xtrig_label(label, seq)
 
     def get_actual_first_point(self, start_point):
         """Get actual first cycle point for the suite
@@ -2086,51 +2092,6 @@ def load_graph(self):
             self._proc_triggers(
                 parser.triggers, parser.original, seq, task_triggers)
 
-        xtcfg = self.cfg['scheduling']['xtriggers']
-        # Taskdefs just know xtrigger labels.
-        for task_name, xt_labels in self.xtriggers.items():
-            for label in xt_labels:
-                try:
-                    xtrig = xtcfg[label]
-                except KeyError:
-                    if label == 'wall_clock':
-                        # Allow predefined zero-offset wall clock xtrigger.
-                        xtrig = SubFuncContext(
-                            'wall_clock', 'wall_clock', [], {})
-                    else:
-                        raise SuiteConfigError(
-                            "ERROR, undefined xtrigger label: %s" % label)
-                if xtrig.func_name.startswith('wall_clock'):
-                    self.xtrigger_mgr.add_clock(label, xtrig)
-                    # Replace existing xclock if the new offset is larger.
-                    try:
-                        offset = get_interval(xtrig.func_kwargs['offset'])
-                    except KeyError:
-                        offset = 0
-                    old_label = self.taskdefs[task_name].xclock_label
-                    if old_label is None:
-                        self.taskdefs[task_name].xclock_label = label
-                    else:
-                        old_xtrig = self.xtrigger_mgr.clockx_map[old_label]
-                        old_offset = get_interval(
-                            old_xtrig.func_kwargs['offset'])
-                        if offset > old_offset:
-                            self.taskdefs[task_name].xclock_label = label
-                else:
-                    try:
-                        if not callable(get_func(xtrig.func_name, self.fdir)):
-                            raise SuiteConfigError(
-                                "ERROR, "
-                                "xtrigger function not callable: %s" %
-                                xtrig.func_name)
-                    except (ImportError, AttributeError):
-                        raise SuiteConfigError(
-                            "ERROR, "
-                            "xtrigger function not found: %s" %
-                            xtrig.func_name)
-                    self.xtrigger_mgr.add_trig(label, xtrig)
-                    self.taskdefs[task_name].xtrig_labels.add(label)
-
         # Detect use of xtrigger names with '@' prefix (creates a task).
         overlap = set(self.taskdefs.keys()).intersection(
             self.cfg['scheduling']['xtriggers'].keys())
diff --git a/lib/cylc/task_pool.py b/lib/cylc/task_pool.py
index 26aba9bfa9c..6547a7cfa50 100644
--- a/lib/cylc/task_pool.py
+++ b/lib/cylc/task_pool.py
@@ -1263,16 +1263,13 @@ def get_task_requisites(self, items, list_prereqs=False):
                 else:
                     extras['External trigger "%s"' % trig] = 'NOT satisfied'
             for label, satisfied in itask.state.xtriggers.items():
+                extra = 'xtrigger "%s = %s"' % (
+                    label, self.xtrigger_mgr.get_xtrig_ctx(
+                        itask, label).get_signature())
                 if satisfied:
-                    extras['xtrigger "%s"' % label] = 'satisfied'
+                    extras[extra] = 'satisfied'
                 else:
-                    extras['xtrigger "%s"' % label] = 'NOT satisfied'
-            if itask.state.xclock is not None:
-                label, satisfied = itask.state.xclock
-                if satisfied:
-                    extras['xclock "%s"' % label] = 'satisfied'
-                else:
-                    extras['xclock "%s"' % label] = 'NOT satisfied'
+                    extras[extra] = 'NOT satisfied'
 
             outputs = []
             for _, msg, is_completed in itask.state.outputs.get_all():
@@ -1289,8 +1286,6 @@ def check_xtriggers(self):
         itasks = self.get_tasks()
         self.xtrigger_mgr.collate(itasks)
         for itask in itasks:
-            if itask.state.xclock is not None:
-                self.xtrigger_mgr.satisfy_xclock(itask)
             if itask.state.xtriggers:
                 self.xtrigger_mgr.satisfy_xtriggers(itask, self.proc_pool)
 
diff --git a/lib/cylc/task_state.py b/lib/cylc/task_state.py
index e9621c360b3..724ed75cf6c 100644
--- a/lib/cylc/task_state.py
+++ b/lib/cylc/task_state.py
@@ -218,10 +218,6 @@ class TaskState(object):
             List of prerequisites that will cause the task to suicide.
         .time_updated (str):
             Time string of latest update time.
-        .xclock (tuple):
-            A tuple (clock_label (str), is_done (boolean)) to indicate if a
-            clock trigger is satisfied or not. Set to `None` if the task has no
-            clock trigger.
         .xtriggers (dict):
             xtriggers as {trigger (str): satisfied (boolean), ...}.
         ._is_satisfied (boolean):
@@ -242,7 +238,6 @@ class TaskState(object):
         "status",
         "suicide_prerequisites",
         "time_updated",
-        "xclock",
         "xtriggers",
         "_is_satisfied",
         "_suicide_is_satisfied",
@@ -274,12 +269,7 @@ def __init__(self, tdef, point, status, hold_swap):
 
         # xtriggers (represented by labels) satisfied or not
         self.xtriggers = {}
-        for label in tdef.xtrig_labels:
-            self.xtriggers[label] = False
-        if tdef.xclock_label:
-            self.xclock = (tdef.xclock_label, False)
-        else:
-            self.xclock = None
+        self._add_xtriggers(point, tdef)
 
         # Message outputs.
         self.outputs = TaskOutputs(tdef)
@@ -301,9 +291,7 @@ def satisfy_me(self, all_task_outputs):
                     self._suicide_is_satisfied = None
 
     def xtriggers_all_satisfied(self):
-        """Return True if xclock and all xtriggers are satisfied."""
-        if self.xclock is not None and not self.xclock[1]:
-            return False
+        """Return True if all xtriggers are satisfied."""
         return all(self.xtriggers.values())
 
     def prerequisites_are_all_satisfied(self):
@@ -528,3 +516,16 @@ def _add_prerequisites(self, point, tdef):
                          p_prev < tdef.start_point)
                 cpre.set_condition(tdef.name)
                 self.prerequisites.append(cpre)
+
+    def _add_xtriggers(self, point, tdef):
+        """Add task xtriggers valid for the current sequence.
+
+        Initialize each one to unsatisfied.
+        """
+        # Triggers for sequence_i only used if my cycle point is a
+        # valid member of sequence_i's sequence of cycle points.
+        for sequence, xtrig_labels in tdef.xtrig_labels.items():
+            if not sequence.is_valid(point):
+                continue
+            for xtrig_label in xtrig_labels:
+                self.xtriggers[xtrig_label] = False
diff --git a/lib/cylc/taskdef.py b/lib/cylc/taskdef.py
index 373da4ae107..2c135a3e99c 100644
--- a/lib/cylc/taskdef.py
+++ b/lib/cylc/taskdef.py
@@ -47,8 +47,7 @@ class TaskDef(object):
         "intercycle_offsets", "sequential", "is_coldstart",
         "suite_polling_cfg", "clocktrigger_offset", "expiration_offset",
         "namespace_hierarchy", "dependencies", "outputs", "param_var",
-        "external_triggers", "xtrig_labels", "xclock_label",
-        "name", "elapsed_times"]
+        "external_triggers", "xtrig_labels", "name", "elapsed_times"]
 
     # Store the elapsed times for a maximum of 10 cycles
     MAX_LEN_ELAPSED_TIMES = 10
@@ -78,12 +77,7 @@ def __init__(self, name, rtcfg, run_mode, start_point, spawn_ahead):
         self.outputs = set()
         self.param_var = {}
         self.external_triggers = []
-        self.xtrig_labels = set()
-        self.xclock_label = None
-        # Note a task can only have one clock xtrigger - if it depends on
-        # several we just keep the label of the one with the largest offset
-        # (this is determined and set during suite config parsing, to avoid
-        # storing the offset here in the taskdef).
+        self.xtrig_labels = {}  # {sequence: [labels]}
 
         self.name = name
         self.elapsed_times = deque(maxlen=self.MAX_LEN_ELAPSED_TIMES)
@@ -97,9 +91,24 @@ def add_dependency(self, dependency, sequence):
                 dependency applies.
 
         """
-        if sequence not in self.dependencies:
-            self.dependencies[sequence] = []
-        self.dependencies[sequence].append(dependency)
+        try:
+            self.dependencies[sequence].append(dependency)
+        except KeyError:
+            self.dependencies[sequence] = [dependency]
+
+    def add_xtrig_label(self, xtrig_label, sequence):
+        """Add an xtrigger to a named sequence.
+
+        Args:
+            xtrig_label: The xtrigger label to add.
+            sequence (cylc.cycling.SequenceBase): The sequence for which this
+                xtrigger applies.
+
+        """
+        try:
+            self.xtrig_labels[sequence].append(xtrig_label)
+        except KeyError:
+            self.xtrig_labels[sequence] = [xtrig_label]
 
     def add_sequence(self, sequence):
         """Add a sequence."""
diff --git a/lib/cylc/xtrigger_mgr.py b/lib/cylc/xtrigger_mgr.py
index 0104df610b1..b5e5101c616 100644
--- a/lib/cylc/xtrigger_mgr.py
+++ b/lib/cylc/xtrigger_mgr.py
@@ -25,6 +25,7 @@
 import cylc.flags
 from cylc.hostuserutil import get_user
 from cylc.xtriggers.wall_clock import wall_clock
+from cylc.subprocpool import get_func
 
 
 # Templates for string replacement in function arg values.
@@ -97,18 +98,14 @@ def __init__(
         """Initialize the xtrigger manager."""
         # Suite function and clock triggers by label.
         self.functx_map = {}
-        self.clockx_map = {}
         # When next to call a function, by signature.
         self.t_next_call = {}
         # Satisfied triggers and their function results, by signature.
         self.sat_xtrig = {}
-        # Signatures of satisfied clock triggers.
-        self.sat_xclock = []
         # Signatures of active functions (waiting on callback).
         self.active = []
         # All trigger and clock signatures in the current task pool.
         self.all_xtrig = []
-        self.all_xclock = []
 
         self.pflag = False
 
@@ -125,12 +122,24 @@ def __init__(
         self.broadcast_mgr = broadcast_mgr
         self.suite_source_dir = suite_source_dir
 
-    def add_clock(self, label, fctx):
-        """Add a new clock xtrigger."""
-        self.clockx_map[label] = fctx
+    def add_trig(self, label, fctx, fdir):
+        """Add a new xtrigger function.
 
-    def add_trig(self, label, fctx):
-        """Add a new xtrigger."""
+        Check the xtrigger function exists here (e.g. during validation).
+        """
+        try:
+            func = get_func(fctx.func_name, fdir)
+        except ImportError:
+            raise ImportError(
+                "ERROR: xtrigger module '%s' not found" % fctx.func_name)
+        except AttributeError:
+            raise AttributeError(
+                "ERROR: attribute '%s' not found in xtrigger module '%s'" % (
+                    fctx.func_name, fctx.func_name))
+        if not callable(func):
+            raise ValueError(
+                "ERROR: '%s' in xtrigger module '%s' is not callable" % (
+                    fctx.func_name, fctx.func_name))
         self.functx_map[label] = fctx
         # Check any string templates in the function arg values (note this
         # won't catch bad task-specific values - which are added dynamically).
@@ -153,70 +162,18 @@ def load_xtrigger_for_restart(self, row_idx, row):
         self.sat_xtrig[sig] = json.loads(results)
 
     def housekeep(self):
-        """Delete satisfied xtriggers and xclocks no longer needed."""
+        """Delete satisfied xtriggers no longer needed."""
         for sig in list(self.sat_xtrig):
             if sig not in self.all_xtrig:
                 del self.sat_xtrig[sig]
-        self.sat_xclock = [
-            sig for sig in self.sat_xclock if sig in self.all_xclock]
-
-    def satisfy_xclock(self, itask):
-        """Attempt to satisfy itask's clock trigger, if it has one."""
-        label, sig, ctx, satisfied = self._get_xclock(itask)
-        if satisfied:
-            return
-        if wall_clock(*ctx.func_args, **ctx.func_kwargs):
-            satisfied = True
-            itask.state.xclock = (label, True)
-            self.sat_xclock.append(sig)
-            LOG.info('clock xtrigger satisfied: %s = %s' % (label, str(ctx)))
 
-    def _get_xclock(self, itask, sig_only=False):
-        """(Internal helper method.)"""
-        label, satisfied = itask.state.xclock
-        ctx = deepcopy(self.clockx_map[label])
-        ctx.func_kwargs.update(
-            {
-                'point_as_seconds': itask.get_point_as_seconds(),
-            }
-        )
-        sig = ctx.get_signature()
-        if sig_only:
-            return sig
-        else:
-            return (label, sig, ctx, satisfied)
-
-    def _get_xtrig(self, itask, unsat_only=False, sigs_only=False):
+    def _get_xtrigs(self, itask, unsat_only=False, sigs_only=False):
         """(Internal helper method.)"""
         res = []
-        farg_templ = {}
-        farg_templ[TMPL_TASK_CYCLE_POINT] = str(itask.point)
-        farg_templ[TMPL_TASK_NAME] = str(itask.tdef.name)
-        farg_templ[TMPL_TASK_IDENT] = str(itask.identity)
-        farg_templ.update(self.farg_templ)
         for label, satisfied in itask.state.xtriggers.items():
             if unsat_only and satisfied:
                 continue
-            ctx = deepcopy(self.functx_map[label])
-            ctx.point = itask.point
-            kwargs = {}
-            args = []
-            # Replace legal string templates in function arg values.
-            for val in ctx.func_args:
-                try:
-                    val = val % farg_templ
-                except TypeError:
-                    pass
-                args.append(val)
-            for key, val in ctx.func_kwargs.items():
-                try:
-                    val = val % farg_templ
-                except TypeError:
-                    pass
-                kwargs[key] = val
-            ctx.func_args = args
-            ctx.func_kwargs = kwargs
-            ctx.update_command(self.suite_source_dir)
+            ctx = self.get_xtrig_ctx(itask, label)
             sig = ctx.get_signature()
             if sigs_only:
                 res.append(sig)
@@ -224,18 +181,58 @@ def _get_xtrig(self, itask, unsat_only=False, sigs_only=False):
                 res.append((label, sig, ctx, satisfied))
         return res
 
+    def get_xtrig_ctx(self, itask, label):
+        """Get a real function context from the template."""
+        farg_templ = {
+            TMPL_TASK_CYCLE_POINT: str(itask.point),
+            TMPL_TASK_NAME: str(itask.tdef.name),
+            TMPL_TASK_IDENT: str(itask.identity)
+        }
+        farg_templ.update(self.farg_templ)
+        ctx = deepcopy(self.functx_map[label])
+        ctx.point = itask.point
+        kwargs = {}
+        args = []
+        for val in ctx.func_args:
+            try:
+                val = val % farg_templ
+            except TypeError:
+                pass
+            args.append(val)
+        for key, val in ctx.func_kwargs.items():
+            try:
+                val = val % farg_templ
+            except TypeError:
+                pass
+            kwargs[key] = val
+        ctx.func_args = args
+        ctx.func_kwargs = kwargs
+        ctx.update_command(self.suite_source_dir)
+        return ctx
+
     def collate(self, itasks):
         """Get list of all current xtrigger sigs."""
         self.all_xtrig = []
-        self.all_xclock = []
         for itask in itasks:
-            self.all_xtrig += self._get_xtrig(itask, sigs_only=True)
-            if itask.state.xclock is not None:
-                self.all_xclock.append(self._get_xclock(itask, sig_only=True))
+            self.all_xtrig += self._get_xtrigs(itask, sigs_only=True)
 
     def satisfy_xtriggers(self, itask, proc_pool):
         """Attempt to satisfy itask's xtriggers."""
-        for label, sig, ctx, _ in self._get_xtrig(itask, unsat_only=True):
+        for label, sig, ctx, _ in self._get_xtrigs(itask, unsat_only=True):
+            if sig.startswith("wall_clock"):
+                # Special case: synchronous clock check.
+                ctx.func_kwargs.update(
+                    {
+                        'point_as_seconds': itask.get_point_as_seconds(),
+                    }
+                )
+                if wall_clock(*ctx.func_args, **ctx.func_kwargs):
+                    itask.state.xtriggers[label] = True
+                    self.sat_xtrig[sig] = {}
+                    LOG.info('clock xtrigger satisfied: %s = %s' % (
+                        label, str(ctx)))
+                continue
+            # General case: asynchronous xtrigger function call.
             if sig in self.sat_xtrig:
                 if not itask.state.xtriggers[label]:
                     itask.state.xtriggers[label] = True

From 019063650c28b30c69cb58e52511693c0c53c8b9 Mon Sep 17 00:00:00 2001
From: Hilary James Oliver <hilary.j.oliver@gmail.com>
Date: Mon, 12 Aug 2019 17:05:42 +1200
Subject: [PATCH 02/10] Add xtrigger tests.

---
 lib/cylc/config.py                      |  3 +-
 tests/validate/69-bare-clock-xtrigger.t | 31 ++++++++++++
 tests/xtriggers/03-sequence.t           | 63 +++++++++++++++++++++++++
 3 files changed, 96 insertions(+), 1 deletion(-)
 create mode 100644 tests/validate/69-bare-clock-xtrigger.t
 create mode 100644 tests/xtriggers/03-sequence.t

diff --git a/lib/cylc/config.py b/lib/cylc/config.py
index 0fd666376d3..19012fe7d49 100644
--- a/lib/cylc/config.py
+++ b/lib/cylc/config.py
@@ -1744,7 +1744,8 @@ def generate_triggers(self, lexpression, left_nodes, right, seq,
                 xtrig = self.cfg['scheduling']['xtriggers'][label]
             except KeyError:
                 if label == 'wall_clock':
-                    # Allow predefined zero-offset wall clock xtrigger.
+                    # Allow "@wall_clock" in the graph as an undeclared
+                    # zero-offset clock xtrigger.
                     xtrig = SubFuncContext(
                         'wall_clock', 'wall_clock', [], {})
                 else:
diff --git a/tests/validate/69-bare-clock-xtrigger.t b/tests/validate/69-bare-clock-xtrigger.t
new file mode 100644
index 00000000000..516c67c0772
--- /dev/null
+++ b/tests/validate/69-bare-clock-xtrigger.t
@@ -0,0 +1,31 @@
+#!/bin/bash
+# THIS FILE IS PART OF THE CYLC SUITE ENGINE.
+# Copyright (C) 2008-2019 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/>.
+
+# Test that undeclared zero-offset clock xtriggers are allowed.
+. "$(dirname "$0")/test_header"
+
+set_test_number 1
+
+cat >'suite.rc' <<'__SUITE_RC__'
+[scheduling]
+    initial cycle point = now
+    [[dependencies]]
+        [[[ T00 ]]]
+            graph = "@wall_clock => foo"
+__SUITE_RC__
+
+run_ok "${TEST_NAME_BASE}-val" cylc validate 'suite.rc'
diff --git a/tests/xtriggers/03-sequence.t b/tests/xtriggers/03-sequence.t
new file mode 100644
index 00000000000..9abd8bfff1c
--- /dev/null
+++ b/tests/xtriggers/03-sequence.t
@@ -0,0 +1,63 @@
+#!/bin/bash
+# THIS FILE IS PART OF THE CYLC SUITE ENGINE.
+# Copyright (C) 2008-2019 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/>.
+
+# Test xtrigger cycle-point specificity -
+#   https://github.com/cylc/cylc-flow/issues/3283
+
+. "$(dirname "$0")/test_header"
+
+set_test_number 3
+
+# Test suite uses built-in 'echo' xtrigger.
+init_suite "${TEST_NAME_BASE}" << '__SUITE_RC__'
+[cylc]
+   cycle point format = %Y
+[scheduling]
+   initial cycle point = 2025
+   final cycle point = +P1Y
+   spawn to max active cycle points = True
+   [[xtriggers]]
+       e1 = echo(name='bob')
+       e2 = echo(name='alice')
+   [[dependencies]]
+      [[[R/^/P2Y]]]
+          graph = "@e1 => foo"
+      [[[R/^+P1Y/P2Y]]]
+          graph = "@e2 => foo"
+__SUITE_RC__
+
+run_ok "${TEST_NAME_BASE}-val" cylc validate 'suite.rc'
+
+# Run suite; it will stall waiting on the never-satisfied xtriggers.
+cylc run "${SUITE_NAME}"
+
+sleep 5
+
+cylc show "${SUITE_NAME}" foo.2025 | egrep '^  o' > foo.2025.log
+cylc show "${SUITE_NAME}" foo.2026 | egrep '^  o' > foo.2026.log
+
+# foo.2025 should get only xtrigger e1.
+cmp_ok foo.2025.log - <<__END__
+  o  xtrigger "e1 = echo(name=bob)" ... NOT satisfied
+__END__
+
+# foo.2026 should get only xtrigger e2.
+cmp_ok foo.2026.log - <<__END__
+  o  xtrigger "e2 = echo(name=alice)" ... NOT satisfied
+__END__
+
+cylc stop --now "${SUITE_NAME}"

From cd885326b3319d6d7d9f76d8beab7ac7585ff668 Mon Sep 17 00:00:00 2001
From: Hilary James Oliver <hilary.j.oliver@gmail.com>
Date: Mon, 12 Aug 2019 17:18:53 +1200
Subject: [PATCH 03/10] Add to change log.

---
 CHANGES.md | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/CHANGES.md b/CHANGES.md
index d442fe77e8e..4dba2cb8eb5 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -38,6 +38,9 @@ with suite configurations that contain tasks with many task outputs.
 
 ### Fixes
 
+[#3285](https://github.com/cylc/cylc-flow/pull/3285) - fix xtrigger
+cycle-sequence specificity.
+
 [#3257](https://github.com/cylc/cylc-flow/pull/3257) - leave '%'-escaped string
 templates alone in xtrigger arguments.
 

From b21d94c35ee0c968ae1cdfa395e59b5c388ad667 Mon Sep 17 00:00:00 2001
From: Hilary James Oliver <hilary.j.oliver@gmail.com>
Date: Mon, 12 Aug 2019 17:52:51 +1200
Subject: [PATCH 04/10] Disallow clock triggers with int cycling.

---
 lib/cylc/config.py                     |  6 ++++
 tests/validate/70-no-clock-int-cycle.t | 40 ++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)
 create mode 100644 tests/validate/70-no-clock-int-cycle.t

diff --git a/lib/cylc/config.py b/lib/cylc/config.py
index 19012fe7d49..a079fa2e96e 100644
--- a/lib/cylc/config.py
+++ b/lib/cylc/config.py
@@ -1751,6 +1751,12 @@ def generate_triggers(self, lexpression, left_nodes, right, seq,
                 else:
                     raise SuiteConfigError(
                         "ERROR, undefined xtrigger label: %s" % label)
+            if (xtrig.func_name == 'wall_clock' and
+                    self.cfg['scheduling']['cycling mode'] == (
+                        INTEGER_CYCLING_TYPE)):
+                raise SuiteConfigError("ERROR: clock triggers are not "
+                        "compatible with integer cycling.\n %s = %s" % (
+                            label, xtrig.get_signature()))
             self.xtrigger_mgr.add_trig(label, xtrig, self.fdir)
             self.taskdefs[right].add_xtrig_label(label, seq)
 
diff --git a/tests/validate/70-no-clock-int-cycle.t b/tests/validate/70-no-clock-int-cycle.t
new file mode 100644
index 00000000000..569ce915259
--- /dev/null
+++ b/tests/validate/70-no-clock-int-cycle.t
@@ -0,0 +1,40 @@
+#!/bin/bash
+# THIS FILE IS PART OF THE CYLC SUITE ENGINE.
+# Copyright (C) 2008-2019 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/>.
+
+# Test that clock xtriggers are not allowed with integer cycling.
+. "$(dirname "$0")/test_header"
+
+set_test_number 2
+
+cat >'suite.rc' <<'__SUITE_RC__'
+[scheduling]
+   cycling mode = integer
+   initial cycle point = 1
+   final cycle point = 2
+   [[xtriggers]]
+       c1 = wall_clock(offset=P0Y)
+   [[dependencies]]
+      [[[R/^/P1]]]
+          graph = "@c1 & foo[-P1] => foo"
+__SUITE_RC__
+
+run_fail "${TEST_NAME_BASE}-val" cylc validate 'suite.rc'
+
+contains_ok "${TEST_NAME_BASE}-val.stderr" <<'__END__'
+ERROR: clock triggers are not compatible with integer cycling.
+ c1 = wall_clock(offset=P0Y)
+__END__

From 8b6b0781d324a3c66f1f44b7ff9b670940d3578a Mon Sep 17 00:00:00 2001
From: Hilary James Oliver <hilary.j.oliver@gmail.com>
Date: Mon, 12 Aug 2019 18:15:06 +1200
Subject: [PATCH 05/10] Fix config unit tests.

---
 lib/cylc/config.py            | 6 +++---
 lib/cylc/tests/test_config.py | 8 ++++----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/cylc/config.py b/lib/cylc/config.py
index a079fa2e96e..2587429b9cc 100644
--- a/lib/cylc/config.py
+++ b/lib/cylc/config.py
@@ -1754,9 +1754,9 @@ def generate_triggers(self, lexpression, left_nodes, right, seq,
             if (xtrig.func_name == 'wall_clock' and
                     self.cfg['scheduling']['cycling mode'] == (
                         INTEGER_CYCLING_TYPE)):
-                raise SuiteConfigError("ERROR: clock triggers are not "
-                        "compatible with integer cycling.\n %s = %s" % (
-                            label, xtrig.get_signature()))
+                raise SuiteConfigError(
+                    "ERROR: clock triggers are not compatible with integer "
+                    "cycling.\n %s = %s" % (label, xtrig.get_signature()))
             self.xtrigger_mgr.add_trig(label, xtrig, self.fdir)
             self.taskdefs[right].add_xtrig_label(label, seq)
 
diff --git a/lib/cylc/tests/test_config.py b/lib/cylc/tests/test_config.py
index 4dd3ac9459c..6ccada972fd 100644
--- a/lib/cylc/tests/test_config.py
+++ b/lib/cylc/tests/test_config.py
@@ -50,7 +50,7 @@ def test_xfunction_imports(self):
             f.flush()
             suite_config = SuiteConfig(suite="name_a_tree", fpath=f.name)
             config = suite_config
-            self.assertTrue('tree' in config.xtriggers['qux'])
+            self.assertTrue('tree' in config.xtrigger_mgr.functx_map)
         shutil.rmtree(temp_dir)
 
     def test_xfunction_import_error(self):
@@ -76,7 +76,7 @@ def test_xfunction_import_error(self):
             graph = '@oopsie => qux'
             """)
             f.flush()
-            with self.assertRaises(SuiteConfigError) as ex:
+            with self.assertRaises(ImportError) as ex:
                 SuiteConfig(suite="caiman_suite", fpath=f.name)
                 self.assertTrue("not found" in str(ex))
         shutil.rmtree(temp_dir)
@@ -104,7 +104,7 @@ def test_xfunction_attribute_error(self):
             graph = '@oopsie => qux'
             """)
             f.flush()
-            with self.assertRaises(SuiteConfigError) as ex:
+            with self.assertRaises(AttributeError) as ex:
                 SuiteConfig(suite="capybara_suite", fpath=f.name)
                 self.assertTrue("not found" in str(ex))
         shutil.rmtree(temp_dir)
@@ -132,7 +132,7 @@ def test_xfunction_not_callable(self):
             graph = '@oopsie => qux'
             """)
             f.flush()
-            with self.assertRaises(SuiteConfigError) as ex:
+            with self.assertRaises(ValueError) as ex:
                 SuiteConfig(suite="suite_with_not_callable", fpath=f.name)
                 self.assertTrue("callable" in str(ex))
         shutil.rmtree(temp_dir)

From 54138dcecb309e0c7b9b9fd921c0e7721a16d39d Mon Sep 17 00:00:00 2001
From: Hilary James Oliver <hilary.j.oliver@gmail.com>
Date: Tue, 13 Aug 2019 14:52:14 +1200
Subject: [PATCH 06/10] Address review feedback.

---
 lib/cylc/xtrigger_mgr.py      |  4 ++--
 tests/xtriggers/03-sequence.t | 11 +++++++++--
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/lib/cylc/xtrigger_mgr.py b/lib/cylc/xtrigger_mgr.py
index b5e5101c616..af7e0366423 100644
--- a/lib/cylc/xtrigger_mgr.py
+++ b/lib/cylc/xtrigger_mgr.py
@@ -229,8 +229,7 @@ def satisfy_xtriggers(self, itask, proc_pool):
                 if wall_clock(*ctx.func_args, **ctx.func_kwargs):
                     itask.state.xtriggers[label] = True
                     self.sat_xtrig[sig] = {}
-                    LOG.info('clock xtrigger satisfied: %s = %s' % (
-                        label, str(ctx)))
+                    LOG.info('xtrigger satisfied: %s = %s' % (label, sig))
                 continue
             # General case: asynchronous xtrigger function call.
             if sig in self.sat_xtrig:
@@ -275,5 +274,6 @@ def callback(self, ctx):
             return
         LOG.debug('%s: returned %s' % (sig, results))
         if satisfied:
+            LOG.info('xtrigger satisfied: %s = %s' % (ctx.label, sig))
             self.pflag = True
             self.sat_xtrig[sig] = results
diff --git a/tests/xtriggers/03-sequence.t b/tests/xtriggers/03-sequence.t
index 9abd8bfff1c..89148149712 100644
--- a/tests/xtriggers/03-sequence.t
+++ b/tests/xtriggers/03-sequence.t
@@ -34,18 +34,24 @@ init_suite "${TEST_NAME_BASE}" << '__SUITE_RC__'
        e1 = echo(name='bob')
        e2 = echo(name='alice')
    [[dependencies]]
+      [[[R1]]]
+          graph = "start"
       [[[R/^/P2Y]]]
           graph = "@e1 => foo"
       [[[R/^+P1Y/P2Y]]]
           graph = "@e2 => foo"
+[runtime]
+   [[start]]
+   [[foo]]
 __SUITE_RC__
 
 run_ok "${TEST_NAME_BASE}-val" cylc validate 'suite.rc'
 
-# Run suite; it will stall waiting on the never-satisfied xtriggers.
 cylc run "${SUITE_NAME}"
 
-sleep 5
+# Use "start" succeed as a proxy for suite initialization completed.
+LOG="${SUITE_RUN_DIR}/log/suite/log"
+poll "! egrep -q 'start.2025.*succeeded' '${LOG}' 2>'/dev/null'"
 
 cylc show "${SUITE_NAME}" foo.2025 | egrep '^  o' > foo.2025.log
 cylc show "${SUITE_NAME}" foo.2026 | egrep '^  o' > foo.2026.log
@@ -61,3 +67,4 @@ cmp_ok foo.2026.log - <<__END__
 __END__
 
 cylc stop --now "${SUITE_NAME}"
+purge_suite "${SUITE_NAME}"

From 18aa1ea2989c40dc0760c89d74336257dbbf0213 Mon Sep 17 00:00:00 2001
From: Hilary James Oliver <hilary.j.oliver@gmail.com>
Date: Tue, 13 Aug 2019 23:52:53 +1200
Subject: [PATCH 07/10] Use logger string formatting.

---
 lib/cylc/xtrigger_mgr.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/cylc/xtrigger_mgr.py b/lib/cylc/xtrigger_mgr.py
index af7e0366423..32c87611826 100644
--- a/lib/cylc/xtrigger_mgr.py
+++ b/lib/cylc/xtrigger_mgr.py
@@ -229,7 +229,7 @@ def satisfy_xtriggers(self, itask, proc_pool):
                 if wall_clock(*ctx.func_args, **ctx.func_kwargs):
                     itask.state.xtriggers[label] = True
                     self.sat_xtrig[sig] = {}
-                    LOG.info('xtrigger satisfied: %s = %s' % (label, sig))
+                    LOG.info('xtrigger satisfied: "%s = %s"', label, sig)
                 continue
             # General case: asynchronous xtrigger function call.
             if sig in self.sat_xtrig:
@@ -274,6 +274,6 @@ def callback(self, ctx):
             return
         LOG.debug('%s: returned %s' % (sig, results))
         if satisfied:
-            LOG.info('xtrigger satisfied: %s = %s' % (ctx.label, sig))
+            LOG.info('xtrigger satisfied: "%s = %s"', ctx.label, sig)
             self.pflag = True
             self.sat_xtrig[sig] = results

From 6fb9b35f37cb6aeac863e544ccbe7cc649c51f3c Mon Sep 17 00:00:00 2001
From: Hilary James Oliver <hilary.j.oliver@gmail.com>
Date: Thu, 15 Aug 2019 19:35:05 +1200
Subject: [PATCH 08/10] Back-port test hostname fix.

---
 tests/database/00-simple.t    | 2 ++
 tests/database/01-broadcast.t | 2 ++
 tests/database/02-retry.t     | 2 ++
 3 files changed, 6 insertions(+)

diff --git a/tests/database/00-simple.t b/tests/database/00-simple.t
index 690f70f5c0b..1e3a7d83d44 100644
--- a/tests/database/00-simple.t
+++ b/tests/database/00-simple.t
@@ -57,6 +57,8 @@ sqlite3 "${DB_FILE}" \
      FROM task_jobs ORDER BY name' \
     >"${NAME}"
 LOCALHOST="$(hostname -f)"
+# FIXME: recent Travis CI failure
+sed -i "s/localhost/${LOCALHOST}/" "${NAME}"
 cmp_ok "${NAME}" - <<__SELECT__
 1|bar|1|1|0|0|${LOCALHOST}|background
 1|baz|1|1|0|0|${LOCALHOST}|background
diff --git a/tests/database/01-broadcast.t b/tests/database/01-broadcast.t
index 6957e7e1b0f..a0901f55622 100755
--- a/tests/database/01-broadcast.t
+++ b/tests/database/01-broadcast.t
@@ -53,6 +53,8 @@ sqlite3 "${DB_FILE}" \
      FROM task_jobs ORDER BY name' \
     >"${NAME}"
 LOCALHOST="$(hostname -f)"
+# FIXME: recent Travis CI failure
+sed -i "s/localhost/${LOCALHOST}/" "${NAME}"
 cmp_ok "${NAME}" <<__SELECT__
 1|recover-t1|1|0|0|0|${LOCALHOST}|background
 1|t1|1|0|0|1|${LOCALHOST}|background
diff --git a/tests/database/02-retry.t b/tests/database/02-retry.t
index 4aa269b259f..d4d6c65c225 100755
--- a/tests/database/02-retry.t
+++ b/tests/database/02-retry.t
@@ -39,6 +39,8 @@ sqlite3 "${DB_FILE}" \
      FROM task_jobs ORDER BY name' \
     >"${NAME}"
 LOCALHOST="$(hostname -f)"
+# FIXME: recent Travis CI failure
+sed -i "s/localhost/${LOCALHOST}/" "${NAME}"
 cmp_ok "${NAME}" <<__SELECT__
 20200101T0000Z|t1|1|1|0|1|${LOCALHOST}|background
 20200101T0000Z|t1|2|2|0|1|${LOCALHOST}|background

From aef43aab47dbe1cb64acefa069cb4cf21d033c07 Mon Sep 17 00:00:00 2001
From: Hilary James Oliver <hilary.j.oliver@gmail.com>
Date: Thu, 15 Aug 2019 19:57:34 +1200
Subject: [PATCH 09/10] Back-port Matt's fix to a test.

---
 tests/runahead/06-release-update.t | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/runahead/06-release-update.t b/tests/runahead/06-release-update.t
index ce0c8cd59c9..46e41f973c5 100644
--- a/tests/runahead/06-release-update.t
+++ b/tests/runahead/06-release-update.t
@@ -28,8 +28,10 @@ run_ok $TEST_NAME cylc validate $SUITE_NAME
 #-------------------------------------------------------------------------------
 TEST_NAME=$TEST_NAME_BASE-check-states
 cylc run $SUITE_NAME
+YYYY="$(date +%Y)"
 LOG="$(cylc get-global-config --print-run-dir)/${SUITE_NAME}/log/suite/log"
-poll "! grep -q -F '[bar.2016] status=running: (received)succeeded' '${LOG}' 2>'/dev/null'"
+poll "! grep -q -F '[bar.${YYYY}] status=running: (received)succeeded' \
+    '${LOG}' 2>'/dev/null'"
 sleep 1
 cylc dump -t $SUITE_NAME | awk '{print $1 $3}' > log
 cmp_ok log - << __END__

From 747785355c91b8e3f7ac4eeb240725b5d2a8f3f4 Mon Sep 17 00:00:00 2001
From: Hilary James Oliver <hilary.j.oliver@gmail.com>
Date: Thu, 15 Aug 2019 20:02:27 +1200
Subject: [PATCH 10/10] cylc-review test: hostname fix.

---
 tests/cylc-review/00-basic.t | 16 ++++++++++------
 tests/cylc-review/01-title.t | 13 +++++++++----
 2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/tests/cylc-review/00-basic.t b/tests/cylc-review/00-basic.t
index 52c42c5d804..092d78fe178 100755
--- a/tests/cylc-review/00-basic.t
+++ b/tests/cylc-review/00-basic.t
@@ -48,10 +48,14 @@ grep_ok 'HTTP/.* 200 OK' "${TEST_NAME}.stdout"
 
 TEST_NAME="${TEST_NAME_BASE}-200-curl-root-json"
 run_ok "${TEST_NAME}" curl "${TEST_CYLC_WS_URL}/?form=json"
+
+# FIXME: recent Travis CI failure
+#HOSTNAME=$(hostname)
+HOSTNAME="localhost"
 cylc_ws_json_greps "${TEST_NAME}.stdout" "${TEST_NAME}.stdout" \
     "[('cylc_version',), '$(cylc version | cut -d' ' -f 2)']" \
     "[('title',), 'Cylc Review']" \
-    "[('host',), '$(hostname)']"
+    "[('host',), '${HOSTNAME}']"
 #-------------------------------------------------------------------------------
 # Data transfer output check for a specific user's page including non-existent
 TEST_NAME="${TEST_NAME_BASE}-200-curl-suites"
@@ -63,7 +67,7 @@ run_ok "${TEST_NAME}" curl "${TEST_CYLC_WS_URL}/suites/${USER}?form=json"
 cylc_ws_json_greps "${TEST_NAME}.stdout" "${TEST_NAME}.stdout" \
     "[('cylc_version',), '$(cylc version | cut -d' ' -f 2)']" \
     "[('title',), 'Cylc Review']" \
-    "[('host',), '$(hostname)']" \
+    "[('host',), '${HOSTNAME}']" \
     "[('user',), '${USER}']"
 
 TEST_NAME="${TEST_NAME_BASE}-404-curl-suites"
@@ -110,7 +114,7 @@ run_ok "${TEST_NAME}" \
 cylc_ws_json_greps "${TEST_NAME}.stdout" "${TEST_NAME}.stdout" \
     "[('cylc_version',), '$(cylc version | cut -d' ' -f 2)']" \
     "[('title',), 'Cylc Review']" \
-    "[('host',), '$(hostname)']" \
+    "[('host',), '${HOSTNAME}']" \
     "[('user',), '${USER}']" \
     "[('suite',), '${SUITE_NAME}']" \
     "[('page',), 1]" \
@@ -133,7 +137,7 @@ FOO1_JOB='log/job/20000101T0000Z/foo1/01/job'
 cylc_ws_json_greps "${TEST_NAME}.stdout" "${TEST_NAME}.stdout" \
     "[('cylc_version',), '$(cylc version | cut -d' ' -f 2)']" \
     "[('title',), 'Cylc Review']" \
-    "[('host',), '$(hostname)']" \
+    "[('host',), '${HOSTNAME}']" \
     "[('user',), '${USER}']" \
     "[('suite',), '${SUITE_NAME}']" \
     "[('is_option_on',), False]" \
@@ -148,7 +152,7 @@ cylc_ws_json_greps "${TEST_NAME}.stdout" "${TEST_NAME}.stdout" \
     "[('states', 'is_failed',), False]" \
     "[('of_n_entries',), 2]" \
     "[('entries', ${FOO0}, 'task_status',), 'succeeded']" \
-    "[('entries', ${FOO0}, 'host',), '$(hostname -f)']" \
+    "[('entries', ${FOO0}, 'host',), '${HOSTNAME}']" \
     "[('entries', ${FOO0}, 'submit_method',), 'background']" \
     "[('entries', ${FOO0}, 'logs', 'job', 'path'), '${FOO0_JOB}']" \
     "[('entries', ${FOO0}, 'logs', 'job.err', 'path'), '${FOO0_JOB}.err']" \
@@ -172,7 +176,7 @@ cylc_ws_json_greps "${TEST_NAME}.stdout" "${TEST_NAME}.stdout" \
     "[('entries', ${FOO0}, 'seq_logs_indexes', 'job.trace.*.html', '32'), 'job.trace.32.html']" \
     "[('entries', ${FOO0}, 'seq_logs_indexes', 'job.trace.*.html', '256'), 'job.trace.256.html']" \
     "[('entries', ${FOO1}, 'task_status',), 'succeeded']" \
-    "[('entries', ${FOO1}, 'host',), '$(hostname -f)']" \
+    "[('entries', ${FOO1}, 'host',), '${HOSTNAME}']" \
     "[('entries', ${FOO1}, 'submit_method',), 'background']" \
     "[('entries', ${FOO1}, 'logs', 'job', 'path'), '${FOO1_JOB}']" \
     "[('entries', ${FOO1}, 'logs', 'job.err', 'path'), '${FOO1_JOB}.err']" \
diff --git a/tests/cylc-review/01-title.t b/tests/cylc-review/01-title.t
index acf30036536..9a139389489 100755
--- a/tests/cylc-review/01-title.t
+++ b/tests/cylc-review/01-title.t
@@ -42,19 +42,24 @@ fi
 ESC_SUITE_NAME="$(echo ${SUITE_NAME} | sed 's|/|%2F|g')"
 #-------------------------------------------------------------------------------
 # Basic data transfer output check
+
+# FIXME: recent Travis CI failure
+#HOSTNAME=$(hostname)
+HOSTNAME="localhost"
+
 TEST_NAME="${TEST_NAME_BASE}-200-curl-root-json"
 run_ok "${TEST_NAME}" curl "${TEST_CYLC_WS_URL}/?form=json"
 cylc_ws_json_greps "${TEST_NAME}.stdout" "${TEST_NAME}.stdout" \
     "[('logo',), 'cylc-logo.png']" \
     "[('title',), 'Cylc Review']" \
-    "[('host',), '$(hostname)']"
+    "[('host',), '${HOSTNAME}']"
 
 TEST_NAME="${TEST_NAME_BASE}-200-curl-suites-json"
 run_ok "${TEST_NAME}" curl "${TEST_CYLC_WS_URL}/suites/${USER}?form=json"
 cylc_ws_json_greps "${TEST_NAME}.stdout" "${TEST_NAME}.stdout" \
     "[('logo',), 'cylc-logo.png']" \
     "[('title',), 'Cylc Review']" \
-    "[('host',), '$(hostname)']"
+    "[('host',), '${HOSTNAME}']"
 
 TEST_NAME="${TEST_NAME_BASE}-200-curl-cycles-json"
 run_ok "${TEST_NAME}" \
@@ -62,7 +67,7 @@ run_ok "${TEST_NAME}" \
 cylc_ws_json_greps "${TEST_NAME}.stdout" "${TEST_NAME}.stdout" \
     "[('logo',), 'cylc-logo.png']" \
     "[('title',), 'Cylc Review']" \
-    "[('host',), '$(hostname)']"
+    "[('host',), '${HOSTNAME}']"
 
 TEST_NAME="${TEST_NAME_BASE}-200-curl-jobs-json"
 run_ok "${TEST_NAME}" \
@@ -70,7 +75,7 @@ run_ok "${TEST_NAME}" \
 cylc_ws_json_greps "${TEST_NAME}.stdout" "${TEST_NAME}.stdout" \
     "[('logo',), 'cylc-logo.png']" \
     "[('title',), 'Cylc Review']" \
-    "[('host',), '$(hostname)']"
+    "[('host',), '${HOSTNAME}']"
 #-------------------------------------------------------------------------------
 # Tidy up - note suite trivial so stops early on by itself
 purge_suite "${SUITE_NAME}"