Merge branch '8.1.x' into fix.sort_lint_listing--correct_number_for_l…

…ine_len

CHANGES.md

@@ -21,6 +21,12 @@ for `cylc play` when called by `cylc vip` or `cylc vr`.
 -------------------------------------------------------------------------------
 ## __cylc-8.1.5 (<span actions:bind='release-date'>Awaiting Release</span>)__
 
+### Enhancements
+
+[#5546](https://github.com/cylc/cylc-flow/pull/5546) -
+`cylc lint` will provide a non-zero return code if any issues are identified.
+This can be overridden using the new `--exit-zero` flag.
+
 ### Fixes
 
 [#5228](https://github.com/cylc/cylc-flow/pull/5228) -

cylc/flow/scheduler.py

@@ -898,10 +898,11 @@ def process_command_queue(self) -> None:
             else:
                 if n_warnings:
                     LOG.info(
-                        'Command succeeded with %s warning(s): %s' %
-                        (n_warnings, cmdstr))
+                        f"Command actioned with {n_warnings} warning(s): "
+                        f"{cmdstr}"
+                    )
                 else:
-                    LOG.info(f"Command succeeded: {cmdstr}")
+                    LOG.info(f"Command actioned: {cmdstr}")
                 self.is_updated = True
             self.command_queue.task_done()
 

cylc/flow/scheduler_cli.py

@@ -523,6 +523,7 @@ def _version_check(
                     process=str.lower,
                 )
             # we are in non-interactive mode, abort abort abort
+            print('Use "--upgrade" to upgrade the workflow.', file=sys.stderr)
             return False
         elif itt > 2 and this > that:
             # restart would INCREASE the Cylc version in a little way

cylc/flow/scripts/lint.py

@@ -31,13 +31,17 @@
 In-place mode ("-i, --inplace") writes suggestions into the file as comments.
 Commit to version control before using this, in case you want to back out.
 
+A non-zero return code will be returned if any issues are identified.
+This can be overridden by providing the "--exit-zero" flag.
+
 Configurations for Cylc lint can also be set in a pyproject.toml file.
 
 """
 from colorama import Fore
 from optparse import Values
 from pathlib import Path
 import re
+import sys
 import tomli
 from typing import Generator, Union
 
@@ -153,8 +157,8 @@
         'url': 'https://github.com/cylc/cylc-flow/issues/3825',
         'index': 10
     },
-    re.compile(r'#.*?{[{%]'): {
-        'short': 'Cylc will process commented Jinja2!',
+    re.compile(r'(?<!{)#.*?{[{%]'): {
+        'short': 'Cylc comments (#) do not prevent Jinja2 processing.',
         'url': '',
         'index': 11
     }
@@ -495,7 +499,7 @@ def check_cylc_file(
                 check.findall(line)
                 and (
                     not line.strip().startswith('#')
-                    or 'commented Jinja2!' in message['short']
+                    or message['index'] == 11  # commented-out Jinja2
                 )
             ):
                 count += 1
@@ -665,6 +669,13 @@ def get_option_parser() -> COP:
         metavar="CODE",
         choices=tuple([f'S{i["index"]:03d}' for i in STYLE_CHECKS.values()])
     )
+    parser.add_option(
+        '--exit-zero',
+        help='Exit with status code "0" even if there are issues.',
+        action='store_true',
+        default=False,
+        dest='exit_zero'
+    )
 
     return parser
 
@@ -677,7 +688,7 @@ def main(parser: COP, options: 'Values', target=None) -> None:
         else:
             rulesets = [options.linter]
         print(get_reference_text(parse_checks(rulesets, reference=True)))
-        exit(0)
+        sys.exit(0)
 
     # If target not given assume we are looking at PWD
     if target is None:
@@ -723,7 +734,9 @@ def main(parser: COP, options: 'Values', target=None) -> None:
             'Lint after renaming '
             '"suite.rc" to "flow.cylc"'
         )
-        exit(0)
+        # Exit with an error code if --exit-zero was not set.
+        # Return codes: sys.exit(True) == 1, sys.exit(False) == 0
+        sys.exit(not options.exit_zero)
     elif not cylc8 and '728' in mergedopts['rulesets']:
         check_names = mergedopts['rulesets']
         check_names.remove('728')
@@ -761,6 +774,11 @@ def main(parser: COP, options: 'Values', target=None) -> None:
 
     print(msg)
 
+    # Exit with an error code if there were warnings and
+    # if --exit-zero was not set.
+    # Return codes: sys.exit(True) == 1, sys.exit(False) == 0
+    sys.exit(count != 0 and not options.exit_zero)
+
 
 # NOTE: use += so that this works with __import__
 # (docstring needed for `cylc help all` output)

tests/functional/cli/03-set-verbosity.t

@@ -39,7 +39,7 @@ workflow_run_ok "${TEST_NAME}-run" cylc play --pause "$WORKFLOW_NAME"
 
 run_ok "$TEST_NAME" cylc set-verbosity DEBUG "$WORKFLOW_NAME"
 log_scan "${TEST_NAME}-grep" "${WORKFLOW_RUN_DIR}/log/scheduler/log" 5 1 \
-    'Command succeeded: set_verbosity'
+    'Command actioned: set_verbosity'
 
 cylc stop "$WORKFLOW_NAME"
 purge

tests/functional/cylc-lint/00.lint.t

@@ -18,7 +18,7 @@
 #------------------------------------------------------------------------------
 # Test workflow installation
 . "$(dirname "$0")/test_header"
-set_test_number 18
+set_test_number 20
 
 cat > flow.cylc <<__HERE__
 # This is definitely not an OK flow.cylc file.
@@ -29,15 +29,15 @@ __HERE__
 rm etc/global.cylc
 
 TEST_NAME="${TEST_NAME_BASE}.vanilla"
-run_ok "${TEST_NAME}" cylc lint .
+run_fail "${TEST_NAME}" cylc lint .
 named_grep_ok "check-for-error-code" "S004" "${TEST_NAME}.stdout"
 
 TEST_NAME="${TEST_NAME_BASE}.pick-a-ruleset"
-run_ok "${TEST_NAME}" cylc lint . -r 728
+run_fail "${TEST_NAME}" cylc lint . -r 728
 named_grep_ok "check-for-error-code" "U024" "${TEST_NAME}.stdout"
 
 TEST_NAME="${TEST_NAME_BASE}.inplace"
-run_ok "${TEST_NAME}" cylc lint . -i
+run_fail "${TEST_NAME}" cylc lint . -i
 named_grep_ok "check-for-error-code-in-file" "U024" flow.cylc
 
 rm flow.cylc
@@ -47,12 +47,18 @@ cat > suite.rc <<__HERE__
 {{FOO}}
 __HERE__
 
-TEST_NAME="${TEST_NAME_BASE}.pick-a-ruleset"
-run_ok "${TEST_NAME}" cylc lint . -r 728
+TEST_NAME="${TEST_NAME_BASE}.pick-a-ruleset-728"
+run_fail "${TEST_NAME}" cylc lint . -r 728
 named_grep_ok "do-not-upgrade-check-if-compat-mode" "Lint after renaming" "${TEST_NAME}.stderr"
 
-TEST_NAME="${TEST_NAME_BASE}.pick-a-ruleset2"
-run_ok "${TEST_NAME}" cylc lint . -r all
+TEST_NAME="${TEST_NAME_BASE}.pick-a-ruleset-728-exit-zero"
+run_ok "${TEST_NAME}" cylc lint . -r 728 --exit-zero
+
+TEST_NAME="${TEST_NAME_BASE}.pick-a-ruleset-all"
+run_fail "${TEST_NAME}" cylc lint . -r all
+
+TEST_NAME="${TEST_NAME_BASE}.exit-zero"
+run_ok "${TEST_NAME}" cylc lint --exit-zero .
 
 rm suite.rc
 

tests/functional/cylc-lint/01.lint-toml.t

@@ -47,7 +47,7 @@ __HERE__
 
 # Control tests
 TEST_NAME="it lints without toml file"
-run_ok "${TEST_NAME}" cylc lint
+run_fail "${TEST_NAME}" cylc lint
 TESTOUT="${TEST_NAME}.stdout"
 named_grep_ok "it returns error code" "S004" "${TESTOUT}"
 named_grep_ok "it returns error from subdirectory" "niwa.cylc" "${TESTOUT}"
@@ -75,7 +75,7 @@ __HERE__
 
 # Test that results are different:
 TEST_NAME="it_lints_with_toml_file"
-run_ok "${TEST_NAME}" cylc lint
+run_fail "${TEST_NAME}" cylc lint
 TESTOUT="${TEST_NAME}.stdout"
 grep_fail "S004" "${TESTOUT}"
 grep_fail "niwa.cylc" "${TESTOUT}"
@@ -93,7 +93,7 @@ cat > flow.cylc <<__HERE__
 __HERE__
 
 TEST_NAME="it_fails_if_max-line-length_set"
-run_ok "${TEST_NAME}" cylc lint
+run_fail "${TEST_NAME}" cylc lint
 named_grep_ok "${TEST_NAME}-line-too-long-message" \
     "\[S008\] flow.cylc:2: line > 4 characters." \
     "${TEST_NAME}.stdout"

tests/functional/hold-release/00-workflow/flow.cylc

@@ -23,7 +23,7 @@
         script = """
             cylc__job__wait_cylc_message_started
             cylc hold --after=1900 "${CYLC_WORKFLOW_ID}"
-            cylc__job__poll_grep_workflow_log -F 'INFO - Command succeeded: set_hold_point'
+            cylc__job__poll_grep_workflow_log -F 'INFO - Command actioned: set_hold_point'
             cylc release --all "${CYLC_WORKFLOW_ID}"
         """
     [[foo,bar]]

tests/functional/hold-release/05-release.t

@@ -34,7 +34,7 @@ init_workflow "${TEST_NAME_BASE}" <<'__FLOW_CONFIG__'
         script = """
             cylc__job__wait_cylc_message_started
             cylc hold --after=0 ${CYLC_WORKFLOW_ID}
-            cylc__job__poll_grep_workflow_log 'Command succeeded: set_hold_point'
+            cylc__job__poll_grep_workflow_log 'Command actioned: set_hold_point'
             cylc release "${CYLC_WORKFLOW_ID}//1/*FF"  # inexact fam
             cylc release "${CYLC_WORKFLOW_ID}//1/TOAST"  # exact fam
             cylc release "${CYLC_WORKFLOW_ID}//1/cat*"  # inexact tasks

tests/functional/pause-resume/00-workflow/flow.cylc

@@ -19,7 +19,7 @@
         script = """
             wait
             cylc pause "${CYLC_WORKFLOW_ID}"
-            cylc__job__poll_grep_workflow_log -F 'INFO - Command succeeded: pause()'
+            cylc__job__poll_grep_workflow_log -F 'INFO - Command actioned: pause()'
             cylc play "${CYLC_WORKFLOW_ID}"
         """
     [[foo,bar]]

tests/functional/pause-resume/12-pause-then-retry/flow.cylc

@@ -19,7 +19,7 @@
     [[t-pause]]
         script = """
             cylc pause "${CYLC_WORKFLOW_ID}"
-            cylc__job__poll_grep_workflow_log -F 'Command succeeded: pause'
+            cylc__job__poll_grep_workflow_log -F 'Command actioned: pause'
 
             # Poll t-submit-retry-able, should return submit-fail
             cylc poll "${CYLC_WORKFLOW_ID}//*/t-submit-retry-able"

tests/functional/reload/25-xtriggers.t

@@ -64,7 +64,7 @@ log_scan "${TEST_NAME_BASE}-scan" \
     "$(cylc cat-log -m p "${WORKFLOW_NAME}")" \
     1 1 \
     '1/broken .* (received)failed/ERR' \
-    'Command succeeded: reload_workflow()' \
+    'Command actioned: reload_workflow()' \
     'xtrigger satisfied: _cylc_retry_1/broken' \
     '\[1/broken .* => succeeded'
 

tests/functional/spawn-on-demand/05-stop-flow/flow.cylc

@@ -10,5 +10,5 @@
     [[bar]]
          script = """
              cylc stop --flow=1 ${CYLC_WORKFLOW_ID}
-             cylc__job__poll_grep_workflow_log 'Command succeeded: stop'
+             cylc__job__poll_grep_workflow_log 'Command actioned: stop'
          """

tests/functional/spawn-on-demand/06-stop-flow-2/flow.cylc

@@ -14,7 +14,7 @@
          script = """
 if (( CYLC_TASK_SUBMIT_NUMBER == 2 )); then
    cylc stop --flow=1 ${CYLC_WORKFLOW_ID}
-   cylc__job__poll_grep_workflow_log "Command succeeded: stop"
+   cylc__job__poll_grep_workflow_log "Command actioned: stop"
 fi
          """
     [[baz]]

tests/integration/test_resolvers.py

@@ -234,6 +234,6 @@ async def test_stop(
         resolvers.stop(StopMode.REQUEST_CLEAN)
         one.process_command_queue()
         assert log_filter(
-            log, level=logging.INFO, contains="Command succeeded: stop"
+            log, level=logging.INFO, contains="Command actioned: stop"
         )
         assert one.stop_mode == StopMode.REQUEST_CLEAN

tests/unit/scripts/test_lint.py

@@ -6,7 +6,8 @@
 # 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.
-#
+#PID: 12632 Threads: 1 RSS: 1044
+
 # 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
@@ -237,26 +238,10 @@ def test_inherit_lowercase_not_match_none(create_testable_file, inherit_line):
     assert 'S007' not in result.out
 
 
-@pytest.mark.parametrize(
-    'number', range(1, len(STYLE_CHECKS) + 1)
-)
-def test_check_cylc_file_lint(create_testable_file, number):
-    try:
-        result, _ = create_testable_file(
-            LINT_TEST_FILE, ['style'])
-        assert f'S{(number):03d}' in result.out
-    except AssertionError:
-        breakpoint()
-        try:
-            result, _ = create_testable_file(
-                '#!jinja2\n' + LINT_TEST_FILE, ['style'])
-            assert f'S{(number):03d}' in result.out
-        except AssertionError:
-            breakpoint()
-            raise AssertionError(
-                f'missing error number S{number:03d}:'
-                f'{[*STYLE_CHECKS.keys()][number].pattern}'
-            )
+def test_check_cylc_file_lint(create_testable_file):
+    result, _ = create_testable_file(LINT_TEST_FILE, ['style'])
+    for number in range(1, len(STYLE_CHECKS) + 1):
+        assert f'S{number:03d}' in result.out
 
 
 @pytest.mark.parametrize(
@@ -276,6 +261,12 @@ def test_check_exclusions(create_testable_file, exclusion):
         assert item not in result.out
 
 
+def test_check_cylc_file_jinja2_comments(create_testable_file):
+    # Repalce the '# {{' line to be '{# {{' which should not be a warning
+    result, _ = create_testable_file('{# {{ foo }} #}', ['style'])
+    assert 'S011' not in result.out
+
+
 @pytest.fixture
 def create_testable_dir(tmp_path):
     test_file = (tmp_path / 'suite.rc')