Merge branch 'fix.sort_lint_listing--correct_number_for_line_len' of …

…github.com:wxtim/cylc into fix.sort_lint_listing--correct_number_for_line_len

* 'fix.sort_lint_listing--correct_number_for_line_len' of github.com:wxtim/cylc:
  `cylc play`: add suggestion for `--upgrade` in non-interactive terminal (cylc#5535)
  centralize number used for line length check.
  Respond to review
  Fix cylc lint commented-out Jinja2 bug
  Logging: say command actioned instead of succeeded
  cylc lint non zero code from warnings (cylc#5546)
  Fix S011 lint (cylc#5536)

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
 
@@ -86,11 +90,16 @@
     )
 }
 JINJA2_FOUND_WITHOUT_SHEBANG = 'jinja2 found: no shebang (#!jinja2)'
+<<<<<<< HEAD
 CHECKS_DESC = {
     'U': '7 to 8 upgrades',
     'A': 'Auto Generated 7 to 8 upgrades',
     'S': 'Style'
 }
+=======
+CHECKS_DESC = {'U': '7 to 8 upgrades', 'S': 'Style'}
+LINE_LEN_NO = 12
+>>>>>>> 17d027bf27feff0ddce3362911143157bd934989
 STYLE_CHECKS = {
     1: {
         'short': 'Use multiple spaces, not tabs',
@@ -464,8 +473,13 @@ def parse_checks(check_args, ignores=None, max_line_len=None, reference=False):
             for index, meta in ruleset.items():
                 meta.update({'purpose': purpose})
                 if f'{purpose}{index:03d}' not in ignores:
+<<<<<<< HEAD
                     parsedchecks.update({index: meta})
             if 'S' in purpose and "S012" not in ignores:
+=======
+                    parsedchecks.update({pattern: meta})
+            if 'S' in purpose and f"S{LINE_LEN_NO:03d}" not in ignores:
+>>>>>>> 17d027bf27feff0ddce3362911143157bd934989
                 if not max_line_len:
                     max_line_len = 130
                 regex = r"^.{" + str(max_line_len) + r"}"
@@ -479,7 +493,11 @@ def parse_checks(check_args, ignores=None, max_line_len=None, reference=False):
                 parsedchecks[12] = {
                     'short': msg,
                     'url': STYLE_GUIDE + 'line-length-and-continuation',
+<<<<<<< HEAD
                     FUNCTION: re.compile(regex),
+=======
+                    'index': LINE_LEN_NO,
+>>>>>>> 17d027bf27feff0ddce3362911143157bd934989
                     'purpose': 'S'
                 }
     return parsedchecks
@@ -513,7 +531,11 @@ def check_cylc_file(
                 message['function'].findall(line)
                 and (
                     not line.strip().startswith('#')
+<<<<<<< HEAD
                     or 'commented Jinja2!' in message['short']
+=======
+                    or message['index'] == 11  # commented-out Jinja2
+>>>>>>> 17d027bf27feff0ddce3362911143157bd934989
                 )
             ):
                 count += 1
@@ -557,6 +579,15 @@ def get_cylc_files(
                 yield path
 
 
+<<<<<<< HEAD
+=======
+def get_sort_key(check):
+    """Return check purpose and index for sorting:
+    """
+    return (check[1]['purpose'], check[1]['index'])
+
+
+>>>>>>> 17d027bf27feff0ddce3362911143157bd934989
 def get_reference_rst(checks):
     """Print a reference for checks to be carried out.
 
@@ -565,7 +596,11 @@ def get_reference_rst(checks):
     """
     output = ''
     current_checkset = ''
+<<<<<<< HEAD
     for index, meta in sorted(checks.items()):
+=======
+    for check, meta in sorted(checks.items(), key=get_sort_key):
+>>>>>>> 17d027bf27feff0ddce3362911143157bd934989
         # Check if the purpose has changed - if so create a new
         # section title:
         check = meta[FUNCTION]
@@ -603,7 +638,11 @@ def get_reference_text(checks):
     """
     output = ''
     current_checkset = ''
+<<<<<<< HEAD
     for index, meta in sorted(checks.items()):
+=======
+    for check, meta in sorted(checks.items(), key=get_sort_key):
+>>>>>>> 17d027bf27feff0ddce3362911143157bd934989
         # Check if the purpose has changed - if so create a new
         # section title:
         if meta['purpose'] != current_checkset:
@@ -678,6 +717,13 @@ def get_option_parser() -> COP:
         metavar="CODE",
         choices=tuple([f'S{i:03d}' for i in STYLE_CHECKS])
     )
+    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
 
@@ -690,7 +736,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:
@@ -736,7 +782,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')
@@ -774,6 +822,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