From a1f9398e6ffdb21f1e5933a8f047c45f3f03c80a Mon Sep 17 00:00:00 2001 From: ColemanTom <15375218+ColemanTom@users.noreply.github.com> Date: Wed, 17 May 2023 17:22:19 +1000 Subject: [PATCH 1/7] Fix S011 lint (#5536) Fix S011 lint * Cylc does not parse Jinja2 when inside a Jinja2 comment. * Improve test_check_cylc_file_jinja2_comments --- cylc/flow/scripts/lint.py | 4 ++-- tests/unit/scripts/test_lint.py | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/cylc/flow/scripts/lint.py b/cylc/flow/scripts/lint.py index f420eb24f89..18a72ec568e 100755 --- a/cylc/flow/scripts/lint.py +++ b/cylc/flow/scripts/lint.py @@ -152,8 +152,8 @@ 'url': 'https://github.com/cylc/cylc-flow/issues/3825', 'index': 10 }, - re.compile(r'#.*?{[{%]'): { - 'short': 'Cylc will process commented Jinja2!', + re.compile(r'(? Date: Wed, 17 May 2023 17:35:21 +1000 Subject: [PATCH 2/7] cylc lint non zero code from warnings (#5546) cylc lint now returns error code by default for issues * Previously it would provide a return code 0 always. * By default, it will now return 1 if an issue is found. * New flag --exit-zero changes back to original behaviour. --- CHANGES.md | 6 ++++++ cylc/flow/scripts/lint.py | 22 ++++++++++++++++++++-- tests/functional/cylc-lint/00.lint.t | 22 ++++++++++++++-------- tests/functional/cylc-lint/01.lint-toml.t | 6 +++--- 4 files changed, 43 insertions(+), 13 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 8acab64893c..bcec72c7d5b 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -21,6 +21,12 @@ for `cylc play` when called by `cylc vip` or `cylc vr`. ------------------------------------------------------------------------------- ## __cylc-8.1.5 (Awaiting Release)__ +### 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) - diff --git a/cylc/flow/scripts/lint.py b/cylc/flow/scripts/lint.py index 18a72ec568e..59fdb5b9ae2 100755 --- a/cylc/flow/scripts/lint.py +++ b/cylc/flow/scripts/lint.py @@ -31,6 +31,9 @@ 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. """ @@ -38,6 +41,7 @@ from optparse import Values from pathlib import Path import re +import sys import tomli from typing import Generator, Union @@ -666,6 +670,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 @@ -678,7 +689,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: @@ -724,7 +735,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') @@ -762,6 +775,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) diff --git a/tests/functional/cylc-lint/00.lint.t b/tests/functional/cylc-lint/00.lint.t index cafaef32d27..f4e1c80b394 100644 --- a/tests/functional/cylc-lint/00.lint.t +++ b/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 diff --git a/tests/functional/cylc-lint/01.lint-toml.t b/tests/functional/cylc-lint/01.lint-toml.t index 5f0d13ee17d..c31ebd1c4ea 100644 --- a/tests/functional/cylc-lint/01.lint-toml.t +++ b/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" From da65b79799abdfd0f85bcd1d30b3baf4c78b3348 Mon Sep 17 00:00:00 2001 From: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> Date: Wed, 17 May 2023 11:30:56 +0100 Subject: [PATCH 3/7] Logging: say command actioned instead of succeeded --- cylc/flow/scheduler.py | 7 ++++--- tests/functional/cli/03-set-verbosity.t | 2 +- tests/functional/hold-release/00-workflow/flow.cylc | 2 +- tests/functional/hold-release/05-release.t | 2 +- tests/functional/pause-resume/00-workflow/flow.cylc | 2 +- .../functional/pause-resume/12-pause-then-retry/flow.cylc | 2 +- tests/functional/reload/25-xtriggers.t | 2 +- tests/functional/spawn-on-demand/05-stop-flow/flow.cylc | 2 +- tests/functional/spawn-on-demand/06-stop-flow-2/flow.cylc | 2 +- tests/integration/test_resolvers.py | 2 +- 10 files changed, 13 insertions(+), 12 deletions(-) diff --git a/cylc/flow/scheduler.py b/cylc/flow/scheduler.py index 9acac8bd283..7180dbf65d3 100644 --- a/cylc/flow/scheduler.py +++ b/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() diff --git a/tests/functional/cli/03-set-verbosity.t b/tests/functional/cli/03-set-verbosity.t index a93b20e7ea5..85c555fc337 100755 --- a/tests/functional/cli/03-set-verbosity.t +++ b/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 diff --git a/tests/functional/hold-release/00-workflow/flow.cylc b/tests/functional/hold-release/00-workflow/flow.cylc index c9829135240..4afdbc92980 100644 --- a/tests/functional/hold-release/00-workflow/flow.cylc +++ b/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]] diff --git a/tests/functional/hold-release/05-release.t b/tests/functional/hold-release/05-release.t index 0dd50fd3f39..805fe2395cc 100755 --- a/tests/functional/hold-release/05-release.t +++ b/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 diff --git a/tests/functional/pause-resume/00-workflow/flow.cylc b/tests/functional/pause-resume/00-workflow/flow.cylc index b868b8cc605..57a0a24aed5 100644 --- a/tests/functional/pause-resume/00-workflow/flow.cylc +++ b/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]] diff --git a/tests/functional/pause-resume/12-pause-then-retry/flow.cylc b/tests/functional/pause-resume/12-pause-then-retry/flow.cylc index 84ee88140fe..c732dc3bdc9 100644 --- a/tests/functional/pause-resume/12-pause-then-retry/flow.cylc +++ b/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" diff --git a/tests/functional/reload/25-xtriggers.t b/tests/functional/reload/25-xtriggers.t index 0e30922e0c8..8fd1505fe6d 100644 --- a/tests/functional/reload/25-xtriggers.t +++ b/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' diff --git a/tests/functional/spawn-on-demand/05-stop-flow/flow.cylc b/tests/functional/spawn-on-demand/05-stop-flow/flow.cylc index a997b7a35b8..cac2ddf8009 100644 --- a/tests/functional/spawn-on-demand/05-stop-flow/flow.cylc +++ b/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' """ diff --git a/tests/functional/spawn-on-demand/06-stop-flow-2/flow.cylc b/tests/functional/spawn-on-demand/06-stop-flow-2/flow.cylc index 94bb97f296b..9c7da97e974 100644 --- a/tests/functional/spawn-on-demand/06-stop-flow-2/flow.cylc +++ b/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]] diff --git a/tests/integration/test_resolvers.py b/tests/integration/test_resolvers.py index 6301861b058..2f452cda760 100644 --- a/tests/integration/test_resolvers.py +++ b/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 From 2a67452db0fb2bfd77e9c69aa9cc1bad5d49ee72 Mon Sep 17 00:00:00 2001 From: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> Date: Wed, 17 May 2023 12:16:10 +0100 Subject: [PATCH 4/7] Fix cylc lint commented-out Jinja2 bug --- cylc/flow/scripts/lint.py | 5 +---- tests/unit/scripts/test_lint.py | 17 ++++------------- 2 files changed, 5 insertions(+), 17 deletions(-) diff --git a/cylc/flow/scripts/lint.py b/cylc/flow/scripts/lint.py index 59fdb5b9ae2..bdcd6a44aca 100755 --- a/cylc/flow/scripts/lint.py +++ b/cylc/flow/scripts/lint.py @@ -503,10 +503,7 @@ def check_cylc_file( check.findall(line) and ( not line.strip().startswith('#') - or ( - 'commented Jinja2!' in message['short'] - and check.findall(line) - ) + or message['index'] == 11 # commented-out Jinja2 ) ): count += 1 diff --git a/tests/unit/scripts/test_lint.py b/tests/unit/scripts/test_lint.py index 5f1a36e96d0..9683b2c411f 100644 --- a/tests/unit/scripts/test_lint.py +++ b/tests/unit/scripts/test_lint.py @@ -233,19 +233,10 @@ def test_inherit_lowercase_not_match_none(create_testable_file, inherit_line): assert 'S007' not in result.out -@pytest.mark.parametrize( - 'number', range(len(STYLE_CHECKS)) -) -def test_check_cylc_file_lint(create_testable_file, number): - try: - result, _ = create_testable_file( - LINT_TEST_FILE, ['style']) - assert f'S{(number + 1):03d}' in result.out - except AssertionError: - 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( From a262c667096bc2447c94c21222021f4df839d674 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Wed, 17 May 2023 08:59:47 +0100 Subject: [PATCH 5/7] Respond to review --- cylc/flow/scripts/lint.py | 15 ++++---- tests/unit/scripts/test_lint.py | 62 ++++++++++++++++++++++----------- 2 files changed, 47 insertions(+), 30 deletions(-) diff --git a/cylc/flow/scripts/lint.py b/cylc/flow/scripts/lint.py index cdb1195b634..7a090060539 100755 --- a/cylc/flow/scripts/lint.py +++ b/cylc/flow/scripts/lint.py @@ -446,7 +446,7 @@ def parse_checks(check_args, ignores=None, max_line_len=None, reference=False): meta.update({'index': index}) if f'{purpose}{index:03d}' not in ignores: parsedchecks.update({pattern: meta}) - if 'S' in purpose and "S008" not in ignores: + if 'S' in purpose and "S012" not in ignores: if not max_line_len: max_line_len = 130 regex = r"^.{" + str(max_line_len) + r"}" @@ -460,7 +460,7 @@ def parse_checks(check_args, ignores=None, max_line_len=None, reference=False): parsedchecks[re.compile(regex)] = { 'short': msg, 'url': STYLE_GUIDE + 'line-length-and-continuation', - 'index': len(STYLE_GUIDE) + 1, + 'index': 12, 'purpose': 'S' } return parsedchecks @@ -494,10 +494,7 @@ def check_cylc_file( check.findall(line) and ( not line.strip().startswith('#') - or ( - 'commented Jinja2!' in message['short'] - and check.findall(line) - ) + or 'commented Jinja2!' in message['short'] ) ): count += 1 @@ -541,7 +538,7 @@ def get_cylc_files( yield path -def sort_checks(check): +def get_sort_key(check): """Return check purpose and index for sorting: """ return (check[1]['purpose'], check[1]['index']) @@ -555,7 +552,7 @@ def get_reference_rst(checks): """ output = '' current_checkset = '' - for check, meta in sorted(checks.items(), key=sort_checks): + for check, meta in sorted(checks.items(), key=get_sort_key): # Check if the purpose has changed - if so create a new # section title: if meta['purpose'] != current_checkset: @@ -592,7 +589,7 @@ def get_reference_text(checks): """ output = '' current_checkset = '' - for check, meta in sorted(checks.items(), key=sort_checks): + for check, meta in sorted(checks.items(), key=get_sort_key): # Check if the purpose has changed - if so create a new # section title: if meta['purpose'] != current_checkset: diff --git a/tests/unit/scripts/test_lint.py b/tests/unit/scripts/test_lint.py index a83f280a8fa..86bf35a54ec 100644 --- a/tests/unit/scripts/test_lint.py +++ b/tests/unit/scripts/test_lint.py @@ -24,7 +24,6 @@ import re from cylc.flow.scripts.lint import ( - STYLE_CHECKS, check_cylc_file, get_cylc_files, get_reference_rst, @@ -33,12 +32,12 @@ get_upgrader_info, merge_cli_with_tomldata, parse_checks, - sort_checks, + get_sort_key, validate_toml_items ) from cylc.flow.exceptions import CylcError - +STYLE_CHECKS = parse_checks(['style']) UPG_CHECKS = parse_checks(['728']) TEST_FILE = """ @@ -163,8 +162,11 @@ """ LINT_TEST_FILE += ( - '\nscript = the quick brown fox jumps over the lazy dog ' - 'until it becomes clear that this line is far longer the 79 characters.') + '\nscript = the quick brown fox jumps over the lazy dog,' + ' Sphinx of black quartz hear my vow,' + ' Lorum ipsum doobity doo' + ' until it becomes clear that this line' + ' is far longer the 79 characters.') @pytest.fixture() @@ -236,18 +238,25 @@ def test_inherit_lowercase_not_match_none(create_testable_file, inherit_line): @pytest.mark.parametrize( - 'number', range(len(STYLE_CHECKS)) + '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 + 1):03d}' in result.out + assert f'S{(number):03d}' in result.out except AssertionError: - raise AssertionError( - f'missing error number S{number:03d}:' - f'{[*STYLE_CHECKS.keys()][number].pattern}' - ) + 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}' + ) @pytest.mark.parametrize( @@ -283,11 +292,11 @@ def create_testable_dir(tmp_path): @pytest.mark.parametrize( - 'number', range(len(UPG_CHECKS)) + 'number', range(1, len(UPG_CHECKS) + 1) ) def test_check_cylc_file_inplace(create_testable_dir, number): try: - assert f'[U{number + 1:03d}]' in create_testable_dir + assert f'[U{number:03d}]' in create_testable_dir except AssertionError: raise AssertionError( f'missing error number {number:03d}:7-to-8 - ' @@ -535,16 +544,27 @@ def test_invalid_tomlfile(tmp_path): ) def test_parse_checks_reference_mode(ref, expect): result = parse_checks(['style'], reference=ref) - key = [i for i in result.keys()][-1] + key = list(result.keys())[-1] value = result[key] assert expect in value['short'] def test_checks_are_sorted(): - checks = {} - for i in range(2): - for origin in [STYLE_CHECKS, UPG_CHECKS]: - checks[[k for k in origin][i]] = [v for v in origin.values()][i] - sorted_checks = sorted(checks.items(), key=sort_checks) - result = [i[1]['purpose'] + str(i[1]['index']) for i in sorted_checks] - assert result == ['S1', 'S2', 'U1', 'U2'] + sorted_checks = sorted( + [*UPG_CHECKS.items(), *STYLE_CHECKS.items()], + key=get_sort_key + ) + assert [ + f"{v['purpose']}{v['index']}" for _, v in sorted_checks + ] == [ + *[f"S{n}" for n in range(1, len(STYLE_CHECKS) + 1)], + *[f"U{n}" for n in range(1, len(UPG_CHECKS) + 1)] + ] + + +@pytest.mark.parametrize( + 'test_set', [STYLE_CHECKS, UPG_CHECKS] +) +def test_lint_codes_are_sequential(test_set): + checks = [i['index'] for i in test_set.values()] + assert checks == list(range(1, len(checks) + 1)) From f7ed6aa5a2db979de2a55804633f7cd27ea100a5 Mon Sep 17 00:00:00 2001 From: WXTIM <26465611+wxtim@users.noreply.github.com> Date: Wed, 17 May 2023 19:34:21 +0100 Subject: [PATCH 6/7] centralize number used for line length check. --- cylc/flow/scripts/lint.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cylc/flow/scripts/lint.py b/cylc/flow/scripts/lint.py index 7a090060539..ee2d0ef7fa0 100755 --- a/cylc/flow/scripts/lint.py +++ b/cylc/flow/scripts/lint.py @@ -85,6 +85,7 @@ } JINJA2_FOUND_WITHOUT_SHEBANG = 'jinja2 found: no shebang (#!jinja2)' CHECKS_DESC = {'U': '7 to 8 upgrades', 'S': 'Style'} +LINE_LEN_NO = 12 STYLE_CHECKS = { re.compile(r'^\t'): { 'short': 'Use multiple spaces, not tabs', @@ -446,7 +447,7 @@ def parse_checks(check_args, ignores=None, max_line_len=None, reference=False): meta.update({'index': index}) if f'{purpose}{index:03d}' not in ignores: parsedchecks.update({pattern: meta}) - if 'S' in purpose and "S012" not in ignores: + if 'S' in purpose and f"S{LINE_LEN_NO:03d}" not in ignores: if not max_line_len: max_line_len = 130 regex = r"^.{" + str(max_line_len) + r"}" @@ -460,7 +461,7 @@ def parse_checks(check_args, ignores=None, max_line_len=None, reference=False): parsedchecks[re.compile(regex)] = { 'short': msg, 'url': STYLE_GUIDE + 'line-length-and-continuation', - 'index': 12, + 'index': LINE_LEN_NO, 'purpose': 'S' } return parsedchecks From 31b6916d95a271f21e85401edb97c03ed9d1d9f1 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Thu, 18 May 2023 12:06:52 +0100 Subject: [PATCH 7/7] `cylc play`: add suggestion for `--upgrade` in non-interactive terminal (#5535) add message on how to use --upgrade --- cylc/flow/scheduler_cli.py | 1 + 1 file changed, 1 insertion(+) diff --git a/cylc/flow/scheduler_cli.py b/cylc/flow/scheduler_cli.py index d97ca81c459..cdf249ffa38 100644 --- a/cylc/flow/scheduler_cli.py +++ b/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