Skip to content

Commit 5d27413

Browse files
lint: Warn users that CYLC_VERSION={{CYLC_VERSION}} is deprecated (#5890)
* Cylc Lint to warn users that CYLC_VERSION={{CYLC_VERSION}} is deprecated. Additional warnings for ROSE_VERSION and FCM_VERSION in the same manner. * Fix check which was returning false positives Made the test for presence of lint messages optionally count those messages. * fix-flake8 * fix * Fixed merge * correction * Update cylc/flow/scripts/lint.py Co-authored-by: Oliver Sanders <[email protected]> --------- Co-authored-by: Oliver Sanders <[email protected]>
1 parent b8a55d8 commit 5d27413

File tree

3 files changed

+58
-7
lines changed

3 files changed

+58
-7
lines changed

changes.d/5890.feat.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Lint: Warn users that setting ``CYLC_VERSION``, ``ROSE_VERSION`` or
2+
``FCM_VERSION`` in the workflow config is deprecated.

cylc/flow/scripts/lint.py

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,27 @@ def check_indentation(line: str) -> bool:
320320
return bool(len(match[0]) % 4 != 0)
321321

322322

323+
CHECK_FOR_OLD_VARS = re.compile(
324+
r'CYLC_VERSION\s*=\s*\{\{\s*CYLC_VERSION\s*\}\}'
325+
r'|ROSE_VERSION\s*=\s*\{\{\s*ROSE_VERSION\s*\}\}'
326+
r'|FCM_VERSION\s*=\s*\{\{\s*FCM_VERSION\s*\}\}'
327+
)
328+
329+
330+
def list_wrapper(line: str, check: Callable) -> Optional[Dict[str, str]]:
331+
"""Take a line and a check function and return a Dict if there is a
332+
result, None otherwise.
333+
334+
Returns
335+
336+
Dict, in for {'vars': Error string}
337+
"""
338+
result = check(line)
339+
if result:
340+
return {'vars': '\n * '.join(result)}
341+
return None
342+
343+
323344
FUNCTION = 'function'
324345

325346
STYLE_GUIDE = (
@@ -655,7 +676,20 @@ def check_indentation(line: str) -> bool:
655676
'writing-workflows/runtime.html#task-event-template-variables'
656677
),
657678
FUNCTION: check_for_deprecated_task_event_template_vars,
658-
}
679+
},
680+
'U016': {
681+
'short': 'Deprecated template vars: {vars}',
682+
'rst': (
683+
'It is no longer necessary to configure the environment variables '
684+
'``CYLC_VERSION``, ``ROSE_VERSION`` or ``FCM_VERSION``.'
685+
),
686+
'url': (
687+
'https://cylc.github.io/cylc-doc/stable/html/plugins/'
688+
'cylc-rose.html#special-variables'
689+
),
690+
FUNCTION: functools.partial(
691+
list_wrapper, check=CHECK_FOR_OLD_VARS.findall),
692+
},
659693
}
660694
RULESETS = ['728', 'style', 'all']
661695
EXTRA_TOML_VALIDATION = {

tests/unit/scripts/test_lint.py

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,12 @@
9696
graph = MyFaM:finish-all => remote => !mash_theme
9797
9898
[runtime]
99+
[[root]]
100+
[[[environment]]]
101+
CYLC_VERSION={{CYLC_VERSION}}
102+
ROSE_VERSION = {{ROSE_VERSION }}
103+
FCM_VERSION = {{ FCM_VERSION }}
104+
99105
[[MyFaM]]
100106
extra log files = True
101107
{% from 'cylc.flow' import LOG %}
@@ -151,7 +157,6 @@
151157
[scheduler]
152158
153159
[[dependencies]]
154-
155160
{% set N = 009 %}
156161
{% foo %}
157162
{{foo}}
@@ -206,13 +211,22 @@ def filter_strings(items, contains):
206211
]
207212

208213

209-
def assert_contains(items, contains):
214+
def assert_contains(items, contains, instances=None):
210215
"""Pass if at least one item contains a given string."""
211-
if not filter_strings(items, contains):
216+
filtered = filter_strings(items, contains)
217+
if not filtered:
212218
raise Exception(
213219
f'Could not find: "{contains}" in:\n'
214-
+ pformat(items)
215-
)
220+
+ pformat(items))
221+
elif instances and len(filtered) != instances:
222+
raise Exception(
223+
f'Expected "{contains}" to appear {instances} times'
224+
f', got it {len(filtered)} times.')
225+
226+
227+
EXPECT_INSTANCES_OF_ERR = {
228+
16: 3,
229+
}
216230

217231

218232
@pytest.mark.parametrize(
@@ -222,7 +236,8 @@ def assert_contains(items, contains):
222236
def test_check_cylc_file_7to8(number):
223237
"""TEST File has one of each manual deprecation;"""
224238
lint = lint_text(TEST_FILE, ['728'])
225-
assert_contains(lint.messages, f'[U{number:03d}]')
239+
instances = EXPECT_INSTANCES_OF_ERR.get(number, None)
240+
assert_contains(lint.messages, f'[U{number:03d}]', instances)
226241

227242

228243
def test_check_cylc_file_7to8_has_shebang():

0 commit comments

Comments
 (0)