Delete opts after post install

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,
@@ -54,12 +55,12 @@ def __str__(self):
         return msg
 
 
-def pre_configure(srcdir=None, opts=None, rundir=None):
-    srcdir, rundir = paths_to_pathlib([srcdir, rundir])
+def pre_configure(srcdir, opts):
+    srcdir = Path(srcdir)
     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])
@@ -75,6 +76,14 @@ 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
+    # 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 = []
+    os.unsetenv(ROSE_SUITE_OPT_CONF_KEYS)
+
     return results
 
 

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):

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.5, <8.3
     metomi-isodatetime
     ansimarkup
     jinja2

tests/conftest.py

@@ -15,9 +15,14 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 """Functional tests for top-level function record_cylc_install_options and
 """
-
+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 uuid import uuid4
 
 from cylc.flow import __version__ as CYLC_VERSION
 from cylc.flow.option_parsers import Options
@@ -38,6 +43,11 @@
 )
 
 
+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 +138,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 +213,64 @@ 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, inverse: bool = False
+    ):
+        timeout_func(
+            lambda: fpath.exists() != inverse,
+            f"file {fpath} {'still' if inverse else 'not'} found after "
+            f"{timeout} seconds",
+            timeout
+        )
+    return _inner
+
+
+@pytest.fixture
+def purge_workflow(run_ok, file_poll):
+    """Ensure workflow is stopped and cleaned"""
+    def _inner(id_, timeout=5):
+        stop = f'cylc stop {id_} --now --now'
+        clean = f'cylc clean {id_}'
+        timeout_func(
+            partial(run_ok, stop),
+            message=f'Not run after {timeout} seconds: {stop}',
+            timeout=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}',
+            timeout=timeout
+        )
+    return _inner
+
+
+@pytest.fixture
+def run_ok():
+    """Run a bash script.
+    Fail if it fails and return its output.
+    """
+    def _inner(script: str):
+        result = run(split(script), capture_output=True)
+        assert result.returncode == 0, f'{script} failed: {result.stderr}'
+        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)

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
 

tests/functional/test_reinstall.py

@@ -117,14 +117,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 +152,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 +171,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 +213,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 +229,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'

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'

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 <http://www.gnu.org/licenses/>.
+"""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)

tests/functional/test_rose_fileinstall.py

@@ -85,15 +85,15 @@ 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())
 
 
 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()
              ))