Adding or updating tests to improve codecov coverage.

cylc/flow/time_parser.py

@@ -347,6 +347,13 @@ def parse_recurrence(
     def _get_interval_from_expression(
         self, expr: Optional[str], context: Optional['TimePoint'] = None
     ) -> Optional[Duration]:
+        """
+        Gets a duration from an expression.
+        Currently contains a bug, referenced here:
+        https://github.com/cylc/cylc-flow/issues/2382#issuecomment-1651837780
+        Which results in unexpected behaviour around 31 day periods and
+        month boundaries.
+        """
         if expr is None:
             if context is None or not context.truncated:
                 return None

tests/unit/plugins/test_pre_configure.py

@@ -129,3 +129,7 @@ def test_pre_configure_exception(monkeypatch):
     assert exc_ctx.value.entry_point == 'cylc.pre_configure'
     assert exc_ctx.value.plugin_name == 'pre_configure_error'
     assert str(exc_ctx.value.exc) == 'foo'
+    assert str(exc_ctx.value) == (
+        "Error in plugin cylc.pre_configure.pre_configure_error\n"
+        "Exception: foo"
+    )

tests/unit/test_exceptions.py

@@ -21,6 +21,9 @@
 from cylc.flow.exceptions import (
     CylcError,
     HostSelectException,
+    CommandFailedError,
+    FileRemovalError,
+    CylcVersionError,
 )
 
 
@@ -76,3 +79,37 @@ def test_host_select_exception_returncodes(ret_code, expect):
             ranking='virtual_memory().available > 1',
         )
     )
+
+
+def test_command_failed_exception():
+    """Test exception used for command failures."""
+    # when the value is a string, it should return it directly
+    exc = CommandFailedError("something broke")
+    assert str(exc) == "something broke"
+
+    # when the value is an exception, it should format the type and message
+    exc = CommandFailedError(ValueError("bad value"))
+    assert str(exc) == "ValueError: bad value"
+
+
+def test_file_removal_error():
+    """Test exception used for file removal failures."""
+    exc = FileRemovalError(OSError("error removing file"))
+    assert str(exc) == (
+        "error removing file. This is probably a temporary "
+        "issue with the filesystem, "
+        "not a problem with Cylc."
+    )
+
+
+def test_cylc_version_error():
+    """Test exception used for incompatible Cylc versions."""
+    # when version is provided
+    exc = CylcVersionError(version='7.0.0')
+    assert str(exc) == (
+        'Installed Cylc 7.0.0 workflow is not compatible with Cylc 8.'
+    )
+
+    # when version is None
+    exc = CylcVersionError()
+    assert str(exc) == "Installed workflow is not compatible with Cylc 8."

tests/unit/test_task_id.py

@@ -15,8 +15,10 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import unittest
+import pytest
 
 from cylc.flow.task_id import TaskID
+from cylc.flow.cycling.iso8601 import ISO8601Point
 
 
 class TestTaskId(unittest.TestCase):
@@ -65,3 +67,10 @@ def test_is_valid_id_2(self):
             "abc", "123", "____", "_", "a_b", "a_1", "1_b", "ABC"
         ]:
             self.assertFalse(TaskID.is_valid_id_2(id2))
+
+
+def test_get_standardised_point_string(set_cycling_type):
+    """Test that invalid point string raises ValueError."""
+    set_cycling_type(ISO8601Point.TYPE)
+    with pytest.raises(ValueError, match="Invalid cycle point:"):
+        TaskID.get_standardised_point_string("aaa")

tests/unit/test_task_outputs.py

@@ -140,6 +140,21 @@ def test_completion_explicit():
     assert outputs.is_complete() is True
 
 
+def test_is_message_complete():
+    """It should report the completion status of individual messages."""
+    outputs = TaskOutputs(tdef([TASK_OUTPUT_SUCCEEDED], []))
+
+    # message exists but hasn't been completed yet
+    assert outputs.is_message_complete(TASK_OUTPUT_SUCCEEDED) is False
+
+    # message doesn't exist in this task's outputs
+    assert outputs.is_message_complete('no-such-output') is None
+
+    # message has been completed
+    outputs.set_message_complete(TASK_OUTPUT_SUCCEEDED)
+    assert outputs.is_message_complete(TASK_OUTPUT_SUCCEEDED) is True
+
+
 @pytest.mark.parametrize(
     'required, optional, expression', [
         pytest.param(
@@ -217,6 +232,7 @@ def test_get_completion_expression_explicit():
 
 
 def test_format_completion_status():
+    """It should format the completion status of the outputs."""
     outputs = TaskOutputs(
         tdef(
             {TASK_OUTPUT_SUCCEEDED, 'x', 'y'},
@@ -251,6 +267,48 @@ def test_format_completion_status():
     )
 
 
+def test_format_completion_status_nested():
+    """
+    It should format nested completion expressions with operators.
+    Catches the case where the opening paren is preceded by an operator.
+    """
+    outputs = TaskOutputs(
+        tdef(
+            [],
+            [TASK_OUTPUT_SUCCEEDED, TASK_OUTPUT_FAILED, 'x', 'y'],
+            completion='succeeded or (failed and x)',
+        )
+    )
+    result = outputs.format_completion_status(indent=2, gutter=2)
+    # the opening paren should be preceded by the 'or' operator
+    assert 'or (' in result
+
+
+def test_format_completion_status_ansimarkup():
+    """It should colour-code output status when ansimarkup is enabled."""
+    outputs = TaskOutputs(
+        tdef({TASK_OUTPUT_SUCCEEDED}, {TASK_OUTPUT_EXPIRED})
+    )
+    outputs.set_message_complete(TASK_OUTPUT_SUCCEEDED)
+
+    # ansimarkup=1: only completed outputs get colour
+    result = outputs.format_completion_status(indent=2, gutter=2, ansimarkup=1)
+    assert '<green>succeeded</green>' in result
+    assert '<red>' not in result  # incomplete outputs are NOT coloured
+
+    # ansimarkup=2: both complete and incomplete get colour
+    result = outputs.format_completion_status(indent=2, gutter=2, ansimarkup=2)
+    assert '<green>succeeded</green>' in result
+    assert '<red>expired</red>' in result
+
+
+def test_is_compvar_complete_invalid():
+    """It should raise KeyError for an unknown completion variable."""
+    outputs = TaskOutputs(tdef([TASK_OUTPUT_SUCCEEDED], []))
+    with pytest.raises(KeyError):
+        outputs._is_compvar_complete('no_such_var')
+
+
 @pytest.mark.parametrize(
     'required, optional, expected_required, expected_expression', [
         # this task has three required outputs and one optional output

tests/unit/test_task_proxy.py

@@ -29,6 +29,46 @@
 from cylc.flow.taskdef import TaskDef
 
 
+def test_get_point_as_seconds_cached():
+    """Already-computed value is returned immediately."""
+    mock_itask = Mock(point_as_seconds=42)
+    assert TaskProxy.get_point_as_seconds(mock_itask) == 42
+
+
+def test_get_point_as_seconds_known_tz(set_cycling_type):
+    """Cycle point with explicit UTC timezone."""
+    set_cycling_type(ISO8601Point.TYPE)
+    mock_itask = Mock(
+        point=ISO8601Point('19700101T00Z').standardise(),
+        point_as_seconds=None,
+    )
+    assert TaskProxy.get_point_as_seconds(mock_itask) == 0
+
+
+def test_get_point_as_seconds_unknown_tz(monkeypatch):
+    """Cycle point without timezone triggers local-tz offset adjustment."""
+    # Mock point_parse to return a timepoint with unknown timezone
+    mock_timepoint = Mock(
+        seconds_since_unix_epoch=3600,
+        time_zone=Mock(unknown=True),
+    )
+    monkeypatch.setattr(
+        'cylc.flow.task_proxy.point_parse',
+        lambda _: mock_timepoint,
+    )
+    # Pretend local timezone is UTC+5:30
+    monkeypatch.setattr(
+        'cylc.flow.task_proxy.get_local_time_zone',
+        lambda: (5, 30),
+    )
+    mock_itask = Mock(
+        point='19700101T01',
+        point_as_seconds=None,
+    )
+    result = TaskProxy.get_point_as_seconds(mock_itask)
+    assert result == 3600 + (5*3600 + 30*60)
+
+
 @pytest.mark.parametrize(
     'itask_point, offset_str, expected',
     [
@@ -71,6 +111,49 @@ def test_get_clock_trigger_time(
         mock_itask, mock_itask.point, offset_str) == expected
 
 
+def test_is_ready_to_run_held():
+    """A held task is not ready to run."""
+    mock_itask = Mock(state=Mock(is_held=True))
+    assert TaskProxy.is_ready_to_run(mock_itask) is False
+
+
+def test_is_ready_to_run_try_timer():
+    """A task with an active try timer delegates to is_delay_done()."""
+    mock_timer = Mock()
+    mock_timer.is_delay_done.return_value = True
+    mock_itask = Mock(
+        state=Mock(is_held=False, status='submission-failed'),
+        try_timers={'submission-failed': mock_timer},
+    )
+    assert TaskProxy.is_ready_to_run(mock_itask) is True
+    assert mock_timer.is_delay_done.call_count == 1
+
+
+@pytest.mark.parametrize(
+    'is_waiting, prereqs, ext_trigs, xtrigs, expected',
+    [
+        param(True, True, True, True, True, id="all-satisfied"),
+        param(False, True, True, True, False, id="not-waiting"),
+        param(True, False, True, True, False, id="prereqs-unsatisfied"),
+        param(True, True, False, True, False, id="ext-trigs-unsatisfied"),
+        param(True, True, True, False, False, id="xtrigs-unsatisfied"),
+    ]
+)
+def test_is_ready_to_run_conditions(
+    is_waiting, prereqs, ext_trigs, xtrigs, expected
+):
+    """Test the final return with various combinations of conditions."""
+    mock_itask = Mock(
+        state=Mock(is_held=False, status='waiting'),
+        try_timers={},
+    )
+    mock_itask.state.return_value = is_waiting
+    mock_itask.prereqs_are_satisfied = Mock(return_value=prereqs)
+    mock_itask.state.external_triggers_all_satisfied.return_value = ext_trigs
+    mock_itask.state.xtriggers_all_satisfied.return_value = xtrigs
+    assert TaskProxy.is_ready_to_run(mock_itask) is expected
+
+
 @pytest.mark.parametrize(
     'name_str, expected',
     [('beer', True),

tests/unit/test_time_parser.py

@@ -18,6 +18,9 @@
 
 import pytest
 
+from unittest.mock import Mock
+from metomi.isodatetime.data import Duration
+from cylc.flow.exceptions import CylcTimeSyntaxError
 from cylc.flow.time_parser import (
     CylcTimeParser,
     UTC_UTC_OFFSET_HOURS_MINUTES,
@@ -208,3 +211,107 @@ def test_interval(parsers):
     for expression in tests:
         test_data = str(parsers[0].parse_interval(expression))
         assert test_data == expression
+
+
+def test_parse_timepoint_invalid(parsers):
+    """It should raise CylcTimeSyntaxError for invalid expressions."""
+    with pytest.raises(CylcTimeSyntaxError, match="not a valid"):
+        parsers[0].parse_timepoint("not_a_date")
+
+
+def test_parse_timepoint_none(parsers):
+    """It should raise CylcTimeSyntaxError when expr is None."""
+    with pytest.raises(CylcTimeSyntaxError, match="not a valid"):
+        parsers[0].parse_timepoint(None)
+
+
+def test_parse_recurrence_invalid(parsers):
+    """It should raise CylcTimeSyntaxError for unparsable expressions."""
+    with pytest.raises(CylcTimeSyntaxError, match="Could not parse"):
+        parsers[0].parse_recurrence("///")
+
+
+def test_parse_recurrence_with_context(parsers):
+    """It should use explicit context points when provided."""
+    parser = parsers[0]
+    start = parser.parse_timepoint("20000101T00Z")
+    end = parser.parse_timepoint("20010101T00Z")
+    recurrence = parser.parse_recurrence(
+        "R2/P1Y/",
+        context_start_point=start,
+        context_end_point=end,
+    )[0]
+    assert str(recurrence) == "R2/P1Y/20010101T0000Z"
+
+
+def test_get_interval_from_expression(parsers):
+    """
+    Method should get a duration from an expression.
+    Currently contains a bug, referenced here:
+    https://github.com/cylc/cylc-flow/issues/2382#issuecomment-1651837780
+    Which results in unexpected behaviour around 31 day periods and
+    month boundaries.
+    """
+    parser = parsers[0]
+
+    # when expr is provided, it should just parse it
+    assert str(parser._get_interval_from_expression("P1D")) == "P1D"
+
+    # when expr is None and context is None, return None
+    assert parser._get_interval_from_expression(None, None) is None
+
+    # when expr is None and context is not truncated, return None
+    context = Mock(truncated=False)
+    assert parser._get_interval_from_expression(None, context) is None
+
+    # test each truncated property name
+    cases = [
+        ("year_of_century", Duration(years=100)),
+        ("year_of_decade", Duration(years=10)),
+        ("month_of_year", Duration(years=1)),
+        ("week_of_year", Duration(years=1)),
+        ("day_of_year", Duration(years=1)),
+        ("day_of_month", Duration(months=1)),
+        ("day_of_week", Duration(days=7)),
+        ("hour_of_day", Duration(days=1)),
+        ("minute_of_hour", Duration(hours=1)),
+        ("second_of_minute", Duration(minutes=1)),
+    ]
+    for prop_name, expected in cases:
+        context = Mock(truncated=True)
+        context.get_largest_truncated_property_name.return_value = prop_name
+        result = parser._get_interval_from_expression(None, context)
+        assert result == expected, f"Failed for {prop_name}"
+
+    # when prop_name doesn't match anything, return None
+    context = Mock(truncated=True)
+    context.get_largest_truncated_property_name.return_value = "unknown_prop"
+    assert parser._get_interval_from_expression(None, context) is None
+
+
+def test_get_min_from_expression_unresolvable(parsers):
+    """It should skip points that cannot be resolved (cpoint is None)."""
+    parser = parsers[0]
+
+    # "+P1M" with context=None results in cpoint=None (pure offset, no
+    # context to resolve against), so it should be skipped.
+    # "20000101T0000Z" resolves fine and should be selected as the min.
+    result = parser._get_min_from_expression(
+        "min(+P1M, 20000101T0000Z)", context=None
+    )
+    assert result == "20000101T0000Z"
+
+
+def test_get_point_from_expression_truncated_isodatetime_error(parsers):
+    """It should continue past truncated expressions that raise
+    IsodatetimeError."""
+    parser = parsers[0]
+
+    # "99T25" matches the TRUNCATED_REC_MAP regex ^\d\dT (for "---" prefix),
+    # but "---99T25" is not a valid truncated ISO point (no day 99, hour 25),
+    # so it raises IsodatetimeError and the loop continues.
+    # Nothing else matches, so CylcTimeSyntaxError is ultimately raised.
+    with pytest.raises(CylcTimeSyntaxError):
+        parser._get_point_from_expression(
+            "99T25", context=None, allow_truncated=True
+        )