From 1f3dcc429b42761195d5cde715d4d121c2524191 Mon Sep 17 00:00:00 2001
From: Tim Pillinger <26465611+wxtim@users.noreply.github.com>
Date: Tue, 16 Apr 2024 12:56:40 +0100
Subject: [PATCH 01/17] delete unwanted opts after post install step.
---
cylc/rose/entry_points.py | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/cylc/rose/entry_points.py b/cylc/rose/entry_points.py
index 8da910b0..9ff0918b 100644
--- a/cylc/rose/entry_points.py
+++ b/cylc/rose/entry_points.py
@@ -75,6 +75,11 @@ def post_install(srcdir=None, opts=None, rundir=None):
if results['fileinstall']:
dump_rose_log(rundir=rundir, node=results['fileinstall'])
+ # Having dumped the config we clear rose options:
+ opts.rose_template_vars = []
+ opts.opt_conf_keys = []
+ opts.defines = []
+
return results
From d2120680c651e12ad62bde669746669f9c4aa8f6 Mon Sep 17 00:00:00 2001
From: Tim Pillinger <26465611+wxtim@users.noreply.github.com>
Date: Tue, 16 Apr 2024 13:05:24 +0100
Subject: [PATCH 02/17] Removed test because it duplicated
test_no_rose_suite_conf_in_devdir
---
tests/unit/test_functional_post_install.py | 7 -------
1 file changed, 7 deletions(-)
diff --git a/tests/unit/test_functional_post_install.py b/tests/unit/test_functional_post_install.py
index 9f8791be..a24497ba 100644
--- a/tests/unit/test_functional_post_install.py
+++ b/tests/unit/test_functional_post_install.py
@@ -353,10 +353,3 @@ def broken():
(tmp_path / 'rose-suite.conf').touch()
with pytest.raises(FileNotFoundError):
rose_fileinstall(srcdir=tmp_path, rundir=tmp_path)
-
-
-def test_cylc_no_rose(tmp_path):
- """A Cylc workflow that contains no ``rose-suite.conf`` installs OK.
- """
- from cylc.rose.entry_points import post_install
- assert post_install(srcdir=tmp_path, rundir=tmp_path) is False
From 2278f3191afaf0b9df6b9c407b541ba6ff2754ab Mon Sep 17 00:00:00 2001
From: Tim Pillinger <26465611+wxtim@users.noreply.github.com>
Date: Tue, 16 Apr 2024 13:45:12 +0100
Subject: [PATCH 03/17] added test to confim that post instlall removes Rose
settings from opts objects.
---
tests/unit/test_functional_post_install.py | 37 ++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/tests/unit/test_functional_post_install.py b/tests/unit/test_functional_post_install.py
index a24497ba..5b481bb1 100644
--- a/tests/unit/test_functional_post_install.py
+++ b/tests/unit/test_functional_post_install.py
@@ -62,6 +62,43 @@ def test_no_rose_suite_conf_in_devdir(tmp_path):
assert result is False
+def test_post_install_clears_rose_opts(tmp_path, monkeypatch):
+ """Ensure that rose opts are cleared after being dumped to
+ rose-suite-cylc-install.conf - we don't want them polluting downstream
+ Cylc configurations.
+ """
+ # Mock functions which would otherwise fail, but we aren't interested
+ # in here:
+ for func in ['rose_config_exists', 'copy_config_file']:
+ monkeypatch.setattr(
+ f'cylc.rose.entry_points.{func}', lambda *_, **__: True)
+
+ # Setup opts-like object
+ opts = SimpleNamespace(
+ rose_template_vars=['FOO=42'],
+ defines=['[section]BAR="MOUTH"'],
+ opt_conf_keys=['gules'],
+ elephants=['Dumbo', 'BaBar']
+ )
+
+ # Setup workflow folder:
+ opt = (tmp_path / 'opt')
+ opt.mkdir(parents=True)
+ (opt / 'rose-suite-gules.conf').touch()
+ (tmp_path / 'rose-suite.conf').touch()
+
+ # Function under test
+ post_install(rundir=tmp_path, opts=opts, srcdir=tmp_path)
+
+ # All the opts values for Rose Have been cleared:
+ assert opts.rose_template_vars == []
+ assert opts.opt_conf_keys == []
+ assert opts.defines == []
+
+ # All the non-rose opts have been left alone:
+ assert opts.elephants == ['Dumbo', 'BaBar']
+
+
def test_rose_fileinstall_no_config_in_folder():
# It returns false if no rose-suite.conf
assert rose_fileinstall(Path('/dev/null')) is False
From 06b5ab7cf07e63b795c28415f8a4d7d12f5db1a8 Mon Sep 17 00:00:00 2001
From: Tim Pillinger <26465611+wxtim@users.noreply.github.com>
Date: Tue, 16 Apr 2024 14:24:33 +0100
Subject: [PATCH 04/17] lower pin cylc version for this fix
---
setup.cfg | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/setup.cfg b/setup.cfg
index 6c954c36..e7114612 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -56,7 +56,7 @@ python_requires = >=3.7
include_package_data = True
install_requires =
metomi-rose>=2.1.0, <2.3.0
- cylc-flow==8.2.*
+ cylc-flow>=8.2.6
metomi-isodatetime
ansimarkup
jinja2
From f96d468ce90f36c492a537aa6c07b9f00fc1c0a0 Mon Sep 17 00:00:00 2001
From: Tim Pillinger <26465611+wxtim@users.noreply.github.com>
Date: Tue, 16 Apr 2024 15:11:23 +0100
Subject: [PATCH 05/17] Added a test for Cylc Reload overriding existing tvars
correctly Removed the runN from the tests
---
tests/conftest.py | 69 +++++++++++++++++++-
tests/functional/test_ROSE_ORIG_HOST.py | 4 +-
tests/functional/test_reinstall.py | 73 +++++++++++++++++++---
tests/functional/test_reinstall_clean.py | 6 +-
tests/functional/test_rose_fileinstall.py | 4 +-
tests/unit/test_functional_post_install.py | 44 +++----------
6 files changed, 147 insertions(+), 53 deletions(-)
diff --git a/tests/conftest.py b/tests/conftest.py
index 4031397e..82dbabf4 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -15,9 +15,14 @@
# along with this program. If not, see .
"""Functional tests for top-level function record_cylc_install_options and
"""
-
+from functools import partial
+from subprocess import run
+from shlex import split
import pytest
+from time import sleep
from types import SimpleNamespace
+from typing import TYPE_CHECKING
+from uuid import uuid4
from cylc.flow import __version__ as CYLC_VERSION
from cylc.flow.option_parsers import Options
@@ -38,6 +43,15 @@
)
+if TYPE_CHECKING:
+ from pathlib import Path
+
+
+def test_workflow_name():
+ """Return a UUID to use as a test name"""
+ return f'cylc-rose-test-{str(uuid4())[:8]}'
+
+
@pytest.fixture(scope='module')
def mod_capsys(request):
from _pytest.capture import SysCapture
@@ -128,8 +142,14 @@ def _inner(srcpath, args=None):
srcpath:
args: Dictionary of arguments.
"""
+ if not args or not args.get('workflow_name', ''):
+ id_ = test_workflow_name()
+ args = {'workflow_name': id_}
+
+ args.update({'no_run_name': True})
options = Options(install_gop(), args)()
output = SimpleNamespace()
+ output.id = args['workflow_name']
try:
cylc_install(options, str(srcpath))
@@ -197,3 +217,50 @@ def cylc_validate_cli(capsys, caplog):
@pytest.fixture(scope='module')
def mod_cylc_validate_cli(mod_capsys, mod_caplog):
return _cylc_validate_cli(mod_capsys, mod_caplog)
+
+
+@pytest.fixture
+def file_poll():
+ """Poll for the existance of a file.
+ """
+ def _inner(fpath: "Path", timeout: int = 5):
+ timeout_func(
+ fpath.exists,
+ f'file {fpath} not found after {timeout} seconds'
+ )
+ return _inner
+
+
+@pytest.fixture
+def purge_workflow(run_ok):
+ """Ensure workflow is stopped and cleaned"""
+ def _inner(id_, timeout=5):
+ run_ok(f'cylc stop {id_} --now --now')
+ clean = f'cylc clean {id_}'
+ timeout_func(
+ partial(run_ok, clean),
+ message=f'Not run after {timeout} seconds: {clean}'
+ )
+ return _inner
+
+
+@pytest.fixture
+def run_ok():
+ """Run a bash script.
+ Fail if it fails and return its output.
+ """
+ def _inner(script):
+ result = run(split(script), capture_output=True)
+ assert result.returncode == 0
+ return result
+ return _inner
+
+
+def timeout_func(func, message, timeout=5):
+ """Wrap a function in a timeout"""
+ for _ in range(timeout):
+ if func():
+ break
+ sleep(1)
+ else:
+ raise TimeoutError(message)
diff --git a/tests/functional/test_ROSE_ORIG_HOST.py b/tests/functional/test_ROSE_ORIG_HOST.py
index b5037da8..743bb518 100644
--- a/tests/functional/test_ROSE_ORIG_HOST.py
+++ b/tests/functional/test_ROSE_ORIG_HOST.py
@@ -120,7 +120,7 @@ def fixture_install_flow(
)
install_conf_path = (
fixture_provide_flow['flowpath'] /
- 'runN/opt/rose-suite-cylc-install.conf'
+ 'opt/rose-suite-cylc-install.conf'
)
text = install_conf_path.read_text()
text = re.sub('ROSE_ORIG_HOST=.*', 'ROSE_ORIG_HOST=foo', text)
@@ -143,7 +143,7 @@ def test_cylc_validate_srcdir(fixture_install_flow, mod_cylc_validate_cli):
def test_cylc_validate_rundir(fixture_install_flow, mod_cylc_validate_cli):
"""Sanity check that workflow validates:
"""
- flowpath = fixture_install_flow['flowpath'] / 'runN'
+ flowpath = fixture_install_flow['flowpath']
result = mod_cylc_validate_cli(flowpath)
assert 'ROSE_ORIG_HOST (env) is:' in result.logging
diff --git a/tests/functional/test_reinstall.py b/tests/functional/test_reinstall.py
index db4c5e46..10076fbc 100644
--- a/tests/functional/test_reinstall.py
+++ b/tests/functional/test_reinstall.py
@@ -28,7 +28,9 @@
"""
import pytest
+from shlex import split
import shutil
+from subprocess import run
from itertools import product
from pathlib import Path
@@ -117,14 +119,14 @@ def test_cylc_install_run(fixture_install_flow):
'file_, expect',
[
(
- 'run1/rose-suite.conf', (
+ 'rose-suite.conf', (
'# Config Options \'b c (cylc-install)\' from CLI appended to'
' options already in `rose-suite.conf`.\n'
'opts=a b c (cylc-install)\n'
)
),
(
- 'run1/opt/rose-suite-cylc-install.conf', (
+ 'opt/rose-suite-cylc-install.conf', (
'# This file records CLI Options.\n\n'
'!opts=b c\n'
f'\n[env]\n#{ROHIOS}\nROSE_ORIG_HOST={HOST}\n'
@@ -152,7 +154,7 @@ def fixture_reinstall_flow(
"""
monkeymodule.delenv('ROSE_SUITE_OPT_CONF_KEYS', raising=False)
result = mod_cylc_reinstall_cli(
- f'{fixture_provide_flow["test_flow_name"]}/run1',
+ f'{fixture_provide_flow["test_flow_name"]}',
{
'opt_conf_keys': ['d']
}
@@ -171,14 +173,14 @@ def test_cylc_reinstall_run(fixture_reinstall_flow):
'file_, expect',
[
(
- 'run1/rose-suite.conf', (
+ 'rose-suite.conf', (
'# Config Options \'b c d (cylc-install)\' from CLI appended '
'to options already in `rose-suite.conf`.\n'
'opts=a b c d (cylc-install)\n'
)
),
(
- 'run1/opt/rose-suite-cylc-install.conf', (
+ 'opt/rose-suite-cylc-install.conf', (
'# This file records CLI Options.\n\n'
'!opts=b c d\n'
f'\n[env]\n#{ROHIOS}\nROSE_ORIG_HOST={HOST}\n'
@@ -213,7 +215,7 @@ def fixture_reinstall_flow2(
'opts=z\n'
)
result = mod_cylc_reinstall_cli(
- f'{fixture_provide_flow["test_flow_name"]}/run1'
+ f'{fixture_provide_flow["test_flow_name"]}'
)
yield {
'fixture_provide_flow': fixture_provide_flow,
@@ -229,14 +231,14 @@ def test_cylc_reinstall_run2(fixture_reinstall_flow2):
'file_, expect',
[
(
- 'run1/rose-suite.conf', (
+ 'rose-suite.conf', (
'# Config Options \'b c d (cylc-install)\' from CLI appended '
'to options already in `rose-suite.conf`.\n'
'opts=z b c d (cylc-install)\n'
)
),
(
- 'run1/opt/rose-suite-cylc-install.conf', (
+ 'opt/rose-suite-cylc-install.conf', (
'# This file records CLI Options.\n\n'
'!opts=b c d\n'
f'\n[env]\n#{ROHIOS}\nROSE_ORIG_HOST={HOST}\n'
@@ -280,3 +282,58 @@ def test_reinstall_workflow(tmp_path):
expect = sorted(['send rose-suite.conf', 'send flow.cylc'])
assert sorted(stdout.split('\n')) == expect
+
+
+def test_reinstall_overrides(
+ cylc_install_cli,
+ cylc_reinstall_cli,
+ file_poll,
+ tmp_path,
+ purge_workflow,
+ run_ok
+):
+ """When reinstalling and reloading the new installation are picked up.
+
+ > cylc install this -S 'var=CLIinstall'
+ > cylc play this --pause
+ > cylc reinstall this -S 'var=CLIreinstall'
+ > cylc play this --pause
+
+ See https://github.com/cylc/cylc-flow/issues/5968
+ """
+ (tmp_path / 'flow.cylc').write_text(
+ '#!jinja2\n'
+ '[scheduling]\n'
+ ' [[graph]]\n'
+ ' R1 = foo\n'
+ '[runtime]\n'
+ ' [[foo]]\n'
+ ' script = cylc message -- {{var}}')
+ (tmp_path / 'rose-suite.conf').write_text(
+ '[template variables]\nvar="rose-suite.conf"')
+
+ # Install workflow.
+ install_results = cylc_install_cli(
+ tmp_path, {'rose_template_vars': ['var="CLIinstall"']})
+ assert install_results.ret == 0
+
+ # Play workflow
+ run_ok(f'cylc play --pause {install_results.id}')
+
+ # Reinstall the workflow:
+ reinstall_results = cylc_reinstall_cli(
+ install_results.id,
+ {'rose_template_vars': ['var="CLIreinstall"']})
+ assert reinstall_results.ret == 0
+
+ # Reload the workflow:
+ run_ok(f'cylc reload {install_results.id}')
+
+ # The config being run has been modified:
+ run_dir = Path.home() / 'cylc-run' / install_results.id
+ config_log = (run_dir / 'log/config/02-reload-01.cylc')
+ file_poll(config_log)
+
+ assert 'cylc message -- CLIreinstall' in config_log.read_text()
+
+ purge_workflow(install_results.id)
diff --git a/tests/functional/test_reinstall_clean.py b/tests/functional/test_reinstall_clean.py
index 2883afb5..f443d0ef 100644
--- a/tests/functional/test_reinstall_clean.py
+++ b/tests/functional/test_reinstall_clean.py
@@ -108,7 +108,7 @@ def test_cylc_install_run(fixture_install_flow):
'file_, expect',
[
(
- 'run1/opt/rose-suite-cylc-install.conf', (
+ 'opt/rose-suite-cylc-install.conf', (
'# This file records CLI Options.\n\n'
'!opts=bar\n\n'
'[env]\n'
@@ -141,7 +141,7 @@ def fixture_reinstall_flow(
"""
monkeymodule.delenv('ROSE_SUITE_OPT_CONF_KEYS', raising=False)
result = mod_cylc_reinstall_cli(
- f'{fixture_provide_flow["test_flow_name"]}/run1',
+ f'{fixture_provide_flow["test_flow_name"]}',
{
'opt_conf_keys': ['baz'],
'defines': ['[env]BAR=2'],
@@ -162,7 +162,7 @@ def test_cylc_reinstall_run(fixture_reinstall_flow):
'file_, expect',
[
(
- 'run1/opt/rose-suite-cylc-install.conf', (
+ 'opt/rose-suite-cylc-install.conf', (
'# This file records CLI Options.\n\n'
'!opts=baz\n\n'
'[env]\n'
diff --git a/tests/functional/test_rose_fileinstall.py b/tests/functional/test_rose_fileinstall.py
index e11a496f..0b134122 100644
--- a/tests/functional/test_rose_fileinstall.py
+++ b/tests/functional/test_rose_fileinstall.py
@@ -85,7 +85,7 @@ def test_rose_fileinstall_subfolders(fixture_install_flow):
"""File installed into a sub directory:
"""
_, datapath, _, _, destpath = fixture_install_flow
- assert ((destpath / 'runN/lib/python/lion.py').read_text() ==
+ assert ((destpath / 'lib/python/lion.py').read_text() ==
(datapath / 'lion.py').read_text())
@@ -93,7 +93,7 @@ def test_rose_fileinstall_concatenation(fixture_install_flow):
"""Multiple files concatenated on install(source contained wildcard):
"""
_, datapath, _, _, destpath = fixture_install_flow
- assert ((destpath / 'runN/data').read_text() ==
+ assert ((destpath / 'data').read_text() ==
((datapath / 'randoms1.data').read_text() +
(datapath / 'randoms3.data').read_text()
))
diff --git a/tests/unit/test_functional_post_install.py b/tests/unit/test_functional_post_install.py
index 5b481bb1..9f8791be 100644
--- a/tests/unit/test_functional_post_install.py
+++ b/tests/unit/test_functional_post_install.py
@@ -62,43 +62,6 @@ def test_no_rose_suite_conf_in_devdir(tmp_path):
assert result is False
-def test_post_install_clears_rose_opts(tmp_path, monkeypatch):
- """Ensure that rose opts are cleared after being dumped to
- rose-suite-cylc-install.conf - we don't want them polluting downstream
- Cylc configurations.
- """
- # Mock functions which would otherwise fail, but we aren't interested
- # in here:
- for func in ['rose_config_exists', 'copy_config_file']:
- monkeypatch.setattr(
- f'cylc.rose.entry_points.{func}', lambda *_, **__: True)
-
- # Setup opts-like object
- opts = SimpleNamespace(
- rose_template_vars=['FOO=42'],
- defines=['[section]BAR="MOUTH"'],
- opt_conf_keys=['gules'],
- elephants=['Dumbo', 'BaBar']
- )
-
- # Setup workflow folder:
- opt = (tmp_path / 'opt')
- opt.mkdir(parents=True)
- (opt / 'rose-suite-gules.conf').touch()
- (tmp_path / 'rose-suite.conf').touch()
-
- # Function under test
- post_install(rundir=tmp_path, opts=opts, srcdir=tmp_path)
-
- # All the opts values for Rose Have been cleared:
- assert opts.rose_template_vars == []
- assert opts.opt_conf_keys == []
- assert opts.defines == []
-
- # All the non-rose opts have been left alone:
- assert opts.elephants == ['Dumbo', 'BaBar']
-
-
def test_rose_fileinstall_no_config_in_folder():
# It returns false if no rose-suite.conf
assert rose_fileinstall(Path('/dev/null')) is False
@@ -390,3 +353,10 @@ def broken():
(tmp_path / 'rose-suite.conf').touch()
with pytest.raises(FileNotFoundError):
rose_fileinstall(srcdir=tmp_path, rundir=tmp_path)
+
+
+def test_cylc_no_rose(tmp_path):
+ """A Cylc workflow that contains no ``rose-suite.conf`` installs OK.
+ """
+ from cylc.rose.entry_points import post_install
+ assert post_install(srcdir=tmp_path, rundir=tmp_path) is False
From 2dd84cc4f82531e6ac14ff3bedb8d9a7b4009e33 Mon Sep 17 00:00:00 2001
From: Tim Pillinger <26465611+wxtim@users.noreply.github.com>
Date: Wed, 17 Apr 2024 12:51:30 +0100
Subject: [PATCH 06/17] Update tests/conftest.py
Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>
---
tests/conftest.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/conftest.py b/tests/conftest.py
index 82dbabf4..cb1a2778 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -249,7 +249,7 @@ def run_ok():
"""Run a bash script.
Fail if it fails and return its output.
"""
- def _inner(script):
+ def _inner(script: str):
result = run(split(script), capture_output=True)
assert result.returncode == 0
return result
From aecea88a5d95044c05c239f3621a231e18050d6a Mon Sep 17 00:00:00 2001
From: Tim Pillinger <26465611+wxtim@users.noreply.github.com>
Date: Wed, 17 Apr 2024 12:51:45 +0100
Subject: [PATCH 07/17] Update setup.cfg
Co-authored-by: Oliver Sanders
---
setup.cfg | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/setup.cfg b/setup.cfg
index e7114612..05f54b89 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -56,7 +56,7 @@ python_requires = >=3.7
include_package_data = True
install_requires =
metomi-rose>=2.1.0, <2.3.0
- cylc-flow>=8.2.6
+ cylc-flow>=8.2.6, <8.3
metomi-isodatetime
ansimarkup
jinja2
From dd08a641df149e426d12f05b2db233d9f8ea7ba4 Mon Sep 17 00:00:00 2001
From: Tim Pillinger <26465611+wxtim@users.noreply.github.com>
Date: Wed, 17 Apr 2024 12:52:14 +0100
Subject: [PATCH 08/17] Update cylc/rose/entry_points.py
Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>
---
cylc/rose/entry_points.py | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/cylc/rose/entry_points.py b/cylc/rose/entry_points.py
index 9ff0918b..97e5a29c 100644
--- a/cylc/rose/entry_points.py
+++ b/cylc/rose/entry_points.py
@@ -75,7 +75,9 @@ def post_install(srcdir=None, opts=None, rundir=None):
if results['fileinstall']:
dump_rose_log(rundir=rundir, node=results['fileinstall'])
- # Having dumped the config we clear rose options:
+ # Having dumped the config we clear rose options
+ # as they do not apply after this.
+ # see https://github.com/cylc/cylc-rose/pull/312
opts.rose_template_vars = []
opts.opt_conf_keys = []
opts.defines = []
From 5c8b05f9abb045e864b7953c6e4be1aaf9caec95 Mon Sep 17 00:00:00 2001
From: Tim Pillinger <26465611+wxtim@users.noreply.github.com>
Date: Wed, 17 Apr 2024 13:00:59 +0100
Subject: [PATCH 09/17] response to review
---
cylc/rose/entry_points.py | 4 ++--
tests/functional/test_reinstall.py | 20 ++++++++++----------
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/cylc/rose/entry_points.py b/cylc/rose/entry_points.py
index 9ff0918b..d906ab1a 100644
--- a/cylc/rose/entry_points.py
+++ b/cylc/rose/entry_points.py
@@ -54,12 +54,12 @@ def __str__(self):
return msg
-def pre_configure(srcdir=None, opts=None, rundir=None):
+def pre_configure(srcdir, opts, rundir=None):
srcdir, rundir = paths_to_pathlib([srcdir, rundir])
return get_rose_vars(srcdir=srcdir, opts=opts)
-def post_install(srcdir=None, opts=None, rundir=None):
+def post_install(srcdir, opts, rundir):
if not rose_config_exists(srcdir, opts):
return False
srcdir, rundir = paths_to_pathlib([srcdir, rundir])
diff --git a/tests/functional/test_reinstall.py b/tests/functional/test_reinstall.py
index 10076fbc..9106e46b 100644
--- a/tests/functional/test_reinstall.py
+++ b/tests/functional/test_reinstall.py
@@ -28,9 +28,8 @@
"""
import pytest
-from shlex import split
import shutil
-from subprocess import run
+from textwrap import dedent
from itertools import product
from pathlib import Path
@@ -301,14 +300,15 @@ def test_reinstall_overrides(
See https://github.com/cylc/cylc-flow/issues/5968
"""
- (tmp_path / 'flow.cylc').write_text(
- '#!jinja2\n'
- '[scheduling]\n'
- ' [[graph]]\n'
- ' R1 = foo\n'
- '[runtime]\n'
- ' [[foo]]\n'
- ' script = cylc message -- {{var}}')
+ (tmp_path / 'flow.cylc').write_text(dedent("""
+ #!jinja2
+ [scheduling]\n
+ [[graph]]\n
+ R1 = foo\n
+ [runtime]\n
+ [[foo]]\n
+ script = cylc message -- {{var}}
+ """))
(tmp_path / 'rose-suite.conf').write_text(
'[template variables]\nvar="rose-suite.conf"')
From f3e62ef03954ff66ffa8516d47aa73ed6ee550fb Mon Sep 17 00:00:00 2001
From: Tim Pillinger <26465611+wxtim@users.noreply.github.com>
Date: Thu, 18 Apr 2024 09:51:01 +0100
Subject: [PATCH 10/17] Update setup.cfg
Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>
---
setup.cfg | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/setup.cfg b/setup.cfg
index 05f54b89..31373618 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -56,7 +56,7 @@ python_requires = >=3.7
include_package_data = True
install_requires =
metomi-rose>=2.1.0, <2.3.0
- cylc-flow>=8.2.6, <8.3
+ cylc-flow>8.2.5, <8.3
metomi-isodatetime
ansimarkup
jinja2
From 78e7f94271cb285cdb11303e99337af7eafe0f78 Mon Sep 17 00:00:00 2001
From: Tim Pillinger <26465611+wxtim@users.noreply.github.com>
Date: Wed, 24 Apr 2024 15:33:19 +0100
Subject: [PATCH 11/17] unset ROSE_SUITE_OPT_CONF_KEYS
---
cylc/rose/entry_points.py | 2 ++
cylc/rose/utilities.py | 7 ++++---
tests/unit/test_functional_post_install.py | 4 ++--
3 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/cylc/rose/entry_points.py b/cylc/rose/entry_points.py
index e4647e26..029bdd4b 100644
--- a/cylc/rose/entry_points.py
+++ b/cylc/rose/entry_points.py
@@ -27,6 +27,7 @@
from metomi.rose.config import ConfigLoader, ConfigDumper
from cylc.rose.utilities import (
ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING,
+ ROSE_SUITE_OPT_CONF_KEYS,
deprecation_warnings,
dump_rose_log,
get_rose_vars_from_config_node,
@@ -81,6 +82,7 @@ def post_install(srcdir, opts, rundir):
opts.rose_template_vars = []
opts.opt_conf_keys = []
opts.defines = []
+ os.unsetenv(ROSE_SUITE_OPT_CONF_KEYS)
return results
diff --git a/cylc/rose/utilities.py b/cylc/rose/utilities.py
index 586812bc..6b8e382f 100644
--- a/cylc/rose/utilities.py
+++ b/cylc/rose/utilities.py
@@ -38,6 +38,7 @@
from optparse import Values
+ROSE_SUITE_OPT_CONF_KEYS = "ROSE_SUITE_OPT_CONF_KEYS"
SECTIONS = {'jinja2:suite.rc', 'empy:suite.rc', 'template variables'}
SET_BY_CYLC = 'set by Cylc'
ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING = (
@@ -257,7 +258,7 @@ def rose_config_tree_loader(srcdir=None, opts=None):
opt_conf_keys = []
# get optional config key set as environment variable:
- opt_conf_keys_env = os.getenv("ROSE_SUITE_OPT_CONF_KEYS")
+ opt_conf_keys_env = os.getenv(ROSE_SUITE_OPT_CONF_KEYS)
if opt_conf_keys_env:
opt_conf_keys += shlex.split(opt_conf_keys_env)
@@ -591,8 +592,8 @@ def merge_opts(config, opt_conf_keys):
all_opt_conf_keys = []
if 'opts' in config:
all_opt_conf_keys.append(config['opts'].value)
- if "ROSE_SUITE_OPT_CONF_KEYS" in os.environ:
- all_opt_conf_keys.append(os.environ["ROSE_SUITE_OPT_CONF_KEYS"])
+ if ROSE_SUITE_OPT_CONF_KEYS in os.environ:
+ all_opt_conf_keys.append(os.environ[ROSE_SUITE_OPT_CONF_KEYS])
if opt_conf_keys and isinstance(opt_conf_keys, str):
all_opt_conf_keys.append(opt_conf_keys)
if opt_conf_keys and isinstance(opt_conf_keys, list):
diff --git a/tests/unit/test_functional_post_install.py b/tests/unit/test_functional_post_install.py
index 9f8791be..9ff625e6 100644
--- a/tests/unit/test_functional_post_install.py
+++ b/tests/unit/test_functional_post_install.py
@@ -58,7 +58,7 @@ def assert_rose_conf_full_equal(left, right, no_ignore=True):
def test_no_rose_suite_conf_in_devdir(tmp_path):
- result = post_install(srcdir=tmp_path)
+ result = post_install(srcdir=tmp_path, rundir=None, opts=None)
assert result is False
@@ -359,4 +359,4 @@ def test_cylc_no_rose(tmp_path):
"""A Cylc workflow that contains no ``rose-suite.conf`` installs OK.
"""
from cylc.rose.entry_points import post_install
- assert post_install(srcdir=tmp_path, rundir=tmp_path) is False
+ assert post_install(srcdir=tmp_path, rundir=tmp_path, opts={}) is False
From 9df1d59bf9abc0f285607c9eaaf11d10ab13d885 Mon Sep 17 00:00:00 2001
From: WXTIM <26465611+wxtim@users.noreply.github.com>
Date: Thu, 25 Apr 2024 15:33:16 +0100
Subject: [PATCH 12/17] fix functional tests
---
tests/conftest.py | 35 +++++++++++-------
tests/functional/test_reinstall.py | 57 ------------------------------
2 files changed, 23 insertions(+), 69 deletions(-)
diff --git a/tests/conftest.py b/tests/conftest.py
index cb1a2778..e908b88c 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -18,10 +18,10 @@
from functools import partial
from subprocess import run
from shlex import split
+from pathlib import Path
import pytest
from time import sleep
from types import SimpleNamespace
-from typing import TYPE_CHECKING
from uuid import uuid4
from cylc.flow import __version__ as CYLC_VERSION
@@ -43,10 +43,6 @@
)
-if TYPE_CHECKING:
- from pathlib import Path
-
-
def test_workflow_name():
"""Return a UUID to use as a test name"""
return f'cylc-rose-test-{str(uuid4())[:8]}'
@@ -223,24 +219,39 @@ def mod_cylc_validate_cli(mod_capsys, mod_caplog):
def file_poll():
"""Poll for the existance of a file.
"""
- def _inner(fpath: "Path", timeout: int = 5):
+ def _inner(
+ fpath: "Path", timeout: int = 5, inverse: bool = False
+ ):
+ if inverse:
+ def check(f):
+ not f.exists()
+ else:
+ def check(f):
+ f.exists()
+
timeout_func(
- fpath.exists,
+ partial(check, fpath),
f'file {fpath} not found after {timeout} seconds'
)
return _inner
@pytest.fixture
-def purge_workflow(run_ok):
+def purge_workflow(run_ok, file_poll):
"""Ensure workflow is stopped and cleaned"""
def _inner(id_, timeout=5):
- run_ok(f'cylc stop {id_} --now --now')
+ stop = f'cylc stop {id_} --now --now'
clean = f'cylc clean {id_}'
timeout_func(
- partial(run_ok, clean),
- message=f'Not run after {timeout} seconds: {clean}'
+ partial(run_ok, stop),
+ message=f'Not run after {timeout} seconds: {stop}')
+ file_poll(
+ Path.home() / 'cylc-run' / id_ / '.service/contact',
+ inverse=True,
)
+ timeout_func(
+ partial(run_ok, clean),
+ message=f'Not run after {timeout} seconds: {clean}')
return _inner
@@ -251,7 +262,7 @@ def run_ok():
"""
def _inner(script: str):
result = run(split(script), capture_output=True)
- assert result.returncode == 0
+ assert result.returncode == 0, f'{script} failed: {result.stderr}'
return result
return _inner
diff --git a/tests/functional/test_reinstall.py b/tests/functional/test_reinstall.py
index 9106e46b..d7507824 100644
--- a/tests/functional/test_reinstall.py
+++ b/tests/functional/test_reinstall.py
@@ -29,7 +29,6 @@
import pytest
import shutil
-from textwrap import dedent
from itertools import product
from pathlib import Path
@@ -281,59 +280,3 @@ def test_reinstall_workflow(tmp_path):
expect = sorted(['send rose-suite.conf', 'send flow.cylc'])
assert sorted(stdout.split('\n')) == expect
-
-
-def test_reinstall_overrides(
- cylc_install_cli,
- cylc_reinstall_cli,
- file_poll,
- tmp_path,
- purge_workflow,
- run_ok
-):
- """When reinstalling and reloading the new installation are picked up.
-
- > cylc install this -S 'var=CLIinstall'
- > cylc play this --pause
- > cylc reinstall this -S 'var=CLIreinstall'
- > cylc play this --pause
-
- See https://github.com/cylc/cylc-flow/issues/5968
- """
- (tmp_path / 'flow.cylc').write_text(dedent("""
- #!jinja2
- [scheduling]\n
- [[graph]]\n
- R1 = foo\n
- [runtime]\n
- [[foo]]\n
- script = cylc message -- {{var}}
- """))
- (tmp_path / 'rose-suite.conf').write_text(
- '[template variables]\nvar="rose-suite.conf"')
-
- # Install workflow.
- install_results = cylc_install_cli(
- tmp_path, {'rose_template_vars': ['var="CLIinstall"']})
- assert install_results.ret == 0
-
- # Play workflow
- run_ok(f'cylc play --pause {install_results.id}')
-
- # Reinstall the workflow:
- reinstall_results = cylc_reinstall_cli(
- install_results.id,
- {'rose_template_vars': ['var="CLIreinstall"']})
- assert reinstall_results.ret == 0
-
- # Reload the workflow:
- run_ok(f'cylc reload {install_results.id}')
-
- # The config being run has been modified:
- run_dir = Path.home() / 'cylc-run' / install_results.id
- config_log = (run_dir / 'log/config/02-reload-01.cylc')
- file_poll(config_log)
-
- assert 'cylc message -- CLIreinstall' in config_log.read_text()
-
- purge_workflow(install_results.id)
From c7c9680c8ec7b53138e18a2372352d59e40f894b Mon Sep 17 00:00:00 2001
From: Tim Pillinger <26465611+wxtim@users.noreply.github.com>
Date: Fri, 26 Apr 2024 13:18:26 +0100
Subject: [PATCH 13/17] Update tests/conftest.py
---
tests/conftest.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/conftest.py b/tests/conftest.py
index e908b88c..e313a49c 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -224,10 +224,10 @@ def _inner(
):
if inverse:
def check(f):
- not f.exists()
+ return not f.exists()
else:
def check(f):
- f.exists()
+ return f.exists()
timeout_func(
partial(check, fpath),
From be04c4ba3c800f6ce52e8f87d008833860ae1a54 Mon Sep 17 00:00:00 2001
From: Tim Pillinger <26465611+wxtim@users.noreply.github.com>
Date: Fri, 26 Apr 2024 16:01:35 +0100
Subject: [PATCH 14/17] Update cylc/rose/entry_points.py
---
cylc/rose/entry_points.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cylc/rose/entry_points.py b/cylc/rose/entry_points.py
index 029bdd4b..e27d2943 100644
--- a/cylc/rose/entry_points.py
+++ b/cylc/rose/entry_points.py
@@ -55,7 +55,7 @@ def __str__(self):
return msg
-def pre_configure(srcdir, opts, rundir=None):
+def pre_configure(srcdir, opts):
srcdir, rundir = paths_to_pathlib([srcdir, rundir])
return get_rose_vars(srcdir=srcdir, opts=opts)
From 101e766df606a26503039868a92e86188a597bd6 Mon Sep 17 00:00:00 2001
From: Tim Pillinger <26465611+wxtim@users.noreply.github.com>
Date: Tue, 30 Apr 2024 10:55:44 +0100
Subject: [PATCH 15/17] Fix testing
MetRonnies test fix
---
cylc/rose/entry_points.py | 2 +-
tests/conftest.py | 13 ++--
tests/functional/test_reinstall_overrides.py | 76 ++++++++++++++++++++
3 files changed, 81 insertions(+), 10 deletions(-)
create mode 100644 tests/functional/test_reinstall_overrides.py
diff --git a/cylc/rose/entry_points.py b/cylc/rose/entry_points.py
index e27d2943..4d38fab6 100644
--- a/cylc/rose/entry_points.py
+++ b/cylc/rose/entry_points.py
@@ -56,7 +56,7 @@ def __str__(self):
def pre_configure(srcdir, opts):
- srcdir, rundir = paths_to_pathlib([srcdir, rundir])
+ srcdir, = paths_to_pathlib([srcdir,])
return get_rose_vars(srcdir=srcdir, opts=opts)
diff --git a/tests/conftest.py b/tests/conftest.py
index e313a49c..4ff09eee 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -222,16 +222,11 @@ def file_poll():
def _inner(
fpath: "Path", timeout: int = 5, inverse: bool = False
):
- if inverse:
- def check(f):
- return not f.exists()
- else:
- def check(f):
- return f.exists()
-
timeout_func(
- partial(check, fpath),
- f'file {fpath} not found after {timeout} seconds'
+ lambda: fpath.exists() != inverse,
+ f"file {fpath} {'still' if inverse else 'not'} found after "
+ f"{timeout} seconds",
+ timeout
)
return _inner
diff --git a/tests/functional/test_reinstall_overrides.py b/tests/functional/test_reinstall_overrides.py
new file mode 100644
index 00000000..8c9a0367
--- /dev/null
+++ b/tests/functional/test_reinstall_overrides.py
@@ -0,0 +1,76 @@
+# THIS FILE IS PART OF THE ROSE-CYLC PLUGIN FOR THE CYLC WORKFLOW ENGINE.
+# Copyright (C) NIWA & British Crown (Met Office) & Contributors.
+#
+# This program is free software: you can redistribute it and/or modify
+# 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.
+#
+# 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
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see .
+"""Functional tests checking Cylc reinstall and reload behaviour is correct
+WRT to deletion of Rose Options at the end of the post install plugin.
+
+https://github.com/cylc/cylc-rose/pull/312
+"""
+
+
+from pathlib import Path
+from textwrap import dedent
+
+
+def test_reinstall_overrides(
+ cylc_install_cli,
+ cylc_reinstall_cli,
+ file_poll,
+ tmp_path,
+ purge_workflow,
+ run_ok
+):
+ """When reinstalling and reloading the new installation are picked up.
+ > cylc install this -S 'var=CLIinstall'
+ > cylc play this --pause
+ > cylc reinstall this -S 'var=CLIreinstall'
+ > cylc play this --pause
+ See https://github.com/cylc/cylc-flow/issues/5968
+ """
+ (tmp_path / 'flow.cylc').write_text(dedent(""" #!jinja2
+ [scheduling]
+ [[graph]]
+ R1 = foo
+ [runtime]
+ [[foo]]
+ script = cylc message -- {{var}}
+ """))
+ (tmp_path / 'rose-suite.conf').write_text(
+ '[template variables]\nvar="rose-suite.conf"')
+
+ # Install workflow.
+ install_results = cylc_install_cli(
+ tmp_path, {'rose_template_vars': ['var="CLIinstall"']})
+ assert install_results.ret == 0
+
+ # Play workflow
+ run_ok(f'cylc play --pause {install_results.id}')
+
+ # Reinstall the workflow:
+ reinstall_results = cylc_reinstall_cli(
+ install_results.id,
+ {'rose_template_vars': ['var="CLIreinstall"']})
+ assert reinstall_results.ret == 0
+
+ # Reload the workflow:
+ run_ok(f'cylc reload {install_results.id}')
+
+ # The config being run has been modified:
+ run_dir = Path.home() / 'cylc-run' / install_results.id
+ config_log = (run_dir / 'log/config/02-reload-01.cylc')
+ file_poll(config_log)
+ assert 'cylc message -- CLIreinstall' in config_log.read_text()
+
+ purge_workflow(install_results.id)
From cb8b102d5a010e6620c0eeadeb3283d30efe5ff5 Mon Sep 17 00:00:00 2001
From: Tim Pillinger <26465611+wxtim@users.noreply.github.com>
Date: Tue, 30 Apr 2024 11:44:54 +0100
Subject: [PATCH 16/17] Apply suggestions from code review
Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>
---
cylc/rose/entry_points.py | 2 +-
tests/conftest.py | 8 ++++++--
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/cylc/rose/entry_points.py b/cylc/rose/entry_points.py
index 4d38fab6..96ee4951 100644
--- a/cylc/rose/entry_points.py
+++ b/cylc/rose/entry_points.py
@@ -56,7 +56,7 @@ def __str__(self):
def pre_configure(srcdir, opts):
- srcdir, = paths_to_pathlib([srcdir,])
+ srcdir = Path(srcdir)
return get_rose_vars(srcdir=srcdir, opts=opts)
diff --git a/tests/conftest.py b/tests/conftest.py
index 4ff09eee..4ec7d33a 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -239,14 +239,18 @@ def _inner(id_, timeout=5):
clean = f'cylc clean {id_}'
timeout_func(
partial(run_ok, stop),
- message=f'Not run after {timeout} seconds: {stop}')
+ message=f'Not run after {timeout} seconds: {stop}',
+ timeout
+ )
file_poll(
Path.home() / 'cylc-run' / id_ / '.service/contact',
inverse=True,
)
timeout_func(
partial(run_ok, clean),
- message=f'Not run after {timeout} seconds: {clean}')
+ message=f'Not run after {timeout} seconds: {clean}',
+ timeout
+ )
return _inner
From 8a13bcf3d154e60938e22a3963311f9ec508068b Mon Sep 17 00:00:00 2001
From: Tim Pillinger <26465611+wxtim@users.noreply.github.com>
Date: Tue, 30 Apr 2024 11:49:50 +0100
Subject: [PATCH 17/17] correct mistake
---
tests/conftest.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/conftest.py b/tests/conftest.py
index 4ec7d33a..e3dc17db 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -240,7 +240,7 @@ def _inner(id_, timeout=5):
timeout_func(
partial(run_ok, stop),
message=f'Not run after {timeout} seconds: {stop}',
- timeout
+ timeout=timeout
)
file_poll(
Path.home() / 'cylc-run' / id_ / '.service/contact',
@@ -249,7 +249,7 @@ def _inner(id_, timeout=5):
timeout_func(
partial(run_ok, clean),
message=f'Not run after {timeout} seconds: {clean}',
- timeout
+ timeout=timeout
)
return _inner