From ac0f5b15b6ec1c9c36b2a48774a91936907be8dc Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Thu, 27 Apr 2023 15:51:07 +0100 Subject: [PATCH 01/48] actions: test up to python 3.11 --- .github/workflows/1_create_release_pr.yml | 3 +-- .github/workflows/2_auto_publish_release.yml | 3 +-- .github/workflows/tests.yml | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/.github/workflows/1_create_release_pr.yml b/.github/workflows/1_create_release_pr.yml index 900fe9c3..3ef968d0 100644 --- a/.github/workflows/1_create_release_pr.yml +++ b/.github/workflows/1_create_release_pr.yml @@ -38,8 +38,7 @@ jobs: - name: Setup Python uses: actions/setup-python@v4 with: - # return to 3.x once cylc-flow is compatible with 3.10+ (pyuv) - python-version: '3.9' + python-version: '3.x' - name: Create & checkout PR branch uses: cylc/release-actions/stage-1/checkout-pr-branch@v1 diff --git a/.github/workflows/2_auto_publish_release.yml b/.github/workflows/2_auto_publish_release.yml index 5f2d6b21..dd202f11 100644 --- a/.github/workflows/2_auto_publish_release.yml +++ b/.github/workflows/2_auto_publish_release.yml @@ -28,8 +28,7 @@ jobs: - name: Setup Python uses: actions/setup-python@v4 with: - # return to 3.x once cylc-flow is compatible with 3.10+ (pyuv) - python-version: '3.9' + python-version: '3.x' - name: Get the version number uses: cylc/release-actions/stage-2/get-version-from-pr@v1 diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index a29e2e48..fe9a2f60 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -14,7 +14,7 @@ jobs: timeout-minutes: 5 strategy: matrix: - python-version: ['3.7', '3.8', '3.9'] + python-version: ['3.7', '3.8', '3.9', '3.10', '3.11'] env: PYTEST_ADDOPTS: --cov --cov-append --color=yes steps: From 9ca1b6ebc9c2686bd230976b93a4cef632836e2e Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Wed, 2 Aug 2023 09:54:09 +0100 Subject: [PATCH 02/48] bump 1.4.0 (#243) --- cylc/rose/__init__.py | 2 +- setup.cfg | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cylc/rose/__init__.py b/cylc/rose/__init__.py index 1229ce43..841796a0 100644 --- a/cylc/rose/__init__.py +++ b/cylc/rose/__init__.py @@ -205,4 +205,4 @@ """ -__version__ = '1.3.1.dev' +__version__ = '1.4.0.dev' diff --git a/setup.cfg b/setup.cfg index 15786025..3b31cc5a 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.* - cylc-flow==8.2.* + cylc-flow==8.3.* metomi-isodatetime ansimarkup jinja2 From 1a72a88d1806359a257f270589a91edd5b094bf5 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 14 Aug 2023 21:28:54 +0000 Subject: [PATCH 03/48] Bump pypa/gh-action-pypi-publish from 1.8.8 to 1.8.10 Bumps [pypa/gh-action-pypi-publish](https://github.com/pypa/gh-action-pypi-publish) from 1.8.8 to 1.8.10. - [Release notes](https://github.com/pypa/gh-action-pypi-publish/releases) - [Commits](https://github.com/pypa/gh-action-pypi-publish/compare/v1.8.8...v1.8.10) --- updated-dependencies: - dependency-name: pypa/gh-action-pypi-publish dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- .github/workflows/2_auto_publish_release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/2_auto_publish_release.yml b/.github/workflows/2_auto_publish_release.yml index 011f1484..d3b359dd 100644 --- a/.github/workflows/2_auto_publish_release.yml +++ b/.github/workflows/2_auto_publish_release.yml @@ -38,7 +38,7 @@ jobs: uses: cylc/release-actions/build-python-package@v1 - name: Publish distribution to PyPI - uses: pypa/gh-action-pypi-publish@v1.8.8 + uses: pypa/gh-action-pypi-publish@v1.8.10 with: user: __token__ # uses the API token feature of PyPI - least permissions possible password: ${{ secrets.PYPI_TOKEN }} From fc466bc4de281c6f1bf29616541544c0d125520c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 4 Sep 2023 21:35:47 +0000 Subject: [PATCH 04/48] Bump actions/checkout from 3 to 4 Bumps [actions/checkout](https://github.com/actions/checkout) from 3 to 4. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](https://github.com/actions/checkout/compare/v3...v4) --- updated-dependencies: - dependency-name: actions/checkout dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/1_create_release_pr.yml | 2 +- .github/workflows/2_auto_publish_release.yml | 2 +- .github/workflows/shortlog.yml | 2 +- .github/workflows/tests.yml | 4 ++-- .github/workflows/update_copyright.yml | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/1_create_release_pr.yml b/.github/workflows/1_create_release_pr.yml index 3ef968d0..a60b92ec 100644 --- a/.github/workflows/1_create_release_pr.yml +++ b/.github/workflows/1_create_release_pr.yml @@ -26,7 +26,7 @@ jobs: uses: cylc/release-actions/stage-1/sanitize-inputs@v1 - name: Checkout repo - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: ref: ${{ env.BASE_REF }} fetch-depth: 0 # need to fetch all commits to check contributors diff --git a/.github/workflows/2_auto_publish_release.yml b/.github/workflows/2_auto_publish_release.yml index b1d0d80c..afa27b24 100644 --- a/.github/workflows/2_auto_publish_release.yml +++ b/.github/workflows/2_auto_publish_release.yml @@ -21,7 +21,7 @@ jobs: steps: - name: Checkout repo - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: ref: ${{ env.MERGE_SHA }} diff --git a/.github/workflows/shortlog.yml b/.github/workflows/shortlog.yml index d8da001b..c119e620 100644 --- a/.github/workflows/shortlog.yml +++ b/.github/workflows/shortlog.yml @@ -13,7 +13,7 @@ jobs: timeout-minutes: 10 steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: fetch-depth: 0 # need to fetch all commits to check contributors diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index fe9a2f60..48a4a45b 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -19,7 +19,7 @@ jobs: PYTEST_ADDOPTS: --cov --cov-append --color=yes steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Configure Python uses: actions/setup-python@v4 @@ -47,7 +47,7 @@ jobs: run: mypy - name: Checkout FCM - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: repository: ${{ github.event.inputs.fcm_repo || 'metomi/fcm' }} ref: ${{ github.event.inputs.fcm_ref || 'master' }} diff --git a/.github/workflows/update_copyright.yml b/.github/workflows/update_copyright.yml index 5be6ad97..15989609 100644 --- a/.github/workflows/update_copyright.yml +++ b/.github/workflows/update_copyright.yml @@ -11,7 +11,7 @@ jobs: timeout-minutes: 10 steps: - name: Checkout repo - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Configure git uses: cylc/release-actions/configure-git@v1 From 545bdb1ff2bdfbdc9447acc7ff923a453233d64f Mon Sep 17 00:00:00 2001 From: Jonny Williams Date: Fri, 6 Oct 2023 04:39:30 +1300 Subject: [PATCH 05/48] Suggested change to warning text re integers starting with a zero (#255) * Suggested change to warning text when integers with leading zeros are found in the cylc workflow definition. * Apply suggestions from code review Co-authored-by: Tim Pillinger <26465611+wxtim@users.noreply.github.com> --------- Co-authored-by: Oliver Sanders Co-authored-by: Tim Pillinger <26465611+wxtim@users.noreply.github.com> --- CONTRIBUTING.md | 1 + cylc/rose/jinja2_parser.py | 7 ++++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c4f5b82f..603761fe 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -44,6 +44,7 @@ below. - Mel Hall - Bruno Kinoshita - Hilary Oliver + - Jonny Williams (All contributors are identifiable with email addresses in the git version diff --git a/cylc/rose/jinja2_parser.py b/cylc/rose/jinja2_parser.py index 491f9827..47387d72 100644 --- a/cylc/rose/jinja2_parser.py +++ b/cylc/rose/jinja2_parser.py @@ -161,10 +161,11 @@ def patch_jinja2_leading_zeros(): if jinja2.lexer.Lexer.wrap._instances: num_examples = 5 LOG.warning( - 'Support for integers with leading zeros was dropped' - ' in Jinja2 v3.' + 'Support for integers with leading zeros (including' + ' lists of integers) was dropped in Jinja2 v3.' ' Rose will extend support until a future version.' - '\nPlease amend your Rose configuration files e.g:' + '\nPlease amend your Rose configuration files,' + ' which currently contain:' '\n * ' + ( '\n * '.join( From 08d8e113777efbf5fc1f3b154ecc34848f337380 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Mon, 23 Oct 2023 10:19:51 +0100 Subject: [PATCH 06/48] improve test coverage (#227) Improve test coverage Co-authored-by: Oliver Sanders --- .coveragerc | 17 +++++ cylc/rose/entry_points.py | 7 +- cylc/rose/stem.py | 43 +++++------ cylc/rose/utilities.py | 89 +++++++++------------- tests/unit/test_functional_post_install.py | 39 ++++++++-- tests/unit/test_rose_opts.py | 3 +- tests/unit/test_rose_stem_units.py | 80 ++++++++++++++++++- 7 files changed, 188 insertions(+), 90 deletions(-) diff --git a/.coveragerc b/.coveragerc index b1a66bde..052afb62 100644 --- a/.coveragerc +++ b/.coveragerc @@ -4,3 +4,20 @@ source=./cylc [report] precision=2 + +exclude_lines = + pragma: no cover + + # Don't complain if tests don't hit defensive assertion code: + raise NotImplementedError + return NotImplemented + + # Ignore type checking code: + if (typing\.)?TYPE_CHECKING: + @overload( |$) + + # Don't complain about ellipsis (exception classes, typing overloads etc): + \.\.\. + + # Ignore abstract methods + @(abc\.)?abstractmethod diff --git a/cylc/rose/entry_points.py b/cylc/rose/entry_points.py index 7e5327f4..e0d4b4d0 100644 --- a/cylc/rose/entry_points.py +++ b/cylc/rose/entry_points.py @@ -177,9 +177,7 @@ def record_cylc_install_options( """ # Create a config based on command line options: cli_config = get_cli_opts_node(opts, srcdir) - # Leave now if there is nothing to do: - if not cli_config: - return False + # raise error if CLI config has multiple templating sections identify_templating_section(cli_config) @@ -218,8 +216,7 @@ def record_cylc_install_options( dumper.dump(cli_config, str(conf_filepath)) # Merge the opts section of the rose-suite.conf with those set by CLI: - if not rose_conf_filepath.is_file(): - rose_conf_filepath.touch() + rose_conf_filepath.touch() rose_suite_conf = loader.load(str(rose_conf_filepath)) rose_suite_conf = add_cylc_install_to_rose_conf_node_opts( rose_suite_conf, cli_config diff --git a/cylc/rose/stem.py b/cylc/rose/stem.py index 49b05efb..11565cd9 100644 --- a/cylc/rose/stem.py +++ b/cylc/rose/stem.py @@ -101,21 +101,13 @@ def __repr__(self): __str__ = __repr__ -class ConfigSourceTreeSetEvent(Event): - - """Event to report a source tree for config files.""" - - LEVEL = Event.V - - def __repr__(self): - return "Using config files from source %s" % (self.args[0]) - - __str__ = __repr__ - - class NameSetEvent(Event): - """Event to report a name for the suite being set.""" + """Event to report a name for the suite being set. + + Simple parser of output expected to be in the format: + Key: Value. + """ LEVEL = Event.V @@ -437,6 +429,21 @@ def _prepend_localhost(self, url): url = self.host_selector.get_local_host() + ':' + url return url + def _parse_auto_opts(self): + """Load the site config file and return any automatic-options. + + Parse options in the form of a space separated list of key=value + pairs. + """ + auto_opts = self._read_auto_opts() + if auto_opts: + automatic_options = auto_opts.split() + for option in automatic_options: + elements = option.split("=") + if len(elements) == 2: + self._add_define_option( + elements[0], '"' + elements[1] + '"') + def process(self): """Process STEM options into 'rose suite-run' options.""" # Generate options for source trees @@ -496,15 +503,7 @@ def process(self): self.opts.defines.append( f"{self.template_section}RUN_NAMES={str(expanded_groups)}") - # Load the config file and return any automatic-options - auto_opts = self._read_auto_opts() - if auto_opts: - automatic_options = auto_opts.split() - for option in automatic_options: - elements = option.split("=") - if len(elements) == 2: - self._add_define_option( - elements[0], '"' + elements[1] + '"') + self._parse_auto_opts() # Change into the suite directory if getattr(self.opts, 'workflow_conf_dir', None): diff --git a/cylc/rose/utilities.py b/cylc/rose/utilities.py index da367b47..0295c80c 100644 --- a/cylc/rose/utilities.py +++ b/cylc/rose/utilities.py @@ -65,8 +65,6 @@ def get_rose_vars_from_config_node(config, config_node, environ): Dictionary of environment variables """ - templating = None - # Don't allow multiple templating sections. templating = identify_templating_section(config_node) @@ -130,54 +128,43 @@ def get_rose_vars_from_config_node(config, config_node, environ): # For each of the template language sections extract items to a simple # dict to be returned. - if 'env' in config_node.value: - - config['env'] = { - item[0][1]: item[1].value for item in - config_node.value['env'].walk() - if item[1].state == ConfigNode.STATE_NORMAL - } - if templating in config_node.value: - config['template_variables'] = { - item[0][1]: item[1].value for item in - config_node.value[templating].walk() - if item[1].state == ConfigNode.STATE_NORMAL - } - elif 'template variables' in config_node.value: - config['template_variables'] = { - item[0][1]: item[1].value for item in - config_node.value['template variables'].walk() - if item[1].state == ConfigNode.STATE_NORMAL - } + config['env'] = { + item[0][1]: item[1].value for item in + config_node.value['env'].walk() + if item[1].state == ConfigNode.STATE_NORMAL + } + config['template_variables'] = { + item[0][1]: item[1].value for item in + config_node.value[templating].walk() + if item[1].state == ConfigNode.STATE_NORMAL + } # Add the entire config to ROSE_SUITE_VARIABLES to allow for programatic # access. - if templating is not None: - with patch_jinja2_leading_zeros(): - # BACK COMPAT: patch_jinja2_leading_zeros - # back support zero-padded integers for a limited time to help - # users migrate before upgrading cylc-flow to Jinja2>=3.1 - parser = Parser() - for key, value in config['template_variables'].items(): - # The special variables are already Python variables. - if key not in ['ROSE_ORIG_HOST', 'ROSE_VERSION', 'ROSE_SITE']: - try: - config['template_variables'][key] = ( - parser.literal_eval(value) - ) - except Exception: - raise ConfigProcessError( - [templating, key], - value, - f'Invalid template variable: {value}' - '\nMust be a valid Python or Jinja2 literal' - ' (note strings "must be quoted").' - ) from None + with patch_jinja2_leading_zeros(): + # BACK COMPAT: patch_jinja2_leading_zeros + # back support zero-padded integers for a limited time to help + # users migrate before upgrading cylc-flow to Jinja2>=3.1 + parser = Parser() + for key, value in config['template_variables'].items(): + # The special variables are already Python variables. + if key not in ['ROSE_ORIG_HOST', 'ROSE_VERSION', 'ROSE_SITE']: + try: + config['template_variables'][key] = ( + parser.literal_eval(value) + ) + except Exception: + raise ConfigProcessError( + [templating, key], + value, + f'Invalid template variable: {value}' + '\nMust be a valid Python or Jinja2 literal' + ' (note strings "must be quoted").' + ) from None # Add ROSE_SUITE_VARIABLES to config of templating engines in use. - if templating is not None: - config['template_variables'][ - 'ROSE_SUITE_VARIABLES'] = config['template_variables'] + config['template_variables'][ + 'ROSE_SUITE_VARIABLES'] = config['template_variables'] def identify_templating_section(config_node): @@ -246,7 +233,7 @@ def rose_config_tree_loader(srcdir=None, opts=None): if opts and 'opt_conf_keys' in dir(opts) and opts.opt_conf_keys: if isinstance(opts.opt_conf_keys, str): opt_conf_keys += opts.opt_conf_keys.split() - elif isinstance(opts.opt_conf_keys, list): + else: opt_conf_keys += opts.opt_conf_keys # Optional definitions @@ -430,6 +417,9 @@ def get_cli_opts_node(opts=None, srcdir=None): # Construct new config node representing CLI config items: newconfig = ConfigNode() + newconfig.set(['opts'], ConfigNode()) + + # For each __define__ determine whether it is an env or template define. for define in defines: parsed_define = parse_cli_defines(define) if parsed_define: @@ -454,9 +444,7 @@ def get_cli_opts_node(opts=None, srcdir=None): ) # Specialised treatement of optional configs. - if 'opts' not in newconfig: - newconfig['opts'] = ConfigNode() - newconfig['opts'].value = '' + newconfig['opts'].value = '' newconfig['opts'].value = merge_opts(newconfig, opt_conf_keys) newconfig['opts'].state = '!' @@ -539,8 +527,7 @@ def merge_opts(config, opt_conf_keys): 'aleph bet gimmel' """ all_opt_conf_keys = [] - if 'opts' in config: - all_opt_conf_keys.append(config['opts'].value) + 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 opt_conf_keys and isinstance(opt_conf_keys, str): diff --git a/tests/unit/test_functional_post_install.py b/tests/unit/test_functional_post_install.py index 9f8791be..eb352672 100644 --- a/tests/unit/test_functional_post_install.py +++ b/tests/unit/test_functional_post_install.py @@ -27,15 +27,18 @@ from metomi.isodatetime.datetimeoper import DateTimeOperator +import cylc from cylc.flow.hostuserutil import get_host from cylc.rose.entry_points import ( - record_cylc_install_options, rose_fileinstall, post_install + record_cylc_install_options, rose_fileinstall, post_install, + copy_config_file ) from cylc.rose.utilities import ( ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING, MultipleTemplatingEnginesError ) from metomi.rose.config import ConfigLoader +from metomi.rose.config_tree import ConfigTree HOST = get_host() @@ -346,13 +349,24 @@ def test_template_section_conflict( def test_rose_fileinstall_exception(tmp_path, monkeypatch): - def broken(): - raise FileNotFoundError('Any Old Error') - import os - monkeypatch.setattr(os, 'getcwd', broken) - (tmp_path / 'rose-suite.conf').touch() + """It throws an exception if you try to install files to a non existent + destination. + + (And returns to the directory you started at) + """ + def fakenode(_, __): + tree = ConfigTree() + tree.node.value = {'file': ''} + return tree + + monkeypatch.setattr( + cylc.rose.entry_points, 'rose_config_tree_loader', + fakenode + ) + monkeypatch.setattr( + cylc.rose.entry_points, "rose_config_exists", lambda x, y: True) with pytest.raises(FileNotFoundError): - rose_fileinstall(srcdir=tmp_path, rundir=tmp_path) + rose_fileinstall(srcdir=tmp_path, rundir='/oiruhgaqhnaigujhj') def test_cylc_no_rose(tmp_path): @@ -360,3 +374,14 @@ def test_cylc_no_rose(tmp_path): """ from cylc.rose.entry_points import post_install assert post_install(srcdir=tmp_path, rundir=tmp_path) is False + + +def test_copy_config_file_fails(): + """It fails when source or rundir not specified.""" + with pytest.raises(FileNotFoundError, match='both source and rundir'): + copy_config_file() + + +def test_copy_config_file_fails2(): + """It fails if source not a rose suite.""" + copy_config_file(srcdir='/foo', rundir='/bar') diff --git a/tests/unit/test_rose_opts.py b/tests/unit/test_rose_opts.py index fde1eb4b..14bb9884 100644 --- a/tests/unit/test_rose_opts.py +++ b/tests/unit/test_rose_opts.py @@ -86,8 +86,7 @@ def test_rose_fileinstall_validate(fixture_provide_flow, cylc_validate_cli): def test_rose_fileinstall_run(fixture_install_flow): """Workflow installs: """ - _, _, _, result, _ = fixture_install_flow - assert result.ret == 0 + pass # this tests the fixture itself def test_rose_fileinstall_rose_conf(fixture_install_flow): diff --git a/tests/unit/test_rose_stem_units.py b/tests/unit/test_rose_stem_units.py index d88bbdec..2c142f64 100644 --- a/tests/unit/test_rose_stem_units.py +++ b/tests/unit/test_rose_stem_units.py @@ -135,8 +135,9 @@ def test__add_define_option(get_StemRunner, capsys, exisiting_defines): 0, 'url: file:///worthwhile/foo/bar/baz/trunk@1\n' 'project: \n' - 'some waffle which ought to be ignored', - '' + 'key_with_no_value_ignored:\n' + 'some waffle which ought to be ignored\n', + None ), id='Good fcm output' ) @@ -210,7 +211,9 @@ def test__get_project_from_url( ('foo/bar', 'some_dir'), ) ) -def test__generate_name(get_StemRunner, monkeypatch, tmp_path, source, expect): +def test__generate_name( + get_StemRunner, monkeypatch, tmp_path, source, expect, caplog, capsys +): """It generates a name if StemRunner._ascertain_project fails. (This happens if the workflow source is not controlled with FCM) @@ -223,7 +226,9 @@ def test__generate_name(get_StemRunner, monkeypatch, tmp_path, source, expect): expect = tmp_path.name if expect == 'cwd' else expect stemrunner = get_StemRunner({}, {'workflow_conf_dir': source}) + stemrunner.reporter.contexts['stdout'].verbosity = 5 assert stemrunner._generate_name() == expect + assert 'Suite is named' in capsys.readouterr().out @pytest.mark.parametrize( @@ -263,6 +268,13 @@ def test__this_suite( stemrunner._this_suite() +def test__this_suite_raises_if_no_dir(get_StemRunner): + """It raises an exception if there is no suitefile""" + stemrunner = get_StemRunner({}, {'stem_sources': ['/foo']}) + with pytest.raises(RoseSuiteConfNotFoundException): + stemrunner._this_suite() + + def test__check_suite_version_fails_if_no_stem_source( get_StemRunner, tmp_path ): @@ -297,6 +309,68 @@ def test__deduce_mirror(): StemRunner._deduce_mirror(source_dict, project) +def test_RoseSuiteConfNotFoundException_repr(): + """It handles dirctory not existing _at all_""" + result = RoseSuiteConfNotFoundException('/foo').__repr__() + expect = 'Suite directory /foo is not a valid directory' + assert expect in result + + +def test__ascertain_project(get_StemRunner, monkeypatch): + """It doesn't handle sub_tree if not available.""" + monkeypatch.setattr( + cylc.rose.stem.StemRunner, + '_get_project_from_url', lambda _, __: 'foo' + ) + monkeypatch.setattr( + cylc.rose.stem.StemRunner, + '_deduce_mirror', lambda _, __, ___: 'foo' + ) + stemrunner = get_StemRunner({'popen': MockPopen(( + 0, + ( + 'root: https://foo.com/\n' + 'url: https://foo.com/helloworld\n' + 'project: helloworld\n' + ), + 'stderr' + ))}) + result = stemrunner._ascertain_project('') + assert result == ('foo', '', '', '', 'foo') + + +def test_process_multiple_auto_opts( + monkeypatch: Fixture, get_StemRunner: Fixture +) -> None: + """Read a list of options from site config. + + - Correctly splits list. + - Adds valid key=value pairs to stemrunner.options. + - Rejects malformed items. + """ + stemrunner = get_StemRunner({}, options={'defines': []}) + monkeypatch.setattr( + cylc.rose.stem.StemRunner, '_read_auto_opts', + lambda _: 'foo=bar baz=qux=quiz' + ) + stemrunner._parse_auto_opts() + assert 'foo="bar"' in stemrunner.opts.defines[0] + + +def test_process_no_auto_opts( + monkeypatch: Fixture, get_StemRunner: Fixture +) -> None: + """Read an empty list of options from site config. + """ + stemrunner = get_StemRunner({}, options={'defines': []}) + monkeypatch.setattr( + cylc.rose.stem.StemRunner, '_read_auto_opts', + lambda _: '' + ) + stemrunner._parse_auto_opts() + assert stemrunner.opts.defines == [] + + @pytest.mark.parametrize( 'item, expect, stdout', ( From e21e31c1199844b2415d78de4bbacb4c2fbd1c4c Mon Sep 17 00:00:00 2001 From: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> Date: Wed, 25 Oct 2023 10:28:49 +0100 Subject: [PATCH 07/48] GH Actions: use branch sync workflow --- .github/workflows/branch_sync.yml | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 .github/workflows/branch_sync.yml diff --git a/.github/workflows/branch_sync.yml b/.github/workflows/branch_sync.yml new file mode 100644 index 00000000..375e164b --- /dev/null +++ b/.github/workflows/branch_sync.yml @@ -0,0 +1,20 @@ +name: Sync PR + +on: + push: + branches: + - '*.*.x' + schedule: + - cron: '11 04 * * 1-5' # 04:11 UTC Mon-Fri + workflow_dispatch: + inputs: + head_branch: + description: Branch to merge into master + required: true + +jobs: + sync: + uses: cylc/release-actions/.github/workflows/branch-sync.yml@v1 + with: + head_branch: ${{ inputs.head_branch }} + secrets: inherit From 3d3e3f4727777b345cd95630fc9d05e2bf42e74e Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Tue, 31 Oct 2023 13:51:08 +0000 Subject: [PATCH 08/48] refactor: move utilities out of the entry_points module * Move utils out of the entry points module and move the rose-stem entry point in. * Move fileinstall into its own module. * Sort imports. --- cylc/rose/entry_points.py | 299 +----------------- cylc/rose/fileinstall.py | 80 +++++ cylc/rose/jinja2_parser.py | 11 +- cylc/rose/platform_utils.py | 4 +- cylc/rose/stem.py | 23 +- cylc/rose/utilities.py | 227 ++++++++++++- setup.cfg | 2 +- tests/conftest.py | 23 +- tests/functional/test_ROSE_ORIG_HOST.py | 6 +- tests/functional/test_copy_config_file.py | 4 +- tests/functional/test_pre_configure.py | 15 +- tests/functional/test_reinstall.py | 12 +- tests/functional/test_reinstall_clean.py | 10 +- .../functional/test_reinstall_fileinstall.py | 6 +- tests/functional/test_rose_fileinstall.py | 5 +- tests/functional/test_rose_stem.py | 26 +- tests/unit/test_config_node.py | 12 +- tests/unit/test_config_tree.py | 17 +- tests/unit/test_functional_post_install.py | 26 +- tests/unit/test_generic_utils.py | 4 +- tests/unit/test_platform_utils.py | 16 +- tests/unit/test_rose_opts.py | 5 +- tests/unit/test_rose_stem_units.py | 19 +- 23 files changed, 423 insertions(+), 429 deletions(-) create mode 100644 cylc/rose/fileinstall.py diff --git a/cylc/rose/entry_points.py b/cylc/rose/entry_points.py index 0517bde4..f169be90 100644 --- a/cylc/rose/entry_points.py +++ b/cylc/rose/entry_points.py @@ -13,53 +13,28 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Cylc support for reading and interpreting ``rose-suite.conf`` workflow -configuration files. +"""Top level module providing entry point functions.""" -Top level module providing entry point functions. -""" - -import os -import shutil - -from pathlib import Path - -from metomi.rose.config import ConfigLoader, ConfigDumper from cylc.rose.utilities import ( - ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING, - deprecation_warnings, + copy_config_file, dump_rose_log, - get_rose_vars_from_config_node, - identify_templating_section, - invalid_defines_check, - rose_config_exists, - rose_config_tree_loader, - merge_rose_cylc_suite_install_conf, + get_rose_vars, paths_to_pathlib, - get_cli_opts_node, - add_cylc_install_to_rose_conf_node_opts, + record_cylc_install_options, + rose_config_exists, ) -from cylc.flow.hostuserutil import get_host - - -class NotARoseSuiteException(Exception): - def __str__(self): - msg = ( - 'Cylc-Rose CLI arguments only used ' - 'if a rose-suite.conf file is present:' - '\n * "--opt-conf-key" or "-O"' - '\n * "--define" or "-D"' - '\n * "--rose-template-variable" or "-S"' - ) - return msg def pre_configure(srcdir=None, opts=None, rundir=None): + """Run before the Cylc configuration is read.""" srcdir, rundir = paths_to_pathlib([srcdir, rundir]) return get_rose_vars(srcdir=srcdir, opts=opts) def post_install(srcdir=None, opts=None, rundir=None): + """Run after Cylc file installation has completed.""" + from cylc.rose.fileinstall import rose_fileinstall + if not rose_config_exists(srcdir, opts): return False srcdir, rundir = paths_to_pathlib([srcdir, rundir]) @@ -78,255 +53,9 @@ def post_install(srcdir=None, opts=None, rundir=None): return results -def get_rose_vars(srcdir=None, opts=None): - """Load template variables from Rose suite configuration. - - Loads the Rose suite configuration tree from the filesystem - using the shell environment. - - Args: - srcdir(pathlib.Path): - Path to the Rose suite configuration - (the directory containing the ``rose-suite.conf`` file). - opts: - Options object containing specification of optional - configuarations set by the CLI. - - Returns: - dict - A dictionary of sections of rose-suite.conf. - For each section either a dictionary or None is returned. - E.g. - { - 'env': {'MYVAR': 42}, - 'empy:suite.rc': None, - 'jinja2:suite.rc': { - 'myJinja2Var': {'yes': 'it is a dictionary!'} - } - } - """ - # Set up blank page for returns. - config = { - 'env': {}, - 'template_variables': {}, - 'templating_detected': None - } - - # Return a blank config dict if srcdir does not exist - if not rose_config_exists(srcdir, opts): - if ( - getattr(opts, "opt_conf_keys", None) - or getattr(opts, "defines", None) - or getattr(opts, "rose_template_vars", None) - ): - raise NotARoseSuiteException() - return config - - # Check for definitely invalid defines - if opts and hasattr(opts, 'defines'): - invalid_defines_check(opts.defines) - - # Load the raw config tree - config_tree = rose_config_tree_loader(srcdir, opts) - deprecation_warnings(config_tree) - - # Extract templatevars from the configuration - get_rose_vars_from_config_node( - config, - config_tree.node, - os.environ - ) - - # Export environment vars - for key, val in config['env'].items(): - os.environ[key] = val - - return config - - -def record_cylc_install_options( - rundir=None, - opts=None, - srcdir=None, -): - """Create/modify files recording Cylc install config options. - - Creates a new config based on CLI options and writes it to the workflow - install location as ``rose-suite-cylc-install.conf``. - - If ``rose-suite-cylc-install.conf`` already exists over-writes changed - items, except for ``!opts=`` which is merged and simplified. - - If ``!opts=`` have been changed these are appended to those that have - been written in the installed ``rose-suite.conf``. - - Args: - srcdir (pathlib.Path): - Used to check whether the source directory contains a rose config. - opts: - Cylc option parser object - we want to extract the following - values: - - opt_conf_keys (list or str): - Equivalent of ``rose suite-run --option KEY`` - - defines (list of str): - Equivalent of ``rose suite-run --define KEY=VAL`` - - rose_template_vars (list of str): - Equivalent of ``rose suite-run --define-suite KEY=VAL`` - rundir (pathlib.Path): - Path to dump the rose-suite-cylc-conf - - Returns: - cli_config - Config Node which has been dumped to - ``rose-suite-cylc-install.conf``. - rose_suite_conf['opts'] - Opts section of the config node dumped to - installed ``rose-suite.conf``. - """ - # Create a config based on command line options: - cli_config = get_cli_opts_node(opts, srcdir) - - # raise error if CLI config has multiple templating sections - identify_templating_section(cli_config) - - # Construct path objects representing our target files. - (Path(rundir) / 'opt').mkdir(exist_ok=True) - conf_filepath = Path(rundir) / 'opt/rose-suite-cylc-install.conf' - rose_conf_filepath = Path(rundir) / 'rose-suite.conf' - dumper = ConfigDumper() - loader = ConfigLoader() - - # If file exists we need to merge with our new config, over-writing with - # new items where there are duplicates. - if conf_filepath.is_file(): - if opts.clear_rose_install_opts: - conf_filepath.unlink() - else: - oldconfig = loader.load(str(conf_filepath)) - # Check old config for clashing template variables sections. - identify_templating_section(oldconfig) - cli_config = merge_rose_cylc_suite_install_conf( - oldconfig, cli_config - ) - - # Get Values for standard ROSE variable ROSE_ORIG_HOST. - rose_orig_host = get_host() - for section in [ - 'env', 'jinja2:suite.rc', 'empy:suite.rc', 'template variables' - ]: - if section in cli_config: - cli_config[section].set(['ROSE_ORIG_HOST'], rose_orig_host) - cli_config[section]['ROSE_ORIG_HOST'].comments = [ - ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING - ] - - cli_config.comments = [' This file records CLI Options.'] - dumper.dump(cli_config, str(conf_filepath)) - - # Merge the opts section of the rose-suite.conf with those set by CLI: - rose_conf_filepath.touch() - rose_suite_conf = loader.load(str(rose_conf_filepath)) - rose_suite_conf = add_cylc_install_to_rose_conf_node_opts( - rose_suite_conf, cli_config - ) - identify_templating_section(rose_suite_conf) - - dumper(rose_suite_conf, rose_conf_filepath) - - return cli_config, rose_suite_conf - - -def rose_fileinstall(srcdir=None, opts=None, rundir=None): - """Call Rose Fileinstall. - - Args: - srcdir(pathlib.Path): - Search for a ``rose-suite.conf`` file in this location. - rundir (pathlib.Path) - - """ - if not rose_config_exists(rundir, opts): - return False - - # Load the config tree - config_tree = rose_config_tree_loader(rundir, opts) - - if any(i.startswith('file') for i in config_tree.node.value): - try: - startpoint = os.getcwd() - os.chdir(rundir) - except FileNotFoundError as exc: - raise exc - else: - # Carry out imports. - import asyncio - from metomi.rose.config_processor import ConfigProcessorsManager - from metomi.rose.popen import RosePopener - from metomi.rose.reporter import Reporter - from metomi.rose.fs_util import FileSystemUtil - - # Update config tree with install location - # NOTE-TO-SELF: value=os.environ["CYLC_WORKFLOW_RUN_DIR"] - config_tree.node = config_tree.node.set( - keys=["file-install-root"], value=str(rundir) - ) - - # Artificially set rose to verbose. - event_handler = Reporter(3) - fs_util = FileSystemUtil(event_handler) - popen = RosePopener(event_handler) - - # Get an Asyncio loop if one doesn't exist: - # Rose may need an event loop to invoke async interfaces, - # doing this here incase we want to go async in cylc-rose. - # See https://github.com/cylc/cylc-rose/pull/130/files - try: - asyncio.get_event_loop() - except RuntimeError: - asyncio.set_event_loop(asyncio.new_event_loop()) - - # Process fileinstall. - config_pm = ConfigProcessorsManager(event_handler, popen, fs_util) - config_pm(config_tree, "file") - finally: - os.chdir(startpoint) - - return config_tree.node - - -def copy_config_file( - srcdir=None, - opts=None, - rundir=None, -): - """Copy the ``rose-suite.conf`` from a workflow source to run directory. - - Args: - srcdir (pathlib.Path | or str): - Source Path of Cylc install. - opts: - Not used in this function, but requried for consistent entry point. - rundir (pathlib.Path | or str): - Destination path of Cylc install - the workflow rundir. - - Return: - True if ``rose-suite.conf`` has been installed. - False if insufficiant information to install file given. - """ - if ( - rundir is None or - srcdir is None - ): - raise FileNotFoundError( - "This plugin requires both source and rundir to exist." - ) - - rundir = Path(rundir) - srcdir = Path(srcdir) - srcdir_rose_conf = srcdir / 'rose-suite.conf' - rundir_rose_conf = rundir / 'rose-suite.conf' - - if not srcdir_rose_conf.is_file(): - return False - elif rundir_rose_conf.is_file(): - rundir_rose_conf.unlink() - shutil.copy2(srcdir_rose_conf, rundir_rose_conf) +def rose_stem(): + """Implements the "rose stem" command.""" + from cylc.rose.stem import get_rose_stem_opts - return True + parser, opts = get_rose_stem_opts() + rose_stem(parser, opts) diff --git a/cylc/rose/fileinstall.py b/cylc/rose/fileinstall.py new file mode 100644 index 00000000..4c9c83b0 --- /dev/null +++ b/cylc/rose/fileinstall.py @@ -0,0 +1,80 @@ +# 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 . + +"""Utilities related to performing Rose file installation.""" + +import os + +from cylc.rose.utilities import rose_config_exists, rose_config_tree_loader + + +def rose_fileinstall(srcdir=None, opts=None, rundir=None): + """Call Rose Fileinstall. + + Args: + srcdir(pathlib.Path): + Search for a ``rose-suite.conf`` file in this location. + rundir (pathlib.Path) + + """ + if not rose_config_exists(rundir, opts): + return False + + # Load the config tree + config_tree = rose_config_tree_loader(rundir, opts) + + if any(i.startswith('file') for i in config_tree.node.value): + try: + startpoint = os.getcwd() + os.chdir(rundir) + except FileNotFoundError as exc: + raise exc + else: + # Carry out imports. + import asyncio + + from metomi.rose.config_processor import ConfigProcessorsManager + from metomi.rose.fs_util import FileSystemUtil + from metomi.rose.popen import RosePopener + from metomi.rose.reporter import Reporter + + # Update config tree with install location + # NOTE-TO-SELF: value=os.environ["CYLC_WORKFLOW_RUN_DIR"] + config_tree.node = config_tree.node.set( + keys=["file-install-root"], value=str(rundir) + ) + + # Artificially set rose to verbose. + event_handler = Reporter(3) + fs_util = FileSystemUtil(event_handler) + popen = RosePopener(event_handler) + + # Get an Asyncio loop if one doesn't exist: + # Rose may need an event loop to invoke async interfaces, + # doing this here incase we want to go async in cylc-rose. + # See https://github.com/cylc/cylc-rose/pull/130/files + try: + asyncio.get_event_loop() + except RuntimeError: + asyncio.set_event_loop(asyncio.new_event_loop()) + + # Process fileinstall. + config_pm = ConfigProcessorsManager(event_handler, popen, fs_util) + config_pm(config_tree, "file") + finally: + os.chdir(startpoint) + + return config_tree.node diff --git a/cylc/rose/jinja2_parser.py b/cylc/rose/jinja2_parser.py index 47387d72..aa0528e7 100644 --- a/cylc/rose/jinja2_parser.py +++ b/cylc/rose/jinja2_parser.py @@ -16,22 +16,21 @@ """Utility for parsing Jinja2 expressions.""" from ast import literal_eval as python_literal_eval -from copy import deepcopy from contextlib import contextmanager +from copy import deepcopy import re +from cylc.flow import LOG +import jinja2.lexer from jinja2.nativetypes import NativeEnvironment # type: ignore from jinja2.nodes import ( # type: ignore Literal, + Neg, Output, Pair, + Pos, Template, - Neg, - Pos ) -import jinja2.lexer - -from cylc.flow import LOG def _strip_leading_zeros(string): diff --git a/cylc/rose/platform_utils.py b/cylc/rose/platform_utils.py index 88363954..5c3d6740 100644 --- a/cylc/rose/platform_utils.py +++ b/cylc/rose/platform_utils.py @@ -17,9 +17,9 @@ # ----------------------------------------------------------------------------- """Interfaces for Cylc Platforms for use by rose apps. """ -import subprocess from optparse import Values import sqlite3 +import subprocess from time import sleep from typing import Any, Dict @@ -29,7 +29,7 @@ from cylc.flow.platforms import ( HOST_REC_COMMAND, get_platform, - is_platform_definition_subshell + is_platform_definition_subshell, ) from cylc.flow.rundb import CylcWorkflowDAO diff --git a/cylc/rose/stem.py b/cylc/rose/stem.py index ba585775..47e816a1 100644 --- a/cylc/rose/stem.py +++ b/cylc/rose/stem.py @@ -60,7 +60,6 @@ is intended to specify the revision of `fcm-make` config files. """ -from ansimarkup import parse as cparse from contextlib import suppress from optparse import OptionGroup import os @@ -68,22 +67,19 @@ import re import sys +from ansimarkup import parse as cparse from cylc.flow.exceptions import CylcError -from cylc.flow.scripts.install import ( - get_option_parser, - install as cylc_install -) - -from cylc.rose.entry_points import get_rose_vars -from cylc.rose.utilities import id_templating_section - +from cylc.flow.scripts.install import get_option_parser +from cylc.flow.scripts.install import install as cylc_install import metomi.rose.config from metomi.rose.fs_util import FileSystemUtil from metomi.rose.host_select import HostSelector from metomi.rose.popen import RosePopener -from metomi.rose.reporter import Reporter, Event +from metomi.rose.reporter import Event, Reporter from metomi.rose.resource import ResourceLocator +from cylc.rose.entry_points import get_rose_vars +from cylc.rose.utilities import id_templating_section EXC_EXIT = cparse('{name}: {exc}') DEFAULT_TEST_DIR = 'rose-stem' @@ -552,7 +548,7 @@ def get_source_opt_from_args(opts, args): return opts -def _get_rose_stem_opts(): +def get_rose_stem_opts(): """Implement rose stem.""" # use the cylc install option parser parser = get_option_parser() @@ -603,11 +599,6 @@ def _get_rose_stem_opts(): return parser, opts -def main(): - parser, opts = _get_rose_stem_opts() - rose_stem(parser, opts) - - def rose_stem(parser, opts): try: # modify the CLI options to add whatever rose stem would like to add diff --git a/cylc/rose/utilities.py b/cylc/rose/utilities.py index c570d4ed..853d3fef 100644 --- a/cylc/rose/utilities.py +++ b/cylc/rose/utilities.py @@ -13,26 +13,35 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . + """Cylc support for reading and interpreting ``rose-suite.conf`` workflow configuration files. """ + import itertools import os from pathlib import Path import re import shlex +import shutil from typing import TYPE_CHECKING, Any, List, Optional, Tuple, Union -from cylc.flow.hostuserutil import get_host from cylc.flow import LOG from cylc.flow.exceptions import CylcError from cylc.flow.flags import cylc7_back_compat -from cylc.rose.jinja2_parser import Parser, patch_jinja2_leading_zeros -from metomi.rose import __version__ as ROSE_VERSION +from cylc.flow.hostuserutil import get_host from metomi.isodatetime.datetimeoper import DateTimeOperator -from metomi.rose.config import ConfigNodeDiff, ConfigNode, ConfigDumper +from metomi.rose import __version__ as ROSE_VERSION +from metomi.rose.config import ( + ConfigDumper, + ConfigLoader, + ConfigNode, + ConfigNodeDiff, +) from metomi.rose.config_processor import ConfigProcessError -from metomi.rose.env import env_var_process, UnboundEnvironmentVariableError +from metomi.rose.env import UnboundEnvironmentVariableError, env_var_process + +from cylc.rose.jinja2_parser import Parser, patch_jinja2_leading_zeros if TYPE_CHECKING: from optparse import Values @@ -47,6 +56,18 @@ ALL_MODES = 'all modes' +class NotARoseSuiteException(Exception): + def __str__(self): + msg = ( + 'Cylc-Rose CLI arguments only used ' + 'if a rose-suite.conf file is present:' + '\n * "--opt-conf-key" or "-O"' + '\n * "--define" or "-D"' + '\n * "--rose-template-variable" or "-S"' + ) + return msg + + class MultipleTemplatingEnginesError(CylcError): ... @@ -787,3 +808,199 @@ def deprecation_warnings(config_tree): and name in string.lower() ): LOG.warning(info[MESSAGE]) + + +def get_rose_vars(srcdir=None, opts=None): + """Load template variables from Rose suite configuration. + + Loads the Rose suite configuration tree from the filesystem + using the shell environment. + + Args: + srcdir(pathlib.Path): + Path to the Rose suite configuration + (the directory containing the ``rose-suite.conf`` file). + opts: + Options object containing specification of optional + configuarations set by the CLI. + + Returns: + dict - A dictionary of sections of rose-suite.conf. + For each section either a dictionary or None is returned. + E.g. + { + 'env': {'MYVAR': 42}, + 'empy:suite.rc': None, + 'jinja2:suite.rc': { + 'myJinja2Var': {'yes': 'it is a dictionary!'} + } + } + """ + # Set up blank page for returns. + config = { + 'env': {}, + 'template_variables': {}, + 'templating_detected': None + } + + # Return a blank config dict if srcdir does not exist + if not rose_config_exists(srcdir, opts): + if ( + getattr(opts, "opt_conf_keys", None) + or getattr(opts, "defines", None) + or getattr(opts, "rose_template_vars", None) + ): + raise NotARoseSuiteException() + return config + + # Check for definitely invalid defines + if opts and hasattr(opts, 'defines'): + invalid_defines_check(opts.defines) + + # Load the raw config tree + config_tree = rose_config_tree_loader(srcdir, opts) + deprecation_warnings(config_tree) + + # Extract templatevars from the configuration + get_rose_vars_from_config_node( + config, + config_tree.node, + os.environ + ) + + # Export environment vars + for key, val in config['env'].items(): + os.environ[key] = val + + return config + + +def record_cylc_install_options( + rundir=None, + opts=None, + srcdir=None, +): + """Create/modify files recording Cylc install config options. + + Creates a new config based on CLI options and writes it to the workflow + install location as ``rose-suite-cylc-install.conf``. + + If ``rose-suite-cylc-install.conf`` already exists over-writes changed + items, except for ``!opts=`` which is merged and simplified. + + If ``!opts=`` have been changed these are appended to those that have + been written in the installed ``rose-suite.conf``. + + Args: + srcdir (pathlib.Path): + Used to check whether the source directory contains a rose config. + opts: + Cylc option parser object - we want to extract the following + values: + - opt_conf_keys (list or str): + Equivalent of ``rose suite-run --option KEY`` + - defines (list of str): + Equivalent of ``rose suite-run --define KEY=VAL`` + - rose_template_vars (list of str): + Equivalent of ``rose suite-run --define-suite KEY=VAL`` + rundir (pathlib.Path): + Path to dump the rose-suite-cylc-conf + + Returns: + cli_config - Config Node which has been dumped to + ``rose-suite-cylc-install.conf``. + rose_suite_conf['opts'] - Opts section of the config node dumped to + installed ``rose-suite.conf``. + """ + # Create a config based on command line options: + cli_config = get_cli_opts_node(opts, srcdir) + + # raise error if CLI config has multiple templating sections + identify_templating_section(cli_config) + + # Construct path objects representing our target files. + (Path(rundir) / 'opt').mkdir(exist_ok=True) + conf_filepath = Path(rundir) / 'opt/rose-suite-cylc-install.conf' + rose_conf_filepath = Path(rundir) / 'rose-suite.conf' + dumper = ConfigDumper() + loader = ConfigLoader() + + # If file exists we need to merge with our new config, over-writing with + # new items where there are duplicates. + if conf_filepath.is_file(): + if opts.clear_rose_install_opts: + conf_filepath.unlink() + else: + oldconfig = loader.load(str(conf_filepath)) + # Check old config for clashing template variables sections. + identify_templating_section(oldconfig) + cli_config = merge_rose_cylc_suite_install_conf( + oldconfig, cli_config + ) + + # Get Values for standard ROSE variable ROSE_ORIG_HOST. + rose_orig_host = get_host() + for section in [ + 'env', 'jinja2:suite.rc', 'empy:suite.rc', 'template variables' + ]: + if section in cli_config: + cli_config[section].set(['ROSE_ORIG_HOST'], rose_orig_host) + cli_config[section]['ROSE_ORIG_HOST'].comments = [ + ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING + ] + + cli_config.comments = [' This file records CLI Options.'] + dumper.dump(cli_config, str(conf_filepath)) + + # Merge the opts section of the rose-suite.conf with those set by CLI: + rose_conf_filepath.touch() + rose_suite_conf = loader.load(str(rose_conf_filepath)) + rose_suite_conf = add_cylc_install_to_rose_conf_node_opts( + rose_suite_conf, cli_config + ) + identify_templating_section(rose_suite_conf) + + dumper(rose_suite_conf, rose_conf_filepath) + + return cli_config, rose_suite_conf + + +def copy_config_file( + srcdir=None, + opts=None, + rundir=None, +): + """Copy the ``rose-suite.conf`` from a workflow source to run directory. + + Args: + srcdir (pathlib.Path | or str): + Source Path of Cylc install. + opts: + Not used in this function, but requried for consistent entry point. + rundir (pathlib.Path | or str): + Destination path of Cylc install - the workflow rundir. + + Return: + True if ``rose-suite.conf`` has been installed. + False if insufficiant information to install file given. + """ + if ( + rundir is None or + srcdir is None + ): + raise FileNotFoundError( + "This plugin requires both source and rundir to exist." + ) + + rundir = Path(rundir) + srcdir = Path(srcdir) + srcdir_rose_conf = srcdir / 'rose-suite.conf' + rundir_rose_conf = rundir / 'rose-suite.conf' + + if not srcdir_rose_conf.is_file(): + return False + elif rundir_rose_conf.is_file(): + rundir_rose_conf.unlink() + shutil.copy2(srcdir_rose_conf, rundir_rose_conf) + + return True diff --git a/setup.cfg b/setup.cfg index 3fa012bd..136eeaea 100644 --- a/setup.cfg +++ b/setup.cfg @@ -89,4 +89,4 @@ cylc.pre_configure = cylc.post_install = rose_opts = cylc.rose.entry_points:post_install rose.commands = - stem = cylc.rose.stem:main + stem = cylc.rose.entry_points:rose_stem diff --git a/tests/conftest.py b/tests/conftest.py index 4031397e..93647bf2 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -16,26 +16,17 @@ """Functional tests for top-level function record_cylc_install_options and """ -import pytest from types import SimpleNamespace from cylc.flow import __version__ as CYLC_VERSION from cylc.flow.option_parsers import Options - -from cylc.flow.scripts.validate import ( - _main as cylc_validate, - get_option_parser as validate_gop -) - -from cylc.flow.scripts.install import ( - install_cli as cylc_install, - get_option_parser as install_gop -) - -from cylc.flow.scripts.reinstall import ( - reinstall_cli as cylc_reinstall, - get_option_parser as reinstall_gop -) +from cylc.flow.scripts.install import get_option_parser as install_gop +from cylc.flow.scripts.install import install_cli as cylc_install +from cylc.flow.scripts.reinstall import get_option_parser as reinstall_gop +from cylc.flow.scripts.reinstall import reinstall_cli as cylc_reinstall +from cylc.flow.scripts.validate import _main as cylc_validate +from cylc.flow.scripts.validate import get_option_parser as validate_gop +import pytest @pytest.fixture(scope='module') diff --git a/tests/functional/test_ROSE_ORIG_HOST.py b/tests/functional/test_ROSE_ORIG_HOST.py index b5037da8..df1db67e 100644 --- a/tests/functional/test_ROSE_ORIG_HOST.py +++ b/tests/functional/test_ROSE_ORIG_HOST.py @@ -59,16 +59,14 @@ """ -import pytest +from pathlib import Path import re import shutil - -from pathlib import Path from uuid import uuid4 from cylc.flow.hostuserutil import get_host from cylc.flow.pathutil import get_workflow_run_dir - +import pytest HOST = get_host() diff --git a/tests/functional/test_copy_config_file.py b/tests/functional/test_copy_config_file.py index ae02f35f..76ec3cd6 100644 --- a/tests/functional/test_copy_config_file.py +++ b/tests/functional/test_copy_config_file.py @@ -17,10 +17,10 @@ copy_config_file. """ -import pytest - from pathlib import Path +import pytest + from cylc.rose.entry_points import copy_config_file diff --git a/tests/functional/test_pre_configure.py b/tests/functional/test_pre_configure.py index ddefff41..79f3c9a6 100644 --- a/tests/functional/test_pre_configure.py +++ b/tests/functional/test_pre_configure.py @@ -16,19 +16,18 @@ """Functional tests for top-level function record_cylc_install_options and """ -import os -import pytest -import re - from itertools import product +import os from pathlib import Path -from pytest import param +import re from shlex import split from subprocess import run from types import SimpleNamespace -import cylc -from cylc.rose.entry_points import get_rose_vars, NotARoseSuiteException +import pytest +from pytest import param + +from cylc.rose.utilities import NotARoseSuiteException, get_rose_vars def envar_exporter(dict_): @@ -157,7 +156,7 @@ def test_warn_if_old_templating_set( ): """Test using unsupported root-dir config raises error.""" monkeypatch.setattr( - cylc.rose.utilities, 'cylc7_back_compat', compat_mode + 'cylc.rose.utilities.cylc7_back_compat', compat_mode ) (tmp_path / 'rose-suite.conf').write_text(f'[{rose_config}]') get_rose_vars(srcdir=tmp_path) diff --git a/tests/functional/test_reinstall.py b/tests/functional/test_reinstall.py index db4c5e46..4190a092 100644 --- a/tests/functional/test_reinstall.py +++ b/tests/functional/test_reinstall.py @@ -27,19 +27,19 @@ - ~/cylc-run/temporary-id/opt/rose-suite-cylc-install.conf """ -import pytest -import shutil - from itertools import product from pathlib import Path +import shutil from uuid import uuid4 from cylc.flow.hostuserutil import get_host -from cylc.flow.pathutil import get_workflow_run_dir -from cylc.rose.utilities import ( - ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING as ROHIOS) from cylc.flow.install import reinstall_workflow +from cylc.flow.pathutil import get_workflow_run_dir +import pytest +from cylc.rose.utilities import ( + ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING as ROHIOS, +) HOST = get_host() diff --git a/tests/functional/test_reinstall_clean.py b/tests/functional/test_reinstall_clean.py index 2883afb5..d6a332f0 100644 --- a/tests/functional/test_reinstall_clean.py +++ b/tests/functional/test_reinstall_clean.py @@ -27,17 +27,17 @@ - ~/cylc-run/temporary-id/opt/rose-suite-cylc-install.conf """ -import pytest -import shutil - from pathlib import Path +import shutil from uuid import uuid4 from cylc.flow.hostuserutil import get_host from cylc.flow.pathutil import get_workflow_run_dir -from cylc.rose.utilities import ( - ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING as ROHIOS) +import pytest +from cylc.rose.utilities import ( + ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING as ROHIOS, +) HOST = get_host() diff --git a/tests/functional/test_reinstall_fileinstall.py b/tests/functional/test_reinstall_fileinstall.py index 53f0968f..70ca41e8 100644 --- a/tests/functional/test_reinstall_fileinstall.py +++ b/tests/functional/test_reinstall_fileinstall.py @@ -17,14 +17,12 @@ trouble. """ -import pytest -import shutil - from pathlib import Path +import shutil from uuid import uuid4 from cylc.flow.pathutil import get_workflow_run_dir - +import pytest WORKFLOW_SRC = Path(__file__).parent / '14_reinstall_fileinstall' diff --git a/tests/functional/test_rose_fileinstall.py b/tests/functional/test_rose_fileinstall.py index e11a496f..5998b778 100644 --- a/tests/functional/test_rose_fileinstall.py +++ b/tests/functional/test_rose_fileinstall.py @@ -16,13 +16,12 @@ """Functional tests for top-level function record_cylc_install_options and """ -import pytest -import shutil - from pathlib import Path +import shutil from uuid import uuid4 from cylc.flow.pathutil import get_workflow_run_dir +import pytest @pytest.fixture diff --git a/tests/functional/test_rose_stem.py b/tests/functional/test_rose_stem.py index 01caeb72..743455e2 100644 --- a/tests/functional/test_rose_stem.py +++ b/tests/functional/test_rose_stem.py @@ -63,24 +63,24 @@ """ import os -import pytest +from pathlib import Path import re +from shlex import split import shutil import subprocess - -from pathlib import Path -from shlex import split from types import SimpleNamespace from uuid import uuid4 -from cylc.flow.pathutil import get_workflow_run_dir from cylc.flow.hostuserutil import get_host - -from cylc.rose.stem import ( - RoseStemVersionException, rose_stem, _get_rose_stem_opts) - +from cylc.flow.pathutil import get_workflow_run_dir from metomi.rose.resource import ResourceLocator +import pytest +from cylc.rose.stem import ( + RoseStemVersionException, + get_rose_stem_opts, + rose_stem, +) HOST = get_host().split('.')[0] @@ -225,7 +225,7 @@ def rose_stem_run_template(setup_stem_repo, pytestconfig, monkeymodule): def _inner_fn(rose_stem_opts, verbosity=verbosity): monkeymodule.setattr('sys.argv', ['stem']) monkeymodule.chdir(setup_stem_repo['workingcopy']) - parser, opts = _get_rose_stem_opts() + parser, opts = get_rose_stem_opts() [setattr(opts, key, val) for key, val in rose_stem_opts.items()] run_stem = SimpleNamespace() @@ -593,7 +593,7 @@ def test_incompatible_versions(setup_stem_repo, monkeymodule, caplog, capsys): monkeymodule.setattr('sys.argv', ['stem']) monkeymodule.chdir(setup_stem_repo['workingcopy']) - parser, opts = _get_rose_stem_opts() + parser, opts = get_rose_stem_opts() [setattr(opts, key, val) for key, val in rose_stem_opts.items()] with pytest.raises( @@ -618,7 +618,7 @@ def test_project_not_in_keywords(setup_stem_repo, monkeymodule, capsys): monkeymodule.setattr('sys.argv', ['stem']) monkeymodule.chdir(setup_stem_repo['workingcopy']) - parser, opts = _get_rose_stem_opts() + parser, opts = get_rose_stem_opts() [setattr(opts, key, val) for key, val in rose_stem_opts.items()] rose_stem(parser, opts) @@ -636,7 +636,7 @@ def test_picks_template_section(setup_stem_repo, monkeymodule, capsys): 'ROSE_STEM_VERSION=1\n' '[template_variables]\n' ) - parser, opts = _get_rose_stem_opts() + parser, opts = get_rose_stem_opts() rose_stem(parser, opts) _, err = capsys.readouterr() assert "[jinja2:suite.rc]' is deprecated" not in err diff --git a/tests/unit/test_config_node.py b/tests/unit/test_config_node.py index bcd8d422..848815e3 100644 --- a/tests/unit/test_config_node.py +++ b/tests/unit/test_config_node.py @@ -15,25 +15,23 @@ # along with this program. If not, see . """Tests the plugin with Rose suite configurations via the Python API.""" -import pytest from types import SimpleNamespace from metomi.isodatetime.datetimeoper import DateTimeOperator from metomi.rose import __version__ as ROSE_VERSION -from metomi.rose.config import ( - ConfigNode, -) +from metomi.rose.config import ConfigNode from metomi.rose.config_processor import ConfigProcessError +import pytest from cylc.rose.utilities import ( ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING, - get_rose_vars_from_config_node, + MultipleTemplatingEnginesError, add_cylc_install_to_rose_conf_node_opts, deprecation_warnings, dump_rose_log, - identify_templating_section, + get_rose_vars_from_config_node, id_templating_section, - MultipleTemplatingEnginesError + identify_templating_section, ) diff --git a/tests/unit/test_config_tree.py b/tests/unit/test_config_tree.py index 3718785b..768ee584 100644 --- a/tests/unit/test_config_tree.py +++ b/tests/unit/test_config_tree.py @@ -20,27 +20,24 @@ """ +from io import StringIO import os -import pytest -from pytest import param - from types import SimpleNamespace -from io import StringIO from cylc.flow.hostuserutil import get_host +from metomi.rose.config import ConfigLoader +import pytest +from pytest import param + +from cylc.rose.entry_points import get_rose_vars from cylc.rose.utilities import ( + MultipleTemplatingEnginesError, get_cli_opts_node, merge_opts, merge_rose_cylc_suite_install_conf, rose_config_exists, rose_config_tree_loader, - MultipleTemplatingEnginesError ) -from cylc.rose.entry_points import ( - get_rose_vars, -) -from metomi.rose.config import ConfigLoader - HOST = get_host() diff --git a/tests/unit/test_functional_post_install.py b/tests/unit/test_functional_post_install.py index eb352672..967eeb35 100644 --- a/tests/unit/test_functional_post_install.py +++ b/tests/unit/test_functional_post_install.py @@ -20,26 +20,26 @@ ``cylc install -D [fileinstall:myfile]example`` will lead to the correct file installation. """ -import pytest from pathlib import Path from types import SimpleNamespace +from cylc.flow.hostuserutil import get_host from metomi.isodatetime.datetimeoper import DateTimeOperator +from metomi.rose.config import ConfigLoader +from metomi.rose.config_tree import ConfigTree +import pytest -import cylc -from cylc.flow.hostuserutil import get_host from cylc.rose.entry_points import ( - record_cylc_install_options, rose_fileinstall, post_install, - copy_config_file + copy_config_file, + post_install, + record_cylc_install_options, ) +from cylc.rose.fileinstall import rose_fileinstall from cylc.rose.utilities import ( ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING, - MultipleTemplatingEnginesError + MultipleTemplatingEnginesError, ) -from metomi.rose.config import ConfigLoader -from metomi.rose.config_tree import ConfigTree - HOST = get_host() @@ -360,11 +360,13 @@ def fakenode(_, __): return tree monkeypatch.setattr( - cylc.rose.entry_points, 'rose_config_tree_loader', - fakenode + 'cylc.rose.utilities.rose_config_tree_loader', + fakenode, ) monkeypatch.setattr( - cylc.rose.entry_points, "rose_config_exists", lambda x, y: True) + 'cylc.rose.fileinstall.rose_config_exists', + lambda x, y: True, + ) with pytest.raises(FileNotFoundError): rose_fileinstall(srcdir=tmp_path, rundir='/oiruhgaqhnaigujhj') diff --git a/tests/unit/test_generic_utils.py b/tests/unit/test_generic_utils.py index 82607da3..3005769c 100644 --- a/tests/unit/test_generic_utils.py +++ b/tests/unit/test_generic_utils.py @@ -16,10 +16,10 @@ """Test generic ultilities """ -import pytest - from pathlib import Path +import pytest + from cylc.rose.utilities import paths_to_pathlib diff --git a/tests/unit/test_platform_utils.py b/tests/unit/test_platform_utils.py index 8ad68d78..04433d03 100644 --- a/tests/unit/test_platform_utils.py +++ b/tests/unit/test_platform_utils.py @@ -19,24 +19,22 @@ """ import os -import pytest from pathlib import Path from shutil import rmtree import sqlite3 from uuid import uuid4 -from cylc.rose.platform_utils import ( - get_platform_from_task_def, - get_platforms_from_task_jobs -) - from cylc.flow import __version__ as cylc_version from cylc.flow.cfgspec.globalcfg import SPEC from cylc.flow.parsec.config import ParsecConfig -from cylc.flow.pathutil import ( - get_workflow_run_pub_db_path -) +from cylc.flow.pathutil import get_workflow_run_pub_db_path from cylc.flow.workflow_db_mgr import CylcWorkflowDAO +import pytest + +from cylc.rose.platform_utils import ( + get_platform_from_task_def, + get_platforms_from_task_jobs, +) MOCK_GLBL_CFG = ( 'cylc.flow.platforms.glbl_cfg', diff --git a/tests/unit/test_rose_opts.py b/tests/unit/test_rose_opts.py index 14bb9884..a05658dd 100644 --- a/tests/unit/test_rose_opts.py +++ b/tests/unit/test_rose_opts.py @@ -16,14 +16,13 @@ """Functional tests for top-level function record_cylc_install_options and """ -import pytest -import shutil - from pathlib import Path +import shutil from uuid import uuid4 from cylc.flow.hostuserutil import get_host from cylc.flow.pathutil import get_workflow_run_dir +import pytest from cylc.rose.utilities import ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING diff --git a/tests/unit/test_rose_stem_units.py b/tests/unit/test_rose_stem_units.py index 8a73015b..f58dd47e 100644 --- a/tests/unit/test_rose_stem_units.py +++ b/tests/unit/test_rose_stem_units.py @@ -16,26 +16,25 @@ """Functional tests for top-level function record_cylc_install_options and """ -import cylc.rose -import pytest -from pytest import param from types import SimpleNamespace from typing import Any, Tuple +from metomi.rose.fs_util import FileSystemUtil +from metomi.rose.popen import RosePopener +from metomi.rose.reporter import Reporter +import pytest +from pytest import param + +import cylc.rose from cylc.rose.stem import ( - _get_rose_stem_opts, ProjectNotFoundException, RoseStemVersionException, RoseSuiteConfNotFoundException, StemRunner, + get_rose_stem_opts, get_source_opt_from_args, ) -from metomi.rose.reporter import Reporter -from metomi.rose.popen import RosePopener -from metomi.rose.fs_util import FileSystemUtil - - Fixture = Any @@ -440,7 +439,7 @@ def test_process_template_engine_set_correctly(monkeypatch, language, expect): # We are not interested in these checks, just in the defines # created by the process method. - stemrunner = StemRunner(_get_rose_stem_opts()[1]) + stemrunner = StemRunner(get_rose_stem_opts()[1]) stemrunner._ascertain_project = lambda _: ['', '', '', '', ''] stemrunner._this_suite = lambda: '.' stemrunner._check_suite_version = lambda _: '1' From e4f1cbbcab9b8cb67d67480e1f9ca90364b056d0 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Tue, 31 Oct 2023 15:02:18 +0000 Subject: [PATCH 09/48] tests: fix os.environ test interaction --- tests/functional/test_pre_configure.py | 27 +++++++++----------------- tests/unit/test_config_tree.py | 22 +++++++++------------ 2 files changed, 18 insertions(+), 31 deletions(-) diff --git a/tests/functional/test_pre_configure.py b/tests/functional/test_pre_configure.py index 79f3c9a6..baecc6df 100644 --- a/tests/functional/test_pre_configure.py +++ b/tests/functional/test_pre_configure.py @@ -30,11 +30,6 @@ from cylc.rose.utilities import NotARoseSuiteException, get_rose_vars -def envar_exporter(dict_): - for key, val in dict_.items(): - os.environ[key] = val - - @pytest.mark.parametrize( 'srcdir, expect', [ @@ -82,40 +77,36 @@ def test_validate_fail(srcdir, expect, cylc_validate_cli): ('09_template_vars_vanilla', {'XYZ': 'xyz'}, None), ], ) -def test_validate(tmp_path, srcdir, envvars, args, cylc_validate_cli): - if envvars is not None: - envvars = os.environ.update(envvars) +def test_validate(monkeypatch, srcdir, envvars, args, cylc_validate_cli): + for key, value in (envvars or {}).items(): + monkeypatch.setenv(key, value) srcdir = Path(__file__).parent / srcdir validate = cylc_validate_cli(str(srcdir), args) assert validate.ret == 0 @pytest.mark.parametrize( - 'srcdir, envvars, args', + 'srcdir, envvars', [ - ('00_jinja2_basic', None, None), - ('01_empy', None, None), + ('00_jinja2_basic', None), + ('01_empy', None), ( '04_opts_set_from_env', {'ROSE_SUITE_OPT_CONF_KEYS': 'Gaelige'}, - None ), ( '05_opts_set_from_rose_suite_conf', {'ROSE_SUITE_OPT_CONF_KEYS': ''}, - None ), - ('06_jinja2_thorough', {'XYZ': 'xyz'}, None), + ('06_jinja2_thorough', {'XYZ': 'xyz'}), ], ) -def test_process(tmp_path, srcdir, envvars, args): - if envvars is not None: - envvars = os.environ.update(envvars) +def test_process(srcdir, envvars): srcdir = Path(__file__).parent / srcdir result = run( ['cylc', 'view', '-p', str(srcdir)], capture_output=True, - env=envvars + env={**os.environ, **(envvars or {})} ).stdout.decode() expect = (srcdir / 'processed.conf.control').read_text() assert expect == result diff --git a/tests/unit/test_config_tree.py b/tests/unit/test_config_tree.py index 768ee584..fae84e5d 100644 --- a/tests/unit/test_config_tree.py +++ b/tests/unit/test_config_tree.py @@ -13,15 +13,9 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Tests the plugin with Rose suite configurations on the filesystem. - -Warning: - These tests share the same os.environ so may interact. - -""" +"""Tests the plugin with Rose suite configurations on the filesystem.""" from io import StringIO -import os from types import SimpleNamespace from cylc.flow.hostuserutil import get_host @@ -155,6 +149,7 @@ def wrapped_function(section): ] ) def test_get_rose_vars( + monkeypatch, rose_config_template, override, section, @@ -174,10 +169,10 @@ def test_get_rose_vars( opt_conf_keys=[], defines=[], rose_template_vars=[] ) if override == 'environment': - os.environ['ROSE_SUITE_OPT_CONF_KEYS'] = "gravy" + monkeypatch.setenv('ROSE_SUITE_OPT_CONF_KEYS', 'gravy') else: # Prevent externally set environment var breaking tests. - os.environ['ROSE_SUITE_OPT_CONF_KEYS'] = "" + monkeypatch.setenv('ROSE_SUITE_OPT_CONF_KEYS', '') if override == 'CLI': options.opt_conf_keys = ["chips"] elif override == 'override': @@ -206,9 +201,9 @@ def test_get_rose_vars_env_section(tmp_path): ) == 'Spaniel' -def test_get_rose_vars_expansions(tmp_path): +def test_get_rose_vars_expansions(monkeypatch, tmp_path): """Check that variables are expanded correctly.""" - os.environ['XYZ'] = "xyz" + monkeypatch.setenv('XYZ', 'xyz') (tmp_path / "rose-suite.conf").write_text( "[env]\n" "FOO=a\n" @@ -298,6 +293,7 @@ def test_get_rose_vars_fail_if_empy_AND_jinja2(tmp_path): ] ) def test_rose_config_tree_loader( + monkeypatch, rose_config_template, override, section, @@ -316,10 +312,10 @@ def test_rose_config_tree_loader( """ options = None if override == 'environment': - os.environ['ROSE_SUITE_OPT_CONF_KEYS'] = "gravy" + monkeypatch.setenv('ROSE_SUITE_OPT_CONF_KEYS', 'gravy') else: # Prevent externally set environment var breaking tests. - os.environ['ROSE_SUITE_OPT_CONF_KEYS'] = "" + monkeypatch.setenv('ROSE_SUITE_OPT_CONF_KEYS', '') if opts_format == 'list': conf_keys = ['chips'] else: From 9bd514374d4fe8c38adab55b02f1596eda5e88f3 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Tue, 31 Oct 2023 15:05:53 +0000 Subject: [PATCH 10/48] tests: fix accidentally skipped tests * `['a'].sort()` returns `None` not `['a']` * So `assert a.sort() == b.sort()` will always be `True`. --- tests/unit/test_config_tree.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/unit/test_config_tree.py b/tests/unit/test_config_tree.py index fae84e5d..393fb363 100644 --- a/tests/unit/test_config_tree.py +++ b/tests/unit/test_config_tree.py @@ -228,10 +228,10 @@ def test_get_rose_vars_ROSE_VARS(tmp_path): """Test that rose variables are available in the environment section..""" (tmp_path / "rose-suite.conf").touch() rose_vars = get_rose_vars(tmp_path) - assert list(rose_vars['env'].keys()).sort() == [ + assert sorted(rose_vars['env']) == sorted([ 'ROSE_ORIG_HOST', 'ROSE_VERSION', - ].sort() + ]) def test_get_rose_vars_jinja2_ROSE_VARS(tmp_path): @@ -240,13 +240,13 @@ def test_get_rose_vars_jinja2_ROSE_VARS(tmp_path): "[jinja2:suite.rc]" ) rose_vars = get_rose_vars(tmp_path) - assert list(rose_vars['template_variables'][ - 'ROSE_SUITE_VARIABLES' - ].keys()).sort() == [ + assert sorted( + rose_vars['template_variables']['ROSE_SUITE_VARIABLES'] + ) == sorted([ 'ROSE_VERSION', 'ROSE_ORIG_HOST', 'ROSE_SUITE_VARIABLES' - ].sort() + ]) def test_get_rose_vars_fail_if_empy_AND_jinja2(tmp_path): From 2f33ba1e1ae78f299e3d82050a74e3bbf043efad Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Tue, 31 Oct 2023 15:13:49 +0000 Subject: [PATCH 11/48] utils: make environ arg optional --- cylc/rose/utilities.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cylc/rose/utilities.py b/cylc/rose/utilities.py index 853d3fef..e56f854f 100644 --- a/cylc/rose/utilities.py +++ b/cylc/rose/utilities.py @@ -76,7 +76,7 @@ class InvalidDefineError(CylcError): ... -def get_rose_vars_from_config_node(config, config_node, environ): +def get_rose_vars_from_config_node(config, config_node, environ=os.environ): """Load template variables from a Rose config node. This uses only the provided config node and environment variables @@ -88,7 +88,7 @@ def get_rose_vars_from_config_node(config, config_node, environ): config_node (metomi.rose.config.ConfigNode): Configuration node representing the Rose suite configuration. environ (dict): - Dictionary of environment variables + Dictionary of environment variables (for testing). """ # Don't allow multiple templating sections. @@ -865,7 +865,6 @@ def get_rose_vars(srcdir=None, opts=None): get_rose_vars_from_config_node( config, config_tree.node, - os.environ ) # Export environment vars From 1ee64621ef0eeef7fcb9ff5acd5d8b57ab4f723f Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Tue, 31 Oct 2023 15:28:19 +0000 Subject: [PATCH 12/48] utils: remove unused "opts" arg in "rose_config_exists" --- cylc/rose/entry_points.py | 2 +- cylc/rose/fileinstall.py | 2 +- cylc/rose/utilities.py | 10 ++-------- tests/unit/test_config_tree.py | 20 +++++--------------- tests/unit/test_functional_post_install.py | 2 +- 5 files changed, 10 insertions(+), 26 deletions(-) diff --git a/cylc/rose/entry_points.py b/cylc/rose/entry_points.py index f169be90..7cf88a7b 100644 --- a/cylc/rose/entry_points.py +++ b/cylc/rose/entry_points.py @@ -35,7 +35,7 @@ def post_install(srcdir=None, opts=None, rundir=None): """Run after Cylc file installation has completed.""" from cylc.rose.fileinstall import rose_fileinstall - if not rose_config_exists(srcdir, opts): + if not rose_config_exists(srcdir): return False srcdir, rundir = paths_to_pathlib([srcdir, rundir]) results = {} diff --git a/cylc/rose/fileinstall.py b/cylc/rose/fileinstall.py index 4c9c83b0..1e9dc0ea 100644 --- a/cylc/rose/fileinstall.py +++ b/cylc/rose/fileinstall.py @@ -30,7 +30,7 @@ def rose_fileinstall(srcdir=None, opts=None, rundir=None): rundir (pathlib.Path) """ - if not rose_config_exists(rundir, opts): + if not rose_config_exists(rundir): return False # Load the config tree diff --git a/cylc/rose/utilities.py b/cylc/rose/utilities.py index e56f854f..69fc9370 100644 --- a/cylc/rose/utilities.py +++ b/cylc/rose/utilities.py @@ -43,9 +43,6 @@ from cylc.rose.jinja2_parser import Parser, patch_jinja2_leading_zeros -if TYPE_CHECKING: - from optparse import Values - SECTIONS = {'jinja2:suite.rc', 'empy:suite.rc', 'template variables'} SET_BY_CYLC = 'set by Cylc' @@ -229,14 +226,11 @@ def id_templating_section( return templating -def rose_config_exists( - srcdir: Union[Path, str, None], opts: 'Values' -) -> bool: +def rose_config_exists(srcdir: Union[Path, str, None]) -> bool: """Do opts or srcdir contain a rose config? Args: srcdir: location to test. - opts: Cylc Rose options, which might contain config items. Returns: True if a ``rose-suite.conf`` exists, or option config items have @@ -844,7 +838,7 @@ def get_rose_vars(srcdir=None, opts=None): } # Return a blank config dict if srcdir does not exist - if not rose_config_exists(srcdir, opts): + if not rose_config_exists(srcdir): if ( getattr(opts, "opt_conf_keys", None) or getattr(opts, "defines", None) diff --git a/tests/unit/test_config_tree.py b/tests/unit/test_config_tree.py index 393fb363..89eb608c 100644 --- a/tests/unit/test_config_tree.py +++ b/tests/unit/test_config_tree.py @@ -62,31 +62,21 @@ def test_node_stripper(): assert result == {'foo': 'bar'} -def test_rose_config_exists_no_dir(tmp_path): - assert rose_config_exists(None, SimpleNamespace( - opt_conf_keys=None, defines=[], rose_template_vars=[]) - ) is False +def test_rose_config_exists_no_dir(): + assert not rose_config_exists(None) def test_rose_config_exists_no_rose_suite_conf(tmp_path): - assert rose_config_exists( - tmp_path, SimpleNamespace( - opt_conf_keys=None, defines=[], rose_template_vars=[] - ) - ) is False + assert not rose_config_exists(tmp_path) def test_rose_config_exists_nonexistant_dir(tmp_path): - assert rose_config_exists( - tmp_path / "non-existant-folder", SimpleNamespace( - opt_conf_keys='', defines=[], rose_template_vars=[] - ) - ) is False + assert not rose_config_exists(tmp_path / "non-existant-folder") def test_rose_config_exists_true(tmp_path): (tmp_path / "rose-suite.conf").touch() - assert rose_config_exists(tmp_path, SimpleNamespace()) is True + assert rose_config_exists(tmp_path) @pytest.fixture diff --git a/tests/unit/test_functional_post_install.py b/tests/unit/test_functional_post_install.py index 967eeb35..0e293b3b 100644 --- a/tests/unit/test_functional_post_install.py +++ b/tests/unit/test_functional_post_install.py @@ -365,7 +365,7 @@ def fakenode(_, __): ) monkeypatch.setattr( 'cylc.rose.fileinstall.rose_config_exists', - lambda x, y: True, + lambda x: True, ) with pytest.raises(FileNotFoundError): rose_fileinstall(srcdir=tmp_path, rundir='/oiruhgaqhnaigujhj') From ba070631909b813695488f5a54e9c57f98a7361e Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Tue, 31 Oct 2023 15:31:55 +0000 Subject: [PATCH 13/48] fileinstall: remove unused "srcdir" argument --- cylc/rose/entry_points.py | 4 +--- cylc/rose/fileinstall.py | 2 +- tests/unit/test_functional_post_install.py | 17 +++++------------ 3 files changed, 7 insertions(+), 16 deletions(-) diff --git a/cylc/rose/entry_points.py b/cylc/rose/entry_points.py index 7cf88a7b..3639cf48 100644 --- a/cylc/rose/entry_points.py +++ b/cylc/rose/entry_points.py @@ -43,9 +43,7 @@ def post_install(srcdir=None, opts=None, rundir=None): results['record_install'] = record_cylc_install_options( srcdir=srcdir, opts=opts, rundir=rundir ) - results['fileinstall'] = rose_fileinstall( - srcdir=srcdir, opts=opts, rundir=rundir - ) + results['fileinstall'] = rose_fileinstall(opts=opts, rundir=rundir) # Finally dump a log of the rose-conf in its final state. if results['fileinstall']: dump_rose_log(rundir=rundir, node=results['fileinstall']) diff --git a/cylc/rose/fileinstall.py b/cylc/rose/fileinstall.py index 1e9dc0ea..c0eb2819 100644 --- a/cylc/rose/fileinstall.py +++ b/cylc/rose/fileinstall.py @@ -21,7 +21,7 @@ from cylc.rose.utilities import rose_config_exists, rose_config_tree_loader -def rose_fileinstall(srcdir=None, opts=None, rundir=None): +def rose_fileinstall(opts=None, rundir=None): """Call Rose Fileinstall. Args: diff --git a/tests/unit/test_functional_post_install.py b/tests/unit/test_functional_post_install.py index 0e293b3b..dfcb57cf 100644 --- a/tests/unit/test_functional_post_install.py +++ b/tests/unit/test_functional_post_install.py @@ -65,11 +65,6 @@ def test_no_rose_suite_conf_in_devdir(tmp_path): assert result is False -def test_rose_fileinstall_no_config_in_folder(): - # It returns false if no rose-suite.conf - assert rose_fileinstall(Path('/dev/null')) is False - - def test_rose_fileinstall_uses_rose_template_vars(tmp_path): # Setup source and destination dirs, including the file ``installme``: srcdir = tmp_path / 'source' @@ -89,7 +84,7 @@ def test_rose_fileinstall_uses_rose_template_vars(tmp_path): # Run both record_cylc_install options and fileinstall. record_cylc_install_options(opts=opts, rundir=destdir) - rose_fileinstall(srcdir, opts, destdir) + rose_fileinstall(opts, destdir) assert ((destdir / 'installedme').read_text() == 'Galileo No! We will not let you go.' ) @@ -259,9 +254,7 @@ def fake(*arg, **kwargs): rundir=testdir, opts=opts, srcdir=testdir ) ) - rose_fileinstall( - rundir=testdir, opts=opts, srcdir=testdir - ) + rose_fileinstall(rundir=testdir, opts=opts) ritems = sorted([i.relative_to(refdir) for i in refdir.rglob('*')]) titems = sorted([i.relative_to(testdir) for i in testdir.rglob('*')]) assert titems == ritems @@ -287,7 +280,7 @@ def test_functional_rose_database_dumped_correctly(tmp_path): "[file:Gnu]\nsrc=nicest_work_of.nature\n" ) (rundir / 'cylc.flow').touch() - rose_fileinstall(srcdir=srcdir, rundir=rundir) + rose_fileinstall(rundir=rundir) assert (rundir / '.rose-config_processors-file.db').is_file() @@ -300,7 +293,7 @@ def test_functional_rose_database_dumped_errors(tmp_path): "[file:Gnu]\nsrc=nicest_work_of.nature\n" ) (srcdir / 'cylc.flow').touch() - assert rose_fileinstall(srcdir=Path('/this/path/goes/nowhere')) is False + assert rose_fileinstall() is False @pytest.mark.parametrize( @@ -368,7 +361,7 @@ def fakenode(_, __): lambda x: True, ) with pytest.raises(FileNotFoundError): - rose_fileinstall(srcdir=tmp_path, rundir='/oiruhgaqhnaigujhj') + rose_fileinstall(rundir='/oiruhgaqhnaigujhj') def test_cylc_no_rose(tmp_path): From aa79c43dd073069cbe8fa48bc4aee98b11c482ba Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Tue, 31 Oct 2023 15:50:38 +0000 Subject: [PATCH 14/48] entry_points: catch None values earlier * Avoid having to handle None values for srcdir/rundir throughout the codebase by catching them earlier. --- cylc/rose/entry_points.py | 30 ++++++++++++---- cylc/rose/fileinstall.py | 7 +++- cylc/rose/utilities.py | 42 ++++------------------ tests/functional/test_copy_config_file.py | 40 +++++---------------- tests/unit/test_config_tree.py | 4 --- tests/unit/test_functional_post_install.py | 30 ++++------------ tests/unit/test_generic_utils.py | 42 ---------------------- 7 files changed, 51 insertions(+), 144 deletions(-) delete mode 100644 tests/unit/test_generic_utils.py diff --git a/cylc/rose/entry_points.py b/cylc/rose/entry_points.py index 3639cf48..1f8680bd 100644 --- a/cylc/rose/entry_points.py +++ b/cylc/rose/entry_points.py @@ -15,35 +15,51 @@ # along with this program. If not, see . """Top level module providing entry point functions.""" +from pathlib import Path +from typing import Union + from cylc.rose.utilities import ( copy_config_file, dump_rose_log, get_rose_vars, - paths_to_pathlib, record_cylc_install_options, rose_config_exists, ) -def pre_configure(srcdir=None, opts=None, rundir=None): +def pre_configure(srcdir=None, opts=None, rundir=None) -> dict: """Run before the Cylc configuration is read.""" - srcdir, rundir = paths_to_pathlib([srcdir, rundir]) + if not srcdir: + # not sure how this could happen + return { + 'env': {}, + 'template_variables': {}, + 'templating_detected': None + } + srcdir: Path = Path(srcdir) return get_rose_vars(srcdir=srcdir, opts=opts) -def post_install(srcdir=None, opts=None, rundir=None): +def post_install(srcdir=None, opts=None, rundir=None) -> Union[dict, bool]: """Run after Cylc file installation has completed.""" from cylc.rose.fileinstall import rose_fileinstall - if not rose_config_exists(srcdir): + if ( + not srcdir + or not rundir + or not rose_config_exists(srcdir) + ): + # nothing to do here return False - srcdir, rundir = paths_to_pathlib([srcdir, rundir]) + srcdir: Path = Path(srcdir) + rundir: Path = Path(rundir) + results = {} copy_config_file(srcdir=srcdir, rundir=rundir) results['record_install'] = record_cylc_install_options( srcdir=srcdir, opts=opts, rundir=rundir ) - results['fileinstall'] = rose_fileinstall(opts=opts, rundir=rundir) + results['fileinstall'] = rose_fileinstall(rundir, opts) # Finally dump a log of the rose-conf in its final state. if results['fileinstall']: dump_rose_log(rundir=rundir, node=results['fileinstall']) diff --git a/cylc/rose/fileinstall.py b/cylc/rose/fileinstall.py index c0eb2819..514d06d0 100644 --- a/cylc/rose/fileinstall.py +++ b/cylc/rose/fileinstall.py @@ -17,11 +17,16 @@ """Utilities related to performing Rose file installation.""" import os +from typing import TYPE_CHECKING from cylc.rose.utilities import rose_config_exists, rose_config_tree_loader +if TYPE_CHECKING: + from pathlib import Path + from cylc.flow.option_parser import Values -def rose_fileinstall(opts=None, rundir=None): + +def rose_fileinstall(rundir: 'Path', opts: 'Values'): """Call Rose Fileinstall. Args: diff --git a/cylc/rose/utilities.py b/cylc/rose/utilities.py index 69fc9370..c4019cc3 100644 --- a/cylc/rose/utilities.py +++ b/cylc/rose/utilities.py @@ -24,7 +24,7 @@ import re import shlex import shutil -from typing import TYPE_CHECKING, Any, List, Optional, Tuple, Union +from typing import Any, List, Optional, Tuple, Union from cylc.flow import LOG from cylc.flow.exceptions import CylcError @@ -226,22 +226,17 @@ def id_templating_section( return templating -def rose_config_exists(srcdir: Union[Path, str, None]) -> bool: - """Do opts or srcdir contain a rose config? +def rose_config_exists(dir_: Path) -> bool: + """Does dir_ a rose config? Args: - srcdir: location to test. + dir_: location to test. Returns: True if a ``rose-suite.conf`` exists, or option config items have been set. """ - # Return false if source dir doesn't exist. - if srcdir is None: - return False - - # Return true if and only if the rose suite.conf exists. - return Path(srcdir, 'rose-suite.conf').is_file() + return (dir_ / 'rose-suite.conf').is_file() def rose_config_tree_loader(srcdir=None, opts=None): @@ -662,16 +657,6 @@ def dump_rose_log(rundir, node): return rel_path -def paths_to_pathlib(paths): - """Convert paths to pathlib - """ - return [ - Path(path) if path is not None - else None - for path in paths - ] - - def override_this_variable(node, section, variable): """Variable exists in this section of the config and should be replaced because it is a standard variable. @@ -959,17 +944,14 @@ def record_cylc_install_options( def copy_config_file( - srcdir=None, - opts=None, - rundir=None, + srcdir: Path, + rundir: Path, ): """Copy the ``rose-suite.conf`` from a workflow source to run directory. Args: srcdir (pathlib.Path | or str): Source Path of Cylc install. - opts: - Not used in this function, but requried for consistent entry point. rundir (pathlib.Path | or str): Destination path of Cylc install - the workflow rundir. @@ -977,16 +959,6 @@ def copy_config_file( True if ``rose-suite.conf`` has been installed. False if insufficiant information to install file given. """ - if ( - rundir is None or - srcdir is None - ): - raise FileNotFoundError( - "This plugin requires both source and rundir to exist." - ) - - rundir = Path(rundir) - srcdir = Path(srcdir) srcdir_rose_conf = srcdir / 'rose-suite.conf' rundir_rose_conf = rundir / 'rose-suite.conf' diff --git a/tests/functional/test_copy_config_file.py b/tests/functional/test_copy_config_file.py index 76ec3cd6..caed1a13 100644 --- a/tests/functional/test_copy_config_file.py +++ b/tests/functional/test_copy_config_file.py @@ -19,43 +19,21 @@ from pathlib import Path -import pytest - from cylc.rose.entry_points import copy_config_file -@pytest.mark.parametrize( - 'sources, inputs, expect', - [ - ( - # Valid sourcedir with rose file, rose file at dest: - { - 'src/rose-suite.conf': '[env]\nFOO=2', - 'dest/rose-suite.conf': '[env]\nFOO=1' - }, - {'srcdir': 'src', 'rundir': 'dest'}, - True - ) - ] -) -def test_basic(tmp_path, sources, inputs, expect): +def test_basic(tmp_path): # Create files - for fname, content in sources.items(): + for fname, content in ( + ('src/rose-suite.conf', '[env]\nFOO=2'), + ('dest/rose-suite.conf', '[env]\nFOO=1'), + ): fname = Path(tmp_path / fname) fname.parent.mkdir(parents=True, exist_ok=True) fname.write_text(content) - # Flesh-out filepaths. - inputs = { - kwarg: Path(tmp_path / path) for kwarg, path in inputs.items() - if path is not None - } - # Test - if expect: - assert copy_config_file(**inputs) == expect - assert (Path(tmp_path / 'src/rose-suite.conf').read_text() == - Path(tmp_path / 'dest/rose-suite.conf').read_text() - ) - else: - assert copy_config_file(**inputs) == expect + assert copy_config_file(tmp_path / 'src', tmp_path / 'dest') + assert Path(tmp_path / 'src/rose-suite.conf').read_text() == ( + Path(tmp_path / 'dest/rose-suite.conf').read_text() + ) diff --git a/tests/unit/test_config_tree.py b/tests/unit/test_config_tree.py index 89eb608c..7557aa18 100644 --- a/tests/unit/test_config_tree.py +++ b/tests/unit/test_config_tree.py @@ -62,10 +62,6 @@ def test_node_stripper(): assert result == {'foo': 'bar'} -def test_rose_config_exists_no_dir(): - assert not rose_config_exists(None) - - def test_rose_config_exists_no_rose_suite_conf(tmp_path): assert not rose_config_exists(tmp_path) diff --git a/tests/unit/test_functional_post_install.py b/tests/unit/test_functional_post_install.py index dfcb57cf..9371714a 100644 --- a/tests/unit/test_functional_post_install.py +++ b/tests/unit/test_functional_post_install.py @@ -21,7 +21,6 @@ installation. """ -from pathlib import Path from types import SimpleNamespace from cylc.flow.hostuserutil import get_host @@ -84,7 +83,7 @@ def test_rose_fileinstall_uses_rose_template_vars(tmp_path): # Run both record_cylc_install options and fileinstall. record_cylc_install_options(opts=opts, rundir=destdir) - rose_fileinstall(opts, destdir) + rose_fileinstall(destdir, opts) assert ((destdir / 'installedme').read_text() == 'Galileo No! We will not let you go.' ) @@ -254,7 +253,7 @@ def fake(*arg, **kwargs): rundir=testdir, opts=opts, srcdir=testdir ) ) - rose_fileinstall(rundir=testdir, opts=opts) + rose_fileinstall(testdir, opts) ritems = sorted([i.relative_to(refdir) for i in refdir.rglob('*')]) titems = sorted([i.relative_to(testdir) for i in testdir.rglob('*')]) assert titems == ritems @@ -280,22 +279,11 @@ def test_functional_rose_database_dumped_correctly(tmp_path): "[file:Gnu]\nsrc=nicest_work_of.nature\n" ) (rundir / 'cylc.flow').touch() - rose_fileinstall(rundir=rundir) + rose_fileinstall(rundir, SimpleNamespace()) assert (rundir / '.rose-config_processors-file.db').is_file() -def test_functional_rose_database_dumped_errors(tmp_path): - srcdir = (tmp_path / 'srcdir') - srcdir.mkdir() - (srcdir / 'nicest_work_of.nature').touch() - (srcdir / 'rose-suite.conf').write_text( - "[file:Gnu]\nsrc=nicest_work_of.nature\n" - ) - (srcdir / 'cylc.flow').touch() - assert rose_fileinstall() is False - - @pytest.mark.parametrize( ( 'opts, files, expect' @@ -361,7 +349,7 @@ def fakenode(_, __): lambda x: True, ) with pytest.raises(FileNotFoundError): - rose_fileinstall(rundir='/oiruhgaqhnaigujhj') + rose_fileinstall('/oiruhgaqhnaigujhj', SimpleNamespace()) def test_cylc_no_rose(tmp_path): @@ -371,12 +359,6 @@ def test_cylc_no_rose(tmp_path): assert post_install(srcdir=tmp_path, rundir=tmp_path) is False -def test_copy_config_file_fails(): - """It fails when source or rundir not specified.""" - with pytest.raises(FileNotFoundError, match='both source and rundir'): - copy_config_file() - - -def test_copy_config_file_fails2(): +def test_copy_config_file_fails(tmp_path): """It fails if source not a rose suite.""" - copy_config_file(srcdir='/foo', rundir='/bar') + copy_config_file(tmp_path, tmp_path) diff --git a/tests/unit/test_generic_utils.py b/tests/unit/test_generic_utils.py deleted file mode 100644 index 3005769c..00000000 --- a/tests/unit/test_generic_utils.py +++ /dev/null @@ -1,42 +0,0 @@ -# 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 . -"""Test generic ultilities -""" - -from pathlib import Path - -import pytest - -from cylc.rose.utilities import paths_to_pathlib - - -@pytest.mark.parametrize( - 'paths, expect', - [ - # None is passed through: - ([None, None], [None, None]), - # Path as string is made Path: - (['/foot/path/', None], [Path('/foot/path'), None]), - # Path as Path left alone: - ([Path('/cycle/path/'), None], [Path('/cycle/path'), None]), - # Different starting types re-typed independently: - ( - [Path('/cycle/path/'), '/bridle/path'], - [Path('/cycle/path'), Path('/bridle/path')]), - ] -) -def test_paths_to_pathlib(paths, expect): - assert paths_to_pathlib(paths) == expect From d1ff77dac28807ff928604a0cebaf404bdad2007 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Tue, 31 Oct 2023 17:50:12 +0000 Subject: [PATCH 15/48] utils: split the loading and processing of the config * A future rose stem interface will need to inject itself between config loading and processing. * So colocate the plugin processing logic into one function. --- cylc/rose/entry_points.py | 18 ++++- cylc/rose/fileinstall.py | 1 + cylc/rose/stem.py | 18 ++++- cylc/rose/utilities.py | 103 ++++++++++++++----------- tests/functional/test_pre_configure.py | 8 +- tests/unit/test_config_node.py | 31 ++++---- tests/unit/test_config_tree.py | 57 +++++++++----- tests/unit/test_rose_stem_units.py | 10 ++- 8 files changed, 154 insertions(+), 92 deletions(-) diff --git a/cylc/rose/entry_points.py b/cylc/rose/entry_points.py index 1f8680bd..2389a139 100644 --- a/cylc/rose/entry_points.py +++ b/cylc/rose/entry_points.py @@ -21,7 +21,9 @@ from cylc.rose.utilities import ( copy_config_file, dump_rose_log, - get_rose_vars, + export_environment, + load_rose_config, + process_config, record_cylc_install_options, rose_config_exists, ) @@ -32,12 +34,22 @@ def pre_configure(srcdir=None, opts=None, rundir=None) -> dict: if not srcdir: # not sure how this could happen return { + # default return value 'env': {}, 'template_variables': {}, 'templating_detected': None } - srcdir: Path = Path(srcdir) - return get_rose_vars(srcdir=srcdir, opts=opts) + + # load the Rose config + config_tree = load_rose_config(Path(srcdir), opts=opts) + + # extract plugin return information from the Rose config + plugin_result = process_config(config_tree) + + # set environment variables + export_environment(plugin_result['env']) + + return plugin_result def post_install(srcdir=None, opts=None, rundir=None) -> Union[dict, bool]: diff --git a/cylc/rose/fileinstall.py b/cylc/rose/fileinstall.py index 514d06d0..1be4cfdd 100644 --- a/cylc/rose/fileinstall.py +++ b/cylc/rose/fileinstall.py @@ -39,6 +39,7 @@ def rose_fileinstall(rundir: 'Path', opts: 'Values'): return False # Load the config tree + # TODO: private config_tree = rose_config_tree_loader(rundir, opts) if any(i.startswith('file') for i in config_tree.node.value): diff --git a/cylc/rose/stem.py b/cylc/rose/stem.py index 47e816a1..2a556cc0 100644 --- a/cylc/rose/stem.py +++ b/cylc/rose/stem.py @@ -78,8 +78,14 @@ from metomi.rose.reporter import Event, Reporter from metomi.rose.resource import ResourceLocator -from cylc.rose.entry_points import get_rose_vars -from cylc.rose.utilities import id_templating_section +from cylc.rose.entry_points import ( + export_environment, + load_rose_config, +) +from cylc.rose.utilities import ( + id_templating_section, + process_config, +) EXC_EXIT = cparse('{name}: {exc}') DEFAULT_TEST_DIR = 'rose-stem' @@ -456,8 +462,12 @@ def process(self): self.opts.project.append(project) if i == 0: - template_type = get_rose_vars( - Path(url) / "rose-stem")["templating_detected"] + config_tree = load_rose_config(Path(url) / "rose-stem") + plugin_result = process_config(config_tree) + # set environment variables + export_environment(plugin_result['env']) + template_type = plugin_result['templating_detected'] + self.template_section = id_templating_section( template_type, with_brackets=True) diff --git a/cylc/rose/utilities.py b/cylc/rose/utilities.py index c4019cc3..b2ea9905 100644 --- a/cylc/rose/utilities.py +++ b/cylc/rose/utilities.py @@ -24,7 +24,7 @@ import re import shlex import shutil -from typing import Any, List, Optional, Tuple, Union +from typing import Any, Dict, List, Optional, Tuple, Union from cylc.flow import LOG from cylc.flow.exceptions import CylcError @@ -39,6 +39,7 @@ ConfigNodeDiff, ) from metomi.rose.config_processor import ConfigProcessError +from metomi.rose.config_tree import ConfigTree from metomi.rose.env import UnboundEnvironmentVariableError, env_var_process from cylc.rose.jinja2_parser import Parser, patch_jinja2_leading_zeros @@ -73,28 +74,41 @@ class InvalidDefineError(CylcError): ... -def get_rose_vars_from_config_node(config, config_node, environ=os.environ): - """Load template variables from a Rose config node. +def process_config( + config_tree: 'ConfigTree', + environ=os.environ, +) -> Dict[str, Any]: + """Process template and environment variables. - This uses only the provided config node and environment variables - - there is no system interaction. + Note: + This uses only the provided config node and environment variables, + there is no system interaction. Args: - config (dict): - Object which will be populated with the results. - config_node (metomi.rose.config.ConfigNode): + config_tree: Configuration node representing the Rose suite configuration. - environ (dict): + environ: Dictionary of environment variables (for testing). """ + plugin_result: Dict[str, Any] = { + # default return value + 'env': {}, + 'template_variables': {}, + 'templating_detected': None + } + config_node = config_tree.node + # Don't allow multiple templating sections. templating = identify_templating_section(config_node) if templating != 'template variables': - config['templating_detected'] = templating.replace(':suite.rc', '') + plugin_result['templating_detected'] = templating.replace( + ':suite.rc', + '', + ) else: - config['templating_detected'] = templating + plugin_result['templating_detected'] = templating # Create env section if it doesn't already exist. if 'env' not in config_node.value: @@ -151,29 +165,29 @@ def get_rose_vars_from_config_node(config, config_node, environ=os.environ): # For each of the template language sections extract items to a simple # dict to be returned. - config['env'] = { + plugin_result['env'] = { item[0][1]: item[1].value for item in config_node.value['env'].walk() if item[1].state == ConfigNode.STATE_NORMAL } - config['template_variables'] = { + plugin_result['template_variables'] = { item[0][1]: item[1].value for item in config_node.value[templating].walk() if item[1].state == ConfigNode.STATE_NORMAL } - # Add the entire config to ROSE_SUITE_VARIABLES to allow for programatic - # access. + # Add the entire plugin_result to ROSE_SUITE_VARIABLES to allow for + # programatic access. with patch_jinja2_leading_zeros(): # BACK COMPAT: patch_jinja2_leading_zeros # back support zero-padded integers for a limited time to help # users migrate before upgrading cylc-flow to Jinja2>=3.1 parser = Parser() - for key, value in config['template_variables'].items(): + for key, value in plugin_result['template_variables'].items(): # The special variables are already Python variables. if key not in ['ROSE_ORIG_HOST', 'ROSE_VERSION', 'ROSE_SITE']: try: - config['template_variables'][key] = ( + plugin_result['template_variables'][key] = ( parser.literal_eval(value) ) except Exception: @@ -185,9 +199,11 @@ def get_rose_vars_from_config_node(config, config_node, environ=os.environ): ' (note strings "must be quoted").' ) from None - # Add ROSE_SUITE_VARIABLES to config of templating engines in use. - config['template_variables'][ - 'ROSE_SUITE_VARIABLES'] = config['template_variables'] + # Add ROSE_SUITE_VARIABLES to plugin_result of templating engines in use. + plugin_result['template_variables'][ + 'ROSE_SUITE_VARIABLES'] = plugin_result['template_variables'] + + return plugin_result def identify_templating_section(config_node): @@ -789,19 +805,27 @@ def deprecation_warnings(config_tree): LOG.warning(info[MESSAGE]) -def get_rose_vars(srcdir=None, opts=None): - """Load template variables from Rose suite configuration. +def load_rose_config( + srcdir: Path, + opts=None, + warn: bool = True, +) -> 'ConfigTree': + """Load rose configuration from srcdir. + + Load template variables from Rose suite configuration. Loads the Rose suite configuration tree from the filesystem using the shell environment. Args: - srcdir(pathlib.Path): + srcdir: Path to the Rose suite configuration (the directory containing the ``rose-suite.conf`` file). opts: Options object containing specification of optional configuarations set by the CLI. + warn: + Log deprecation warnings if True. Returns: dict - A dictionary of sections of rose-suite.conf. @@ -815,22 +839,18 @@ def get_rose_vars(srcdir=None, opts=None): } } """ - # Set up blank page for returns. - config = { - 'env': {}, - 'template_variables': {}, - 'templating_detected': None - } - # Return a blank config dict if srcdir does not exist if not rose_config_exists(srcdir): if ( - getattr(opts, "opt_conf_keys", None) - or getattr(opts, "defines", None) - or getattr(opts, "rose_template_vars", None) + opts + and ( + getattr(opts, "opt_conf_keys", None) + or getattr(opts, "defines", None) + or getattr(opts, "rose_template_vars", None) + ) ): raise NotARoseSuiteException() - return config + return ConfigTree() # Check for definitely invalid defines if opts and hasattr(opts, 'defines'): @@ -838,20 +858,17 @@ def get_rose_vars(srcdir=None, opts=None): # Load the raw config tree config_tree = rose_config_tree_loader(srcdir, opts) - deprecation_warnings(config_tree) + if warn: + deprecation_warnings(config_tree) - # Extract templatevars from the configuration - get_rose_vars_from_config_node( - config, - config_tree.node, - ) + return config_tree + +def export_environment(environment): # Export environment vars - for key, val in config['env'].items(): + for key, val in environment.items(): os.environ[key] = val - return config - def record_cylc_install_options( rundir=None, diff --git a/tests/functional/test_pre_configure.py b/tests/functional/test_pre_configure.py index baecc6df..7785735b 100644 --- a/tests/functional/test_pre_configure.py +++ b/tests/functional/test_pre_configure.py @@ -27,7 +27,7 @@ import pytest from pytest import param -from cylc.rose.utilities import NotARoseSuiteException, get_rose_vars +from cylc.rose.utilities import NotARoseSuiteException, load_rose_config @pytest.mark.parametrize( @@ -121,7 +121,7 @@ def test_process(srcdir, envvars): def test_warn_if_root_dir_set(root_dir_config, tmp_path, caplog): """Test using unsupported root-dir config raises error.""" (tmp_path / 'rose-suite.conf').write_text(root_dir_config) - get_rose_vars(srcdir=tmp_path) + load_rose_config(tmp_path) msg = 'rose-suite.conf[root-dir]' assert msg in caplog.records[0].msg @@ -150,7 +150,7 @@ def test_warn_if_old_templating_set( 'cylc.rose.utilities.cylc7_back_compat', compat_mode ) (tmp_path / 'rose-suite.conf').write_text(f'[{rose_config}]') - get_rose_vars(srcdir=tmp_path) + load_rose_config(tmp_path) msg = "Use [template variables]" if compat_mode: assert not caplog.records @@ -174,7 +174,7 @@ def test_fail_if_options_but_no_rose_suite_conf(opts, tmp_path): NotARoseSuiteException, match='^Cylc-Rose CLI' ): - get_rose_vars(tmp_path, opts) + load_rose_config(tmp_path, opts) def generate_params(): diff --git a/tests/unit/test_config_node.py b/tests/unit/test_config_node.py index 848815e3..8c697dad 100644 --- a/tests/unit/test_config_node.py +++ b/tests/unit/test_config_node.py @@ -20,6 +20,7 @@ from metomi.isodatetime.datetimeoper import DateTimeOperator from metomi.rose import __version__ as ROSE_VERSION from metomi.rose.config import ConfigNode +from metomi.rose.config_tree import ConfigTree from metomi.rose.config_processor import ConfigProcessError import pytest @@ -29,7 +30,7 @@ add_cylc_install_to_rose_conf_node_opts, deprecation_warnings, dump_rose_log, - get_rose_vars_from_config_node, + process_config, id_templating_section, identify_templating_section, ) @@ -37,9 +38,7 @@ def test_blank(): """It should provide only standard vars for a blank config.""" - ret = {} - node = ConfigNode() - get_rose_vars_from_config_node(ret, node, {}) + ret = process_config(ConfigTree(), {}) assert set(ret.keys()) == { 'template_variables', 'templating_detected', 'env' } @@ -51,37 +50,40 @@ def test_blank(): def test_invalid_templatevar(): """It should wrap eval errors as something more informative.""" - ret = {} + tree = ConfigTree() node = ConfigNode() node.set(['jinja2:suite.rc', 'X'], 'Y') + tree.node = node with pytest.raises(ConfigProcessError): - get_rose_vars_from_config_node(ret, node, {}) + process_config(tree, {}) -def test_get_rose_vars_from_config_node__unbound_env_var(caplog): +def test_get_plugin_result__unbound_env_var(caplog): """It should fail if variable unset in environment. """ - ret = {} + tree = ConfigTree() node = ConfigNode() node.set(['env', 'X'], '${MYVAR}') + tree.node = node with pytest.raises(ConfigProcessError) as exc: - get_rose_vars_from_config_node(ret, node, {}) + process_config(tree, {}) assert exc.match('env=X: MYVAR: unbound variable') @pytest.fixture def override_version_vars(caplog, scope='module'): - """Set up config tree and pass to get_rose_vars_from_config_node + """Set up config tree and pass to process_config Yields: node: The node after manipulation. message: A string representing the caplog output. """ - ret = {} + tree = ConfigTree() node = ConfigNode() node.set(['template variables', 'ROSE_VERSION'], 99) node.set(['template variables', 'CYLC_VERSION'], 101) - get_rose_vars_from_config_node(ret, node, {}) + tree.node = node + process_config(tree, {}) message = '\n'.join([i.message for i in caplog.records]) yield (node, message) @@ -268,11 +270,12 @@ def test_id_templating_section(input_, expect): @pytest.fixture def node_with_ROSE_ORIG_HOST(): def _inner(comment=''): - ret = {} + tree = ConfigTree() node = ConfigNode() node.set(['env', 'ROSE_ORIG_HOST'], 'IMPLAUSIBLE_HOST_NAME') node['env']['ROSE_ORIG_HOST'].comments = [comment] - get_rose_vars_from_config_node(ret, node, {}) + tree.node = node + process_config(tree, {}) return node yield _inner diff --git a/tests/unit/test_config_tree.py b/tests/unit/test_config_tree.py index 7557aa18..497b6ddd 100644 --- a/tests/unit/test_config_tree.py +++ b/tests/unit/test_config_tree.py @@ -23,7 +23,10 @@ import pytest from pytest import param -from cylc.rose.entry_points import get_rose_vars +from cylc.rose.entry_points import ( + process_config, + load_rose_config, +) from cylc.rose.utilities import ( MultipleTemplatingEnginesError, get_cli_opts_node, @@ -167,12 +170,14 @@ def test_get_rose_vars( f"[{section}:suite.rc]Another_Jinja2_var='Variable overridden'" ] suite_path = rose_config_template(section) - result = get_rose_vars( + config_tree = load_rose_config( suite_path, options - )['template_variables'] - - assert result['Another_Jinja2_var'] == exp_ANOTHER_JINJA2_ENV - assert result['JINJA2_VAR'] == exp_JINJA2_VAR + ) + template_variables = ( + process_config(config_tree)['template_variables'] + ) + assert template_variables['Another_Jinja2_var'] == exp_ANOTHER_JINJA2_ENV + assert template_variables['JINJA2_VAR'] == exp_JINJA2_VAR def test_get_rose_vars_env_section(tmp_path): @@ -182,9 +187,9 @@ def test_get_rose_vars_env_section(tmp_path): "DOG_TYPE = Spaniel \n" ) - assert ( - get_rose_vars(tmp_path)['env']['DOG_TYPE'] - ) == 'Spaniel' + config_tree = load_rose_config(tmp_path) + environment_variables = process_config(config_tree)['env'] + assert environment_variables['DOG_TYPE'] == 'Spaniel' def test_get_rose_vars_expansions(monkeypatch, tmp_path): @@ -201,20 +206,26 @@ def test_get_rose_vars_expansions(monkeypatch, tmp_path): "BOOL=True\n" 'LIST=["a", 1, True]\n' ) - rose_vars = get_rose_vars(tmp_path) - assert rose_vars['template_variables']['LOCAL_ENV'] == 'xyz' - assert rose_vars['template_variables']['BAR'] == 'ab' - assert rose_vars['template_variables']['ESCAPED_ENV'] == '$HOME' - assert rose_vars['template_variables']['INT'] == 42 - assert rose_vars['template_variables']['BOOL'] is True - assert rose_vars['template_variables']['LIST'] == ["a", 1, True] + config_tree = load_rose_config(tmp_path) + template_variables = ( + process_config(config_tree)['template_variables'] + ) + assert template_variables['LOCAL_ENV'] == 'xyz' + assert template_variables['BAR'] == 'ab' + assert template_variables['ESCAPED_ENV'] == '$HOME' + assert template_variables['INT'] == 42 + assert template_variables['BOOL'] is True + assert template_variables['LIST'] == ["a", 1, True] def test_get_rose_vars_ROSE_VARS(tmp_path): """Test that rose variables are available in the environment section..""" (tmp_path / "rose-suite.conf").touch() - rose_vars = get_rose_vars(tmp_path) - assert sorted(rose_vars['env']) == sorted([ + config_tree = load_rose_config(tmp_path) + environment_variables = ( + process_config(config_tree)['env'] + ) + assert sorted(environment_variables) == sorted([ 'ROSE_ORIG_HOST', 'ROSE_VERSION', ]) @@ -225,9 +236,12 @@ def test_get_rose_vars_jinja2_ROSE_VARS(tmp_path): (tmp_path / "rose-suite.conf").write_text( "[jinja2:suite.rc]" ) - rose_vars = get_rose_vars(tmp_path) + config_tree = load_rose_config(tmp_path) + template_variables = ( + process_config(config_tree)['template_variables'] + ) assert sorted( - rose_vars['template_variables']['ROSE_SUITE_VARIABLES'] + template_variables['ROSE_SUITE_VARIABLES'] ) == sorted([ 'ROSE_VERSION', 'ROSE_ORIG_HOST', @@ -241,8 +255,9 @@ def test_get_rose_vars_fail_if_empy_AND_jinja2(tmp_path): "[jinja2:suite.rc]\n" "[empy:suite.rc]\n" ) + config_tree = load_rose_config(tmp_path) with pytest.raises(MultipleTemplatingEnginesError): - get_rose_vars(tmp_path) + process_config(config_tree) @pytest.mark.parametrize( diff --git a/tests/unit/test_rose_stem_units.py b/tests/unit/test_rose_stem_units.py index f58dd47e..72b7fbbd 100644 --- a/tests/unit/test_rose_stem_units.py +++ b/tests/unit/test_rose_stem_units.py @@ -19,6 +19,7 @@ from types import SimpleNamespace from typing import Any, Tuple +from metomi.rose.config_tree import ConfigTree from metomi.rose.fs_util import FileSystemUtil from metomi.rose.popen import RosePopener from metomi.rose.reporter import Reporter @@ -427,10 +428,13 @@ def test_process_template_engine_set_correctly(monkeypatch, language, expect): https://github.com/cylc/cylc-rose/issues/246 """ - # Mimic expected result from get_rose_vars method: monkeypatch.setattr( - 'cylc.rose.stem.get_rose_vars', - lambda _: {'templating_detected': language} + 'cylc.rose.stem.load_rose_config', + lambda _: ConfigTree() + ) + monkeypatch.setattr( + 'cylc.rose.stem.process_config', + lambda _: {'templating_detected': language, 'env': {}} ) monkeypatch.setattr( 'sys.argv', From 0f376d111e8616ec47c575f3be7f1493526bbc3d Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Wed, 1 Nov 2023 11:10:19 +0000 Subject: [PATCH 16/48] entry_points: remove unused results return value --- cylc/rose/entry_points.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/cylc/rose/entry_points.py b/cylc/rose/entry_points.py index 2389a139..644e311e 100644 --- a/cylc/rose/entry_points.py +++ b/cylc/rose/entry_points.py @@ -16,7 +16,6 @@ """Top level module providing entry point functions.""" from pathlib import Path -from typing import Union from cylc.rose.utilities import ( copy_config_file, @@ -52,7 +51,7 @@ def pre_configure(srcdir=None, opts=None, rundir=None) -> dict: return plugin_result -def post_install(srcdir=None, opts=None, rundir=None) -> Union[dict, bool]: +def post_install(srcdir=None, opts=None, rundir=None) -> bool: """Run after Cylc file installation has completed.""" from cylc.rose.fileinstall import rose_fileinstall @@ -66,17 +65,20 @@ def post_install(srcdir=None, opts=None, rundir=None) -> Union[dict, bool]: srcdir: Path = Path(srcdir) rundir: Path = Path(rundir) - results = {} + # transfer the rose-suite.conf file copy_config_file(srcdir=srcdir, rundir=rundir) - results['record_install'] = record_cylc_install_options( + + # write cylc-install CLI options to an optional config + record_cylc_install_options( srcdir=srcdir, opts=opts, rundir=rundir ) - results['fileinstall'] = rose_fileinstall(rundir, opts) - # Finally dump a log of the rose-conf in its final state. - if results['fileinstall']: - dump_rose_log(rundir=rundir, node=results['fileinstall']) - return results + # perform file installation + config_node = rose_fileinstall(rundir, opts) + if config_node: + dump_rose_log(rundir=rundir, node=config_node) + + return True def rose_stem(): From 9bef3c8ed3fe90d9262b3a3c55c799ed85007513 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Wed, 1 Nov 2023 11:42:42 +0000 Subject: [PATCH 17/48] entry_points: remove type uncertainty * Entry points accept missing or None values for arguments, however, this can never happen in practice. * Set the appropriate type hints and remove code paths which could not get executed. --- cylc/rose/entry_points.py | 27 +++--- cylc/rose/fileinstall.py | 11 ++- cylc/rose/utilities.py | 102 +++++++++++---------- tests/unit/test_config_tree.py | 3 +- tests/unit/test_functional_post_install.py | 18 +--- 5 files changed, 80 insertions(+), 81 deletions(-) diff --git a/cylc/rose/entry_points.py b/cylc/rose/entry_points.py index 644e311e..98b6de37 100644 --- a/cylc/rose/entry_points.py +++ b/cylc/rose/entry_points.py @@ -16,6 +16,7 @@ """Top level module providing entry point functions.""" from pathlib import Path +from typing import TYPE_CHECKING from cylc.rose.utilities import ( copy_config_file, @@ -27,8 +28,11 @@ rose_config_exists, ) +if TYPE_CHECKING: + from cylc.flow.option_parsers import Values -def pre_configure(srcdir=None, opts=None, rundir=None) -> dict: + +def pre_configure(srcdir: Path, opts: 'Values') -> dict: """Run before the Cylc configuration is read.""" if not srcdir: # not sure how this could happen @@ -51,32 +55,25 @@ def pre_configure(srcdir=None, opts=None, rundir=None) -> dict: return plugin_result -def post_install(srcdir=None, opts=None, rundir=None) -> bool: +def post_install(srcdir: Path, rundir: str, opts: 'Values') -> bool: """Run after Cylc file installation has completed.""" from cylc.rose.fileinstall import rose_fileinstall - if ( - not srcdir - or not rundir - or not rose_config_exists(srcdir) - ): + if not rose_config_exists(srcdir): # nothing to do here return False - srcdir: Path = Path(srcdir) - rundir: Path = Path(rundir) + _rundir: Path = Path(rundir) # transfer the rose-suite.conf file - copy_config_file(srcdir=srcdir, rundir=rundir) + copy_config_file(srcdir=srcdir, rundir=_rundir) # write cylc-install CLI options to an optional config - record_cylc_install_options( - srcdir=srcdir, opts=opts, rundir=rundir - ) + record_cylc_install_options(srcdir, _rundir, opts) # perform file installation - config_node = rose_fileinstall(rundir, opts) + config_node = rose_fileinstall(_rundir, opts) if config_node: - dump_rose_log(rundir=rundir, node=config_node) + dump_rose_log(rundir=_rundir, node=config_node) return True diff --git a/cylc/rose/fileinstall.py b/cylc/rose/fileinstall.py index 1be4cfdd..986ab05f 100644 --- a/cylc/rose/fileinstall.py +++ b/cylc/rose/fileinstall.py @@ -17,16 +17,20 @@ """Utilities related to performing Rose file installation.""" import os -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Union from cylc.rose.utilities import rose_config_exists, rose_config_tree_loader if TYPE_CHECKING: from pathlib import Path - from cylc.flow.option_parser import Values + from cylc.flow.option_parsers import Values + from metomi.rose.config import ConfigNode -def rose_fileinstall(rundir: 'Path', opts: 'Values'): +def rose_fileinstall( + rundir: 'Path', + opts: 'Values', +) -> 'Union[ConfigNode, bool]': """Call Rose Fileinstall. Args: @@ -39,7 +43,6 @@ def rose_fileinstall(rundir: 'Path', opts: 'Values'): return False # Load the config tree - # TODO: private config_tree = rose_config_tree_loader(rundir, opts) if any(i.startswith('file') for i in config_tree.node.value): diff --git a/cylc/rose/utilities.py b/cylc/rose/utilities.py index b2ea9905..15540b21 100644 --- a/cylc/rose/utilities.py +++ b/cylc/rose/utilities.py @@ -24,7 +24,7 @@ import re import shlex import shutil -from typing import Any, Dict, List, Optional, Tuple, Union +from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple, Union from cylc.flow import LOG from cylc.flow.exceptions import CylcError @@ -44,6 +44,9 @@ from cylc.rose.jinja2_parser import Parser, patch_jinja2_leading_zeros +if TYPE_CHECKING: + from cylc.flow.option_parsers import Values + SECTIONS = {'jinja2:suite.rc', 'empy:suite.rc', 'template variables'} SET_BY_CYLC = 'set by Cylc' @@ -255,7 +258,10 @@ def rose_config_exists(dir_: Path) -> bool: return (dir_ / 'rose-suite.conf').is_file() -def rose_config_tree_loader(srcdir=None, opts=None): +def rose_config_tree_loader( + srcdir: Path, + opts: 'Optional[Values]', +) -> ConfigTree: """Get a rose config tree from srcdir. Args: @@ -298,7 +304,7 @@ def rose_config_tree_loader(srcdir=None, opts=None): # Reload the Config using the suite_ variables. # (we can't do this first time around because we have no idea what the # templating section is.) - if getattr(opts, 'rose_template_vars', None): + if opts and getattr(opts, 'rose_template_vars', None): template_section = identify_templating_section(config_tree.node) for template_var in opts.rose_template_vars or []: redefinitions.append(f'[{template_section}]{template_var}') @@ -451,7 +457,7 @@ def parse_cli_defines(define: str) -> Union[ return (keys, match['value'], match['state']) -def get_cli_opts_node(opts=None, srcdir=None): +def get_cli_opts_node(srcdir: Path, opts: 'Values'): """Create a ConfigNode representing options set on the command line. Args: @@ -463,12 +469,13 @@ def get_cli_opts_node(opts=None, srcdir=None): Example: >>> from types import SimpleNamespace + >>> from pathlib import Path >>> opts = SimpleNamespace( ... opt_conf_keys='A B', ... defines=["[env]FOO=BAR"], ... rose_template_vars=["QUX=BAZ"] ... ) - >>> node = get_cli_opts_node(opts) + >>> node = get_cli_opts_node(Path('no/such/dir'), opts) >>> node['opts'] {'value': 'A B', 'state': '!', 'comments': []} >>> node['env']['FOO'] @@ -477,9 +484,9 @@ def get_cli_opts_node(opts=None, srcdir=None): {'value': 'BAZ', 'state': '', 'comments': []} """ # Unpack info we want from opts: - opt_conf_keys = [] - defines = [] - rose_template_vars = [] + opt_conf_keys: list = [] + defines: list = [] + rose_template_vars: list = [] if opts and 'opt_conf_keys' in dir(opts): opt_conf_keys = opts.opt_conf_keys or [] if opts and 'defines' in dir(opts): @@ -502,21 +509,26 @@ def get_cli_opts_node(opts=None, srcdir=None): newconfig.set(*parsed_define) # For each __suite define__ add define. - if srcdir is not None: - config_node = rose_config_tree_loader(srcdir, opts).node - templating = identify_templating_section(config_node) - else: + templating: str + if not rose_config_exists(srcdir): templating = 'template variables' + else: + templating = identify_templating_section( + rose_config_tree_loader(srcdir, opts).node + ) for define in rose_template_vars: - match = re.match( + _match = re.match( r'(?P!{0,2})(?P.*)\s*=\s*(?P.*)', define - ).groupdict() + ) + if not _match: + raise ValueError(f'Invalid define: {define}') + _match_groups = _match.groupdict() # Guess templating type? newconfig.set( - keys=[templating, match['key']], - value=match['value'], - state=match['state'] + keys=[templating, _match_groups['key']], + value=_match_groups['value'], + state=_match_groups['state'] ) # Specialised treatement of optional configs. @@ -650,7 +662,7 @@ def simplify_opts_strings(opts): return ' '.join(reversed(seen_once)) -def dump_rose_log(rundir, node): +def dump_rose_log(rundir: Path, node: ConfigNode): """Dump a config node to a timestamped file in the ``log`` sub-directory. Args: @@ -807,8 +819,7 @@ def deprecation_warnings(config_tree): def load_rose_config( srcdir: Path, - opts=None, - warn: bool = True, + opts: 'Optional[Values]' = None, ) -> 'ConfigTree': """Load rose configuration from srcdir. @@ -824,20 +835,12 @@ def load_rose_config( opts: Options object containing specification of optional configuarations set by the CLI. - warn: - Log deprecation warnings if True. + + Note: this is None for "rose stem" usage. Returns: - dict - A dictionary of sections of rose-suite.conf. - For each section either a dictionary or None is returned. - E.g. - { - 'env': {'MYVAR': 42}, - 'empy:suite.rc': None, - 'jinja2:suite.rc': { - 'myJinja2Var': {'yes': 'it is a dictionary!'} - } - } + The Rose configuration tree for "srcdir". + """ # Return a blank config dict if srcdir does not exist if not rose_config_exists(srcdir): @@ -858,23 +861,22 @@ def load_rose_config( # Load the raw config tree config_tree = rose_config_tree_loader(srcdir, opts) - if warn: - deprecation_warnings(config_tree) + deprecation_warnings(config_tree) return config_tree -def export_environment(environment): +def export_environment(environment: Dict[str, str]) -> None: # Export environment vars for key, val in environment.items(): os.environ[key] = val def record_cylc_install_options( - rundir=None, - opts=None, - srcdir=None, -): + srcdir: Path, + rundir: Path, + opts: 'Values', +) -> Tuple[ConfigNode, ConfigNode]: """Create/modify files recording Cylc install config options. Creates a new config based on CLI options and writes it to the workflow @@ -887,28 +889,32 @@ def record_cylc_install_options( been written in the installed ``rose-suite.conf``. Args: - srcdir (pathlib.Path): + srcdir: Used to check whether the source directory contains a rose config. + rundir: + Path to dump the rose-suite-cylc-conf opts: Cylc option parser object - we want to extract the following values: - - opt_conf_keys (list or str): + - opt_conf_keys (list of str): Equivalent of ``rose suite-run --option KEY`` - defines (list of str): Equivalent of ``rose suite-run --define KEY=VAL`` - rose_template_vars (list of str): Equivalent of ``rose suite-run --define-suite KEY=VAL`` - rundir (pathlib.Path): - Path to dump the rose-suite-cylc-conf Returns: - cli_config - Config Node which has been dumped to - ``rose-suite-cylc-install.conf``. - rose_suite_conf['opts'] - Opts section of the config node dumped to - installed ``rose-suite.conf``. + Tuple - (cli_config, rose_suite_conf) + + cli_config: + The Cylc install config aka "rose-suite-cylc-install.conf". + rose_suite_conf: + The "opts" section of the config node dumped to + installed ``rose-suite.conf``. + """ # Create a config based on command line options: - cli_config = get_cli_opts_node(opts, srcdir) + cli_config = get_cli_opts_node(srcdir, opts) # raise error if CLI config has multiple templating sections identify_templating_section(cli_config) diff --git a/tests/unit/test_config_tree.py b/tests/unit/test_config_tree.py index 497b6ddd..86c1d464 100644 --- a/tests/unit/test_config_tree.py +++ b/tests/unit/test_config_tree.py @@ -16,6 +16,7 @@ """Tests the plugin with Rose suite configurations on the filesystem.""" from io import StringIO +from pathlib import Path from types import SimpleNamespace from cylc.flow.hostuserutil import get_host @@ -501,7 +502,7 @@ def test_get_cli_opts_node(opt_confs, defines, rose_template_vars, expect): ) loader = ConfigLoader() expect = loader.load(StringIO(expect)) - result = get_cli_opts_node(opts) + result = get_cli_opts_node(Path('no/such/dir'), opts) for item in ['env', 'template variables', 'opts']: assert result[item] == expect[item] diff --git a/tests/unit/test_functional_post_install.py b/tests/unit/test_functional_post_install.py index 9371714a..fd8f6fba 100644 --- a/tests/unit/test_functional_post_install.py +++ b/tests/unit/test_functional_post_install.py @@ -60,7 +60,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(tmp_path, tmp_path, SimpleNamespace()) assert result is False @@ -82,7 +82,7 @@ def test_rose_fileinstall_uses_rose_template_vars(tmp_path): ) # Run both record_cylc_install options and fileinstall. - record_cylc_install_options(opts=opts, rundir=destdir) + record_cylc_install_options(srcdir, destdir, opts) rose_fileinstall(destdir, opts) assert ((destdir / 'installedme').read_text() == 'Galileo No! We will not let you go.' @@ -248,11 +248,7 @@ def fake(*arg, **kwargs): loader = ConfigLoader() # Run the entry point top-level function: - rose_suite_cylc_install_node, rose_suite_opts_node = ( - record_cylc_install_options( - rundir=testdir, opts=opts, srcdir=testdir - ) - ) + record_cylc_install_options(testdir, testdir, opts=opts) rose_fileinstall(testdir, opts) ritems = sorted([i.relative_to(refdir) for i in refdir.rglob('*')]) titems = sorted([i.relative_to(testdir) for i in testdir.rglob('*')]) @@ -321,11 +317,7 @@ def test_template_section_conflict( with pytest.raises(MultipleTemplatingEnginesError) as exc_info: # Run the entry point top-level function: - rose_suite_cylc_install_node, rose_suite_opts_node = ( - record_cylc_install_options( - rundir=testdir, opts=opts, srcdir=testdir - ) - ) + record_cylc_install_options(testdir, testdir, opts) assert exc_info.match(expect) @@ -356,7 +348,7 @@ 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(tmp_path, tmp_path, SimpleNamespace()) is False def test_copy_config_file_fails(tmp_path): From b993900ed8cc537baeb5df5c3c68349e34c0c7a5 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Wed, 1 Nov 2023 12:08:19 +0000 Subject: [PATCH 18/48] tests: amend docstrings * Amend false docstrings. * Rename a couple of modules. --- cylc/rose/platform_utils.py | 5 +++-- cylc/rose/utilities.py | 4 +--- tests/conftest.py | 2 -- tests/functional/test_ROSE_ORIG_HOST.py | 1 + tests/functional/test_pre_configure.py | 4 ++-- tests/functional/test_reinstall.py | 1 + tests/functional/test_reinstall_clean.py | 1 + tests/functional/test_reinstall_fileinstall.py | 4 +++- tests/functional/test_rose_fileinstall.py | 4 ++-- tests/functional/test_rose_stem.py | 1 + tests/functional/{test_copy_config_file.py => test_utils.py} | 5 ++--- tests/functional/test_validate_no_rose_ignore_items.py | 1 + tests/unit/test_config_node.py | 1 + tests/unit/test_config_tree.py | 1 + tests/unit/{test_rose_opts.py => test_fileinstall.py} | 4 ++-- tests/unit/test_functional_post_install.py | 1 + tests/unit/test_platform_utils.py | 4 ++-- tests/unit/test_rose_stem_units.py | 4 ++-- 18 files changed, 27 insertions(+), 21 deletions(-) rename tests/functional/{test_copy_config_file.py => test_utils.py} (93%) rename tests/unit/{test_rose_opts.py => test_fileinstall.py} (97%) diff --git a/cylc/rose/platform_utils.py b/cylc/rose/platform_utils.py index 5c3d6740..91dd11a7 100644 --- a/cylc/rose/platform_utils.py +++ b/cylc/rose/platform_utils.py @@ -15,8 +15,9 @@ # You should have received a copy of the GNU General Public License # along with Rose. If not, see . # ----------------------------------------------------------------------------- -"""Interfaces for Cylc Platforms for use by rose apps. -""" + +"""Interfaces for Cylc Platforms for use by rose apps.""" + from optparse import Values import sqlite3 import subprocess diff --git a/cylc/rose/utilities.py b/cylc/rose/utilities.py index 15540b21..e23f6e54 100644 --- a/cylc/rose/utilities.py +++ b/cylc/rose/utilities.py @@ -14,9 +14,7 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Cylc support for reading and interpreting ``rose-suite.conf`` workflow -configuration files. -""" +"""Cylc support for reading and interpreting ``rose-suite.conf`` files.""" import itertools import os diff --git a/tests/conftest.py b/tests/conftest.py index 93647bf2..624d7c6a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -13,8 +13,6 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Functional tests for top-level function record_cylc_install_options and -""" from types import SimpleNamespace diff --git a/tests/functional/test_ROSE_ORIG_HOST.py b/tests/functional/test_ROSE_ORIG_HOST.py index df1db67e..73d6fa1a 100644 --- a/tests/functional/test_ROSE_ORIG_HOST.py +++ b/tests/functional/test_ROSE_ORIG_HOST.py @@ -15,6 +15,7 @@ # You should have received a copy of the GNU General Public License # along with Rose. If not, see . # ----------------------------------------------------------------------------- + """ Tests for cylc.rose.stem ======================== diff --git a/tests/functional/test_pre_configure.py b/tests/functional/test_pre_configure.py index 7785735b..00d423b7 100644 --- a/tests/functional/test_pre_configure.py +++ b/tests/functional/test_pre_configure.py @@ -13,8 +13,8 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Functional tests for top-level function record_cylc_install_options and -""" + +"""Test pre_configure entry point.""" from itertools import product import os diff --git a/tests/functional/test_reinstall.py b/tests/functional/test_reinstall.py index 4190a092..65002f9a 100644 --- a/tests/functional/test_reinstall.py +++ b/tests/functional/test_reinstall.py @@ -13,6 +13,7 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . + """Functional tests for reinstalling of config files. This test does the following: diff --git a/tests/functional/test_reinstall_clean.py b/tests/functional/test_reinstall_clean.py index d6a332f0..749778c4 100644 --- a/tests/functional/test_reinstall_clean.py +++ b/tests/functional/test_reinstall_clean.py @@ -13,6 +13,7 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . + """Functional tests for reinstalling of config files. This test does the following: diff --git a/tests/functional/test_reinstall_fileinstall.py b/tests/functional/test_reinstall_fileinstall.py index 70ca41e8..817500c6 100644 --- a/tests/functional/test_reinstall_fileinstall.py +++ b/tests/functional/test_reinstall_fileinstall.py @@ -13,7 +13,9 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Cylc reinstall is able to use the async fileinstall from rose without + +""" +Ensure Cylc reinstall is able to use the async fileinstall from rose without trouble. """ diff --git a/tests/functional/test_rose_fileinstall.py b/tests/functional/test_rose_fileinstall.py index 5998b778..d0bca3b3 100644 --- a/tests/functional/test_rose_fileinstall.py +++ b/tests/functional/test_rose_fileinstall.py @@ -13,8 +13,8 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Functional tests for top-level function record_cylc_install_options and -""" + +"""Functional tests for Rose file installation.""" from pathlib import Path import shutil diff --git a/tests/functional/test_rose_stem.py b/tests/functional/test_rose_stem.py index 743455e2..4d203196 100644 --- a/tests/functional/test_rose_stem.py +++ b/tests/functional/test_rose_stem.py @@ -17,6 +17,7 @@ # You should have received a copy of the GNU General Public License # along with Rose. If not, see . # ----------------------------------------------------------------------------- + """ Tests for cylc.rose.stem ======================== diff --git a/tests/functional/test_copy_config_file.py b/tests/functional/test_utils.py similarity index 93% rename from tests/functional/test_copy_config_file.py rename to tests/functional/test_utils.py index caed1a13..f2b9ef07 100644 --- a/tests/functional/test_copy_config_file.py +++ b/tests/functional/test_utils.py @@ -13,9 +13,8 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Functional tests for top-level function cylc.rose.entry points -copy_config_file. -""" + +"""Unit tests for utilities.""" from pathlib import Path diff --git a/tests/functional/test_validate_no_rose_ignore_items.py b/tests/functional/test_validate_no_rose_ignore_items.py index 5f79b194..99f48318 100644 --- a/tests/functional/test_validate_no_rose_ignore_items.py +++ b/tests/functional/test_validate_no_rose_ignore_items.py @@ -13,6 +13,7 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . + """Functional Test: It ignores commented items in rose-suite.conf See https://github.com/cylc/cylc-rose/pull/171 diff --git a/tests/unit/test_config_node.py b/tests/unit/test_config_node.py index 8c697dad..8254dc15 100644 --- a/tests/unit/test_config_node.py +++ b/tests/unit/test_config_node.py @@ -13,6 +13,7 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . + """Tests the plugin with Rose suite configurations via the Python API.""" from types import SimpleNamespace diff --git a/tests/unit/test_config_tree.py b/tests/unit/test_config_tree.py index 86c1d464..a622aa5d 100644 --- a/tests/unit/test_config_tree.py +++ b/tests/unit/test_config_tree.py @@ -13,6 +13,7 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . + """Tests the plugin with Rose suite configurations on the filesystem.""" from io import StringIO diff --git a/tests/unit/test_rose_opts.py b/tests/unit/test_fileinstall.py similarity index 97% rename from tests/unit/test_rose_opts.py rename to tests/unit/test_fileinstall.py index a05658dd..a4f70abf 100644 --- a/tests/unit/test_rose_opts.py +++ b/tests/unit/test_fileinstall.py @@ -13,8 +13,8 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Functional tests for top-level function record_cylc_install_options and -""" + +"""Unit tests for Rose file installation.""" from pathlib import Path import shutil diff --git a/tests/unit/test_functional_post_install.py b/tests/unit/test_functional_post_install.py index fd8f6fba..7ff194ed 100644 --- a/tests/unit/test_functional_post_install.py +++ b/tests/unit/test_functional_post_install.py @@ -13,6 +13,7 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . + """Functional tests for top-level function record_cylc_install_options and rose_fileinstall diff --git a/tests/unit/test_platform_utils.py b/tests/unit/test_platform_utils.py index 04433d03..7ac65594 100644 --- a/tests/unit/test_platform_utils.py +++ b/tests/unit/test_platform_utils.py @@ -15,8 +15,8 @@ # You should have received a copy of the GNU General Public License # along with Rose. If not, see . # ----------------------------------------------------------------------------- -"""Tests for platform utils module: -""" + +"""Tests for platform utils module.""" import os from pathlib import Path diff --git a/tests/unit/test_rose_stem_units.py b/tests/unit/test_rose_stem_units.py index 72b7fbbd..5128030f 100644 --- a/tests/unit/test_rose_stem_units.py +++ b/tests/unit/test_rose_stem_units.py @@ -13,8 +13,8 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Functional tests for top-level function record_cylc_install_options and -""" + +"""Unit tests for Rose Stem.""" from types import SimpleNamespace from typing import Any, Tuple From 8169941e06eb994a84904f37bd5863f040f6bdac Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Wed, 8 Nov 2023 09:59:27 +0000 Subject: [PATCH 19/48] feedback --- cylc/rose/entry_points.py | 9 --------- cylc/rose/fileinstall.py | 9 +-------- 2 files changed, 1 insertion(+), 17 deletions(-) diff --git a/cylc/rose/entry_points.py b/cylc/rose/entry_points.py index 98b6de37..9f9b5d54 100644 --- a/cylc/rose/entry_points.py +++ b/cylc/rose/entry_points.py @@ -34,15 +34,6 @@ def pre_configure(srcdir: Path, opts: 'Values') -> dict: """Run before the Cylc configuration is read.""" - if not srcdir: - # not sure how this could happen - return { - # default return value - 'env': {}, - 'template_variables': {}, - 'templating_detected': None - } - # load the Rose config config_tree = load_rose_config(Path(srcdir), opts=opts) diff --git a/cylc/rose/fileinstall.py b/cylc/rose/fileinstall.py index 986ab05f..d91d32a3 100644 --- a/cylc/rose/fileinstall.py +++ b/cylc/rose/fileinstall.py @@ -31,14 +31,7 @@ def rose_fileinstall( rundir: 'Path', opts: 'Values', ) -> 'Union[ConfigNode, bool]': - """Call Rose Fileinstall. - - Args: - srcdir(pathlib.Path): - Search for a ``rose-suite.conf`` file in this location. - rundir (pathlib.Path) - - """ + """Call Rose Fileinstall.""" if not rose_config_exists(rundir): return False From f09bdf9c6af7a0a16787363c08ca70d1e32936d9 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Wed, 22 Nov 2023 11:07:08 +0000 Subject: [PATCH 20/48] Exit immediately if folder pre-configure is called on does not have a rose-suite.conf --- 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 9f9b5d54..f749cd56 100644 --- a/cylc/rose/entry_points.py +++ b/cylc/rose/entry_points.py @@ -34,6 +34,10 @@ def pre_configure(srcdir: Path, opts: 'Values') -> dict: """Run before the Cylc configuration is read.""" + if not rose_config_exists(srcdir): + # nothing to do here + return {} + # load the Rose config config_tree = load_rose_config(Path(srcdir), opts=opts) @@ -53,6 +57,7 @@ def post_install(srcdir: Path, rundir: str, opts: 'Values') -> bool: if not rose_config_exists(srcdir): # nothing to do here return False + _rundir: Path = Path(rundir) # transfer the rose-suite.conf file From 6b40aaa324cedff59b9d2b638f91f450d03389ac Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 13 Dec 2023 12:14:43 +0000 Subject: [PATCH 21/48] Bump actions/setup-python from 4 to 5 (#277) --- .github/workflows/1_create_release_pr.yml | 2 +- .github/workflows/2_auto_publish_release.yml | 2 +- .github/workflows/tests.yml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/1_create_release_pr.yml b/.github/workflows/1_create_release_pr.yml index a60b92ec..ebc5ed62 100644 --- a/.github/workflows/1_create_release_pr.yml +++ b/.github/workflows/1_create_release_pr.yml @@ -36,7 +36,7 @@ jobs: uses: cylc/release-actions/check-shortlog@v1 - name: Setup Python - uses: actions/setup-python@v4 + uses: actions/setup-python@v5 with: python-version: '3.x' diff --git a/.github/workflows/2_auto_publish_release.yml b/.github/workflows/2_auto_publish_release.yml index bd9b6d3b..c4156af6 100644 --- a/.github/workflows/2_auto_publish_release.yml +++ b/.github/workflows/2_auto_publish_release.yml @@ -26,7 +26,7 @@ jobs: ref: ${{ env.MERGE_SHA }} - name: Setup Python - uses: actions/setup-python@v4 + uses: actions/setup-python@v5 with: python-version: '3.x' diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 48a4a45b..ec9ae2d1 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -22,7 +22,7 @@ jobs: uses: actions/checkout@v4 - name: Configure Python - uses: actions/setup-python@v4 + uses: actions/setup-python@v5 with: python-version: ${{ matrix.python-version }} From ed4d2e5a957e22d1f03bfc6eaf32c0db7ba8ef48 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 18 Dec 2023 12:11:36 +0000 Subject: [PATCH 22/48] Bump pypa/gh-action-pypi-publish from 1.8.10 to 1.8.11 (#272) --- .github/workflows/2_auto_publish_release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/2_auto_publish_release.yml b/.github/workflows/2_auto_publish_release.yml index c4156af6..4b3ca585 100644 --- a/.github/workflows/2_auto_publish_release.yml +++ b/.github/workflows/2_auto_publish_release.yml @@ -37,7 +37,7 @@ jobs: uses: cylc/release-actions/build-python-package@v1 - name: Publish distribution to PyPI - uses: pypa/gh-action-pypi-publish@v1.8.10 + uses: pypa/gh-action-pypi-publish@v1.8.11 with: user: __token__ # uses the API token feature of PyPI - least permissions possible password: ${{ secrets.PYPI_TOKEN }} From 7d1e505f8ee3b349846405fb454f044fb9f60c70 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 25 Dec 2023 21:08:35 +0000 Subject: [PATCH 23/48] Bump actions/download-artifact from 3 to 4 Bumps [actions/download-artifact](https://github.com/actions/download-artifact) from 3 to 4. - [Release notes](https://github.com/actions/download-artifact/releases) - [Commits](https://github.com/actions/download-artifact/compare/v3...v4) --- updated-dependencies: - dependency-name: actions/download-artifact dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 591b05b9..2e3dec74 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -142,7 +142,7 @@ jobs: uses: actions/checkout@v4 - name: Download coverage artifacts - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 - name: Codecov upload uses: codecov/codecov-action@v3 From 284739756aecd91c834446a33f9366c34411f2a0 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 25 Dec 2023 21:08:40 +0000 Subject: [PATCH 24/48] Bump actions/upload-artifact from 3 to 4 Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 3 to 4. - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](https://github.com/actions/upload-artifact/compare/v3...v4) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 591b05b9..10a85e36 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -97,7 +97,7 @@ jobs: coverage report - name: Upload coverage artifact - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: coverage_py-${{ matrix.python-version }} path: coverage.xml From 3573f5a7075a6ceb7295969cab8bca30709a4881 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 1 Jan 2024 03:24:02 +0000 Subject: [PATCH 25/48] Update copyright year Workflow: Update copyright year, run: 4 --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 8b0a1802..d1d92143 100644 --- a/README.md +++ b/README.md @@ -81,7 +81,7 @@ pip install -e cylc-rose[all] [![License](https://img.shields.io/github/license/cylc/cylc-flow.svg?color=lightgrey)](https://github.com/cylc/cylc-flow/blob/master/COPYING) -Copyright (C) 2008-2023 NIWA & +Copyright (C) 2008-2024 NIWA & British Crown (Met Office) & Contributors. Cylc-rose is free software: you can redistribute it and/or modify it under the terms From fa3611b66a23dd3e5eb7ed317f10f2666a757c66 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Mon, 4 Dec 2023 11:07:21 +0000 Subject: [PATCH 26/48] Ensure that the cached globalconfig object is reloaded after the export of `CYLC_SYMLINKS` variable. This means that using the `CYLC_SYMLINKS` variable allows users to specify installation symlink locations in the `rose-suite.conf`. --- CHANGES.md | 8 ++++ cylc/rose/__init__.py | 29 +++++++++++- cylc/rose/utilities.py | 8 ++++ tests/conftest.py | 24 +++++++--- tests/functional/test_utils.py | 81 ++++++++++++++++++++++++++++++++++ 5 files changed, 143 insertions(+), 7 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 27af8c00..dc165e9c 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -6,6 +6,14 @@ creating a new release entry be sure to copy & paste the span tag with the updated. Only the first match gets replaced, so it's fine to leave the old ones in. --> + +## __cylc-rose-1.4.0 (Upcoming)__ + +### Features + +[#269](https://github.com/cylc/cylc-rose/pull/269) - Allow environment variables +set in ``rose-suite.conf`` to be used when parsing ``global.cylc``. + ## __cylc-rose-1.3.1 (Released 2023-10-24)__ ### Fixes diff --git a/cylc/rose/__init__.py b/cylc/rose/__init__.py index 841796a0..b8283291 100644 --- a/cylc/rose/__init__.py +++ b/cylc/rose/__init__.py @@ -70,11 +70,37 @@ for ease of porting Cylc 7 workflows. +The ``global.cylc`` file +^^^^^^^^^^^^^^^^^^^^^^^^ + +The Cylc Rose Plugin forces the reloading of the ``global.cylc`` file +to allow environment variables set by Rose to change the global configuration. + +For example you could use ``CYLC_SYMLINKS`` as a variable to control +the behaviour of ``cylc install``: + + .. code-block:: cylc + + #!jinja2 + # part of a global.cylc file + [install] + [[symlink dirs]] + [[[hpc]]] + {% if environ["CYLC_SYMLINKS"] | default("x") == "A" %} + run = $LOCATION_A + {% elif environ["CYLC_SYMLINKS"] | default("x") == "B" %} + run = $LOCATION_B + {% else %} + run = $LOCATION_C + {% endif %} + + + Special Variables ----------------- The Cylc Rose plugin provides two environment/template variables -to the Cylc scheduler: +to the Cylc scheduler. ``ROSE_ORIG_HOST`` Cylc commands (such as ``cylc install``, ``cylc validate`` and @@ -111,6 +137,7 @@ ``CYLC_VERSION`` will be removed from your configuration by the Cylc-Rose plugin, as it is now set by Cylc. + Additional CLI options ---------------------- You can use command line options to set or override diff --git a/cylc/rose/utilities.py b/cylc/rose/utilities.py index e23f6e54..515356a5 100644 --- a/cylc/rose/utilities.py +++ b/cylc/rose/utilities.py @@ -27,6 +27,7 @@ from cylc.flow import LOG from cylc.flow.exceptions import CylcError from cylc.flow.flags import cylc7_back_compat +from cylc.flow.cfgspec.glbl_cfg import glbl_cfg from cylc.flow.hostuserutil import get_host from metomi.isodatetime.datetimeoper import DateTimeOperator from metomi.rose import __version__ as ROSE_VERSION @@ -869,6 +870,13 @@ def export_environment(environment: Dict[str, str]) -> None: for key, val in environment.items(): os.environ[key] = val + # If env vars have been set we want to force reload + # the global config so that the value of this vars + # can be used by Jinja2 in the global config. + # https://github.com/cylc/cylc-rose/issues/237 + if environment: + glbl_cfg().load() + def record_cylc_install_options( srcdir: Path, diff --git a/tests/conftest.py b/tests/conftest.py index 624d7c6a..d2baf6d7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -15,9 +15,11 @@ # along with this program. If not, see . from types import SimpleNamespace +from uuid import uuid4 from cylc.flow import __version__ as CYLC_VERSION from cylc.flow.option_parsers import Options +from cylc.flow.pathutil import get_workflow_run_dir from cylc.flow.scripts.install import get_option_parser as install_gop from cylc.flow.scripts.install import install_cli as cylc_install from cylc.flow.scripts.reinstall import get_option_parser as reinstall_gop @@ -27,6 +29,11 @@ import pytest +@pytest.fixture +def generate_workflow_name(): + return 'cylc-rose-test-' + str(uuid4())[:8] + + @pytest.fixture(scope='module') def mod_capsys(request): from _pytest.capture import SysCapture @@ -108,7 +115,7 @@ def _inner(srcpath, args=None): return _inner -def _cylc_install_cli(capsys, caplog): +def _cylc_install_cli(capsys, caplog, generate_workflow_name): """Access the install CLI""" def _inner(srcpath, args=None): """Install a workflow. @@ -119,9 +126,13 @@ def _inner(srcpath, args=None): """ options = Options(install_gop(), args)() output = SimpleNamespace() + if not options.workflow_name: + options.workflow_name = generate_workflow_name + if not args or args and not args.get('no_run_name', ''): + options.no_run_name = True try: - cylc_install(options, str(srcpath)) + output.name, output.id = cylc_install(options, str(srcpath)) output.ret = 0 output.exc = '' except Exception as exc: @@ -129,6 +140,7 @@ def _inner(srcpath, args=None): output.exc = exc output.logging = '\n'.join([i.message for i in caplog.records]) output.out, output.err = capsys.readouterr() + output.run_dir = get_workflow_run_dir(output.id) return output return _inner @@ -159,13 +171,13 @@ def _inner(workflow_id, opts=None): @pytest.fixture -def cylc_install_cli(capsys, caplog): - return _cylc_install_cli(capsys, caplog) +def cylc_install_cli(capsys, caplog, generate_workflow_name): + return _cylc_install_cli(capsys, caplog, generate_workflow_name) @pytest.fixture(scope='module') -def mod_cylc_install_cli(mod_capsys, mod_caplog): - return _cylc_install_cli(mod_capsys, mod_caplog) +def mod_cylc_install_cli(mod_capsys, mod_caplog, generate_workflow_name): + return _cylc_install_cli(mod_capsys, mod_caplog, generate_workflow_name) @pytest.fixture diff --git a/tests/functional/test_utils.py b/tests/functional/test_utils.py index f2b9ef07..59486f30 100644 --- a/tests/functional/test_utils.py +++ b/tests/functional/test_utils.py @@ -36,3 +36,84 @@ def test_basic(tmp_path): assert Path(tmp_path / 'src/rose-suite.conf').read_text() == ( Path(tmp_path / 'dest/rose-suite.conf').read_text() ) + + +def test_CYLC_SYMLINKS_validate(monkeypatch, tmp_path, cylc_validate_cli): + """We reload the global config after exporting env variables.""" + # Setup global config: + global_conf = """#!jinja2 + {% from "cylc.flow" import LOG %} + {% set cylc_symlinks = environ.get('CYLC_SYMLINKS', None) %} + {% do LOG.critical(cylc_symlinks) %} + """ + conf_path = tmp_path / 'conf' + conf_path.mkdir() + monkeypatch.setenv('CYLC_CONF_PATH', conf_path) + + # Setup workflow config: + (conf_path / 'global.cylc').write_text(global_conf) + (tmp_path / 'rose-suite.conf').write_text( + '[env]\nCYLC_SYMLINKS="Foo"\n') + (tmp_path / 'flow.cylc').write_text(""" + [scheduling] + initial cycle point = now + [[graph]] + R1 = x + [runtime] + [[x]] + """) + + # Validate the config: + output = cylc_validate_cli(tmp_path) + assert output.ret == 0 + + # CYLC_SYMLINKS == None the first time the global.cylc + # is loaded and "Foo" the second time. + assert output.logging == 'None\n"Foo"' + + +def test_CYLC_SYMLINKS_install(monkeypatch, tmp_path, cylc_install_cli): + """We reload the global config after exporting env variables.""" + # Setup global config: + global_conf = ( + '#!jinja2\n' + '[install]\n' + ' [[symlink dirs]]\n' + ' [[[localhost]]]\n' + '{% set cylc_symlinks = environ.get(\'CYLC_SYMLINKS\', None) %}\n' + '{% if cylc_symlinks == "foo" %}\n' + f'log = {str(tmp_path)}/foo\n' + '{% else %}\n' + f'log = {str(tmp_path)}/bar\n' + '{% endif %}\n' + ) + glbl_conf_path = tmp_path / 'conf' + glbl_conf_path.mkdir() + (glbl_conf_path / 'global.cylc').write_text(global_conf) + monkeypatch.setenv('CYLC_CONF_PATH', glbl_conf_path) + + # Setup workflow config: + (tmp_path / 'rose-suite.conf').write_text( + '[env]\nCYLC_SYMLINKS=foo\n') + (tmp_path / 'flow.cylc').write_text(""" + [scheduling] + initial cycle point = now + [[graph]] + R1 = x + [runtime] + [[x]] + """) + + # Install the config: + output = cylc_install_cli(tmp_path) + import sys + for i in output.logging.split('\n'): + print(i, file=sys.stderr) + assert output.ret == 0 + + # Assert symlink created back to test_path/foo: + expected_msg = ( + f'Symlink created: {output.run_dir}/log -> ' + f'{tmp_path}/foo/cylc-run/{output.id}/log' + ) + assert expected_msg in output.logging.split('\n')[0] From 6ff23bf8e348b1d7184b109081fc7875a51c89d2 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Tue, 2 Jan 2024 10:56:10 +0000 Subject: [PATCH 27/48] Fixed some broken tests. Added module version of gen workflow name fixture. Fix refs to runN or run1. --- tests/conftest.py | 12 +++++++++--- tests/functional/test_ROSE_ORIG_HOST.py | 4 ++-- tests/functional/test_reinstall.py | 12 ++++++------ tests/functional/test_reinstall_clean.py | 4 ++-- tests/functional/test_rose_fileinstall.py | 6 +++--- tests/functional/test_utils.py | 2 +- 6 files changed, 23 insertions(+), 17 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index d2baf6d7..b65bf050 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -29,11 +29,16 @@ import pytest -@pytest.fixture +@pytest.fixture() def generate_workflow_name(): return 'cylc-rose-test-' + str(uuid4())[:8] +@pytest.fixture(scope='module') +def mod_generate_workflow_name(): + return 'cylc-rose-test-' + str(uuid4())[:8] + + @pytest.fixture(scope='module') def mod_capsys(request): from _pytest.capture import SysCapture @@ -176,8 +181,9 @@ def cylc_install_cli(capsys, caplog, generate_workflow_name): @pytest.fixture(scope='module') -def mod_cylc_install_cli(mod_capsys, mod_caplog, generate_workflow_name): - return _cylc_install_cli(mod_capsys, mod_caplog, generate_workflow_name) +def mod_cylc_install_cli(mod_capsys, mod_caplog, mod_generate_workflow_name): + return _cylc_install_cli( + mod_capsys, mod_caplog, mod_generate_workflow_name) @pytest.fixture diff --git a/tests/functional/test_ROSE_ORIG_HOST.py b/tests/functional/test_ROSE_ORIG_HOST.py index 73d6fa1a..d97e221b 100644 --- a/tests/functional/test_ROSE_ORIG_HOST.py +++ b/tests/functional/test_ROSE_ORIG_HOST.py @@ -119,7 +119,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) @@ -142,7 +142,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 65002f9a..32abc432 100644 --- a/tests/functional/test_reinstall.py +++ b/tests/functional/test_reinstall.py @@ -118,14 +118,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' @@ -172,14 +172,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' @@ -230,14 +230,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' diff --git a/tests/functional/test_reinstall_clean.py b/tests/functional/test_reinstall_clean.py index 749778c4..0a207430 100644 --- a/tests/functional/test_reinstall_clean.py +++ b/tests/functional/test_reinstall_clean.py @@ -109,7 +109,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' @@ -163,7 +163,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 d0bca3b3..ac99b3da 100644 --- a/tests/functional/test_rose_fileinstall.py +++ b/tests/functional/test_rose_fileinstall.py @@ -63,7 +63,7 @@ def fixture_install_flow(fixture_provide_flow, request, cylc_install_cli): yield srcpath, datapath, flow_name, result, destpath if not request.session.testsfailed: - shutil.rmtree(destpath) + shutil.rmtree(destpath, ignore_errors=True) def test_rose_fileinstall_validate(fixture_provide_flow, cylc_validate_cli): @@ -84,7 +84,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()) @@ -92,7 +92,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/functional/test_utils.py b/tests/functional/test_utils.py index 59486f30..0b819dc8 100644 --- a/tests/functional/test_utils.py +++ b/tests/functional/test_utils.py @@ -69,7 +69,7 @@ def test_CYLC_SYMLINKS_validate(monkeypatch, tmp_path, cylc_validate_cli): # CYLC_SYMLINKS == None the first time the global.cylc # is loaded and "Foo" the second time. - assert output.logging == 'None\n"Foo"' + assert output.logging == '"Foo"' def test_CYLC_SYMLINKS_install(monkeypatch, tmp_path, cylc_install_cli): From 6bb14bbc8690a3bb100c76a8cda032c726d74813 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Wed, 3 Jan 2024 14:54:11 +0000 Subject: [PATCH 28/48] Update tests/functional/test_utils.py Co-authored-by: Oliver Sanders Update tests/functional/test_utils.py Co-authored-by: Oliver Sanders fix tests --- tests/conftest.py | 16 ++++++++-------- tests/functional/test_utils.py | 16 ++++++++++++---- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index b65bf050..f6c0e9a1 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -30,12 +30,12 @@ @pytest.fixture() -def generate_workflow_name(): +def workflow_name(): return 'cylc-rose-test-' + str(uuid4())[:8] @pytest.fixture(scope='module') -def mod_generate_workflow_name(): +def mod_workflow_name(): return 'cylc-rose-test-' + str(uuid4())[:8] @@ -120,7 +120,7 @@ def _inner(srcpath, args=None): return _inner -def _cylc_install_cli(capsys, caplog, generate_workflow_name): +def _cylc_install_cli(capsys, caplog, workflow_name): """Access the install CLI""" def _inner(srcpath, args=None): """Install a workflow. @@ -132,7 +132,7 @@ def _inner(srcpath, args=None): options = Options(install_gop(), args)() output = SimpleNamespace() if not options.workflow_name: - options.workflow_name = generate_workflow_name + options.workflow_name = workflow_name if not args or args and not args.get('no_run_name', ''): options.no_run_name = True @@ -176,14 +176,14 @@ def _inner(workflow_id, opts=None): @pytest.fixture -def cylc_install_cli(capsys, caplog, generate_workflow_name): - return _cylc_install_cli(capsys, caplog, generate_workflow_name) +def cylc_install_cli(capsys, caplog, workflow_name): + return _cylc_install_cli(capsys, caplog, workflow_name) @pytest.fixture(scope='module') -def mod_cylc_install_cli(mod_capsys, mod_caplog, mod_generate_workflow_name): +def mod_cylc_install_cli(mod_capsys, mod_caplog, mod_workflow_name): return _cylc_install_cli( - mod_capsys, mod_caplog, mod_generate_workflow_name) + mod_capsys, mod_caplog, mod_workflow_name) @pytest.fixture diff --git a/tests/functional/test_utils.py b/tests/functional/test_utils.py index 0b819dc8..3a303026 100644 --- a/tests/functional/test_utils.py +++ b/tests/functional/test_utils.py @@ -38,8 +38,11 @@ def test_basic(tmp_path): ) -def test_CYLC_SYMLINKS_validate(monkeypatch, tmp_path, cylc_validate_cli): - """We reload the global config after exporting env variables.""" +def test_global_config_environment_validate(monkeypatch, tmp_path, cylc_validate_cli): + """It should reload the global config after exporting env variables. + + See: https://github.com/cylc/cylc-rose/issues/237 + """ # Setup global config: global_conf = """#!jinja2 {% from "cylc.flow" import LOG %} @@ -72,8 +75,13 @@ def test_CYLC_SYMLINKS_validate(monkeypatch, tmp_path, cylc_validate_cli): assert output.logging == '"Foo"' -def test_CYLC_SYMLINKS_install(monkeypatch, tmp_path, cylc_install_cli): - """We reload the global config after exporting env variables.""" +def test_global_config_environment_validate2( + monkeypatch, tmp_path, cylc_install_cli +): + """It should reload the global config after exporting env variables. + + See: https://github.com/cylc/cylc-rose/issues/237 + """ # Setup global config: global_conf = ( '#!jinja2\n' From 69d4a815a3ee88a2200e1189f5e5d9f48904bd97 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 15 Jan 2024 12:14:00 +0000 Subject: [PATCH 29/48] Bump actions/setup-python from 4 to 5 (#280) Bumps [actions/setup-python](https://github.com/actions/setup-python) from 4 to 5. - [Release notes](https://github.com/actions/setup-python/releases) - [Commits](https://github.com/actions/setup-python/compare/v4...v5) --- updated-dependencies: - dependency-name: actions/setup-python dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 591b05b9..289cd4c6 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -111,7 +111,7 @@ jobs: uses: actions/checkout@v4 - name: Configure Python - uses: actions/setup-python@v4 + uses: actions/setup-python@v5 with: python-version: '3.9' From 9c20c28376ccb879bafaa86280d5fec8f5add4a0 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Mon, 15 Jan 2024 13:00:15 +0000 Subject: [PATCH 30/48] Apply suggestions from code review Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> --- cylc/rose/__init__.py | 28 ++++++++++++++-------------- tests/conftest.py | 2 +- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/cylc/rose/__init__.py b/cylc/rose/__init__.py index b8283291..7c7c1750 100644 --- a/cylc/rose/__init__.py +++ b/cylc/rose/__init__.py @@ -79,20 +79,20 @@ For example you could use ``CYLC_SYMLINKS`` as a variable to control the behaviour of ``cylc install``: - .. code-block:: cylc - - #!jinja2 - # part of a global.cylc file - [install] - [[symlink dirs]] - [[[hpc]]] - {% if environ["CYLC_SYMLINKS"] | default("x") == "A" %} - run = $LOCATION_A - {% elif environ["CYLC_SYMLINKS"] | default("x") == "B" %} - run = $LOCATION_B - {% else %} - run = $LOCATION_C - {% endif %} +.. code-block:: cylc + + #!jinja2 + # part of a global.cylc file + [install] + [[symlink dirs]] + [[[hpc]]] + {% if environ["CYLC_SYMLINKS"] | default("x") == "A" %} + run = $LOCATION_A + {% elif environ["CYLC_SYMLINKS"] | default("x") == "B" %} + run = $LOCATION_B + {% else %} + run = $LOCATION_C + {% endif %} diff --git a/tests/conftest.py b/tests/conftest.py index f6c0e9a1..5905240a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -133,7 +133,7 @@ def _inner(srcpath, args=None): output = SimpleNamespace() if not options.workflow_name: options.workflow_name = workflow_name - if not args or args and not args.get('no_run_name', ''): + if not args or not args.get('no_run_name', ''): options.no_run_name = True try: From b3d561c9cd2692ccf0c3876307e9f0be9319a47e Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Mon, 15 Jan 2024 13:08:27 +0000 Subject: [PATCH 31/48] fixed a test --- tests/functional/test_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/test_utils.py b/tests/functional/test_utils.py index 3a303026..157bd1c8 100644 --- a/tests/functional/test_utils.py +++ b/tests/functional/test_utils.py @@ -72,7 +72,7 @@ def test_global_config_environment_validate(monkeypatch, tmp_path, cylc_validate # CYLC_SYMLINKS == None the first time the global.cylc # is loaded and "Foo" the second time. - assert output.logging == '"Foo"' + assert output.logging.split('\n')[-1] == '"Foo"' def test_global_config_environment_validate2( From c452ba48b9d36caab066a3a97d7f7894077619ec Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Mon, 15 Jan 2024 13:13:17 +0000 Subject: [PATCH 32/48] fix flake8 --- tests/functional/test_utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/functional/test_utils.py b/tests/functional/test_utils.py index 157bd1c8..3642fda9 100644 --- a/tests/functional/test_utils.py +++ b/tests/functional/test_utils.py @@ -38,7 +38,9 @@ def test_basic(tmp_path): ) -def test_global_config_environment_validate(monkeypatch, tmp_path, cylc_validate_cli): +def test_global_config_environment_validate( + monkeypatch, tmp_path, cylc_validate_cli +): """It should reload the global config after exporting env variables. See: https://github.com/cylc/cylc-rose/issues/237 From 9664060006214465604c310ec5ecee22b461a4dd Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Thu, 25 Jan 2024 16:29:17 +0000 Subject: [PATCH 33/48] fix merge mistake --- tests/functional/test_rose_stem.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/functional/test_rose_stem.py b/tests/functional/test_rose_stem.py index 7e046863..cdfed64b 100644 --- a/tests/functional/test_rose_stem.py +++ b/tests/functional/test_rose_stem.py @@ -70,16 +70,12 @@ import shutil import subprocess from io import StringIO -from pathlib import Path -from shlex import split from types import SimpleNamespace from uuid import uuid4 from unittest.mock import MagicMock from cylc.flow.hostuserutil import get_host -from cylc.rose.stem import ( - RoseStemVersionException, rose_stem, _get_rose_stem_opts) from metomi.rose.config import ConfigLoader from cylc.flow.pathutil import get_workflow_run_dir From 8b6177671218070dc9b6b852f56116bd57dba5dd Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Fri, 26 Jan 2024 10:59:36 +0000 Subject: [PATCH 34/48] setup: bump rose version --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index 67020f46..0beb5202 100644 --- a/setup.cfg +++ b/setup.cfg @@ -55,7 +55,7 @@ packages = find_namespace: python_requires = >=3.7 include_package_data = True install_requires = - metomi-rose>=2.1.0, <2.3.0 + metomi-rose>=2.3.* cylc-flow==8.3.* metomi-isodatetime ansimarkup From 54cf0e027cd343589f3192d1686e51ce38d050a8 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Fri, 26 Jan 2024 11:43:20 +0000 Subject: [PATCH 35/48] Update setup.cfg --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index 0beb5202..4041cd8c 100644 --- a/setup.cfg +++ b/setup.cfg @@ -55,7 +55,7 @@ packages = find_namespace: python_requires = >=3.7 include_package_data = True install_requires = - metomi-rose>=2.3.* + metomi-rose==2.3.* cylc-flow==8.3.* metomi-isodatetime ansimarkup From 05160640787fa4f185ff60b6c6f253802241b57a Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Mon, 5 Feb 2024 16:05:21 +0000 Subject: [PATCH 36/48] use fqdn in rose-stem tests (#294) --- tests/functional/test_rose_stem.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/functional/test_rose_stem.py b/tests/functional/test_rose_stem.py index 915b122c..85a5a5a9 100644 --- a/tests/functional/test_rose_stem.py +++ b/tests/functional/test_rose_stem.py @@ -73,10 +73,9 @@ from types import SimpleNamespace from uuid import uuid4 -from cylc.flow.hostuserutil import get_host - from metomi.rose.config import ConfigLoader +from metomi.rose.host_select import HostSelector from cylc.flow.pathutil import get_workflow_run_dir from metomi.rose.resource import ResourceLocator import pytest @@ -87,7 +86,12 @@ rose_stem, ) -HOST = get_host().split('.')[0] + +# We want to test Rose-Stem's insertion of the hostname, +# not Rose's method of getting the hostname, so it doesn't +# Matter that we are using the same host selector here as +# in the module under test: +HOST = HostSelector().get_local_host() class SubprocessesError(Exception): From 344fbdffd4a29e5129a5ce5be970504120a6efef Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 6 Feb 2024 13:45:34 +0000 Subject: [PATCH 37/48] Bump codecov/codecov-action from 3 to 4 (#295) --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 043bb321..41cf8918 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -145,7 +145,7 @@ jobs: uses: actions/download-artifact@v4 - name: Codecov upload - uses: codecov/codecov-action@v3 + uses: codecov/codecov-action@v4 with: name: ${{ github.workflow }} fail_ci_if_error: true From f28922b35d7da7aa3ff471fc878bacf85a558c2c Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Wed, 6 Dec 2023 15:21:58 +0000 Subject: [PATCH 38/48] fileinstall: remove asyncio logic * Rose will now take over the role of managing its event loop. * Addresses https://github.com/cylc/cylc-rose/issues/274 --- cylc/rose/fileinstall.py | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/cylc/rose/fileinstall.py b/cylc/rose/fileinstall.py index d91d32a3..c69e7ac1 100644 --- a/cylc/rose/fileinstall.py +++ b/cylc/rose/fileinstall.py @@ -39,6 +39,7 @@ def rose_fileinstall( config_tree = rose_config_tree_loader(rundir, opts) if any(i.startswith('file') for i in config_tree.node.value): + startpoint = None try: startpoint = os.getcwd() os.chdir(rundir) @@ -46,8 +47,6 @@ def rose_fileinstall( raise exc else: # Carry out imports. - import asyncio - from metomi.rose.config_processor import ConfigProcessorsManager from metomi.rose.fs_util import FileSystemUtil from metomi.rose.popen import RosePopener @@ -64,19 +63,11 @@ def rose_fileinstall( fs_util = FileSystemUtil(event_handler) popen = RosePopener(event_handler) - # Get an Asyncio loop if one doesn't exist: - # Rose may need an event loop to invoke async interfaces, - # doing this here incase we want to go async in cylc-rose. - # See https://github.com/cylc/cylc-rose/pull/130/files - try: - asyncio.get_event_loop() - except RuntimeError: - asyncio.set_event_loop(asyncio.new_event_loop()) - # Process fileinstall. config_pm = ConfigProcessorsManager(event_handler, popen, fs_util) config_pm(config_tree, "file") finally: - os.chdir(startpoint) + if startpoint: + os.chdir(startpoint) return config_tree.node From 01707cc3a62c8b3610ff1979933275748ee13b07 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Thu, 22 Feb 2024 12:49:13 +0000 Subject: [PATCH 39/48] tests: fix flaky text * Jinja2 patching wasn't reverted in the event of error. * This isn't an issue for cylc/cylc-rose usage (as we exit on error). * But in the tests, we may continue on error so need to ensure this cleanup logic gets called. --- cylc/rose/jinja2_parser.py | 51 +++++++++++++++++++------------------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/cylc/rose/jinja2_parser.py b/cylc/rose/jinja2_parser.py index aa0528e7..3d52f117 100644 --- a/cylc/rose/jinja2_parser.py +++ b/cylc/rose/jinja2_parser.py @@ -154,35 +154,36 @@ def patch_jinja2_leading_zeros(): jinja2.lexer.Lexer.wrap = _lexer_wrap(jinja2.lexer.Lexer.wrap) # execute the body of the "with" statement - yield - - # report any usage of deprecated syntax - if jinja2.lexer.Lexer.wrap._instances: - num_examples = 5 - LOG.warning( - 'Support for integers with leading zeros (including' - ' lists of integers) was dropped in Jinja2 v3.' - ' Rose will extend support until a future version.' - '\nPlease amend your Rose configuration files,' - ' which currently contain:' - '\n * ' - + ( - '\n * '.join( - f'{before} => {_strip_leading_zeros(before)}' - for before in list( - jinja2.lexer.Lexer.wrap._instances - )[:num_examples] + try: + yield + finally: + # report any usage of deprecated syntax + if jinja2.lexer.Lexer.wrap._instances: + num_examples = 5 + LOG.warning( + 'Support for integers with leading zeros (including' + ' lists of integers) was dropped in Jinja2 v3.' + ' Rose will extend support until a future version.' + '\nPlease amend your Rose configuration files,' + ' which currently contain:' + '\n * ' + + ( + '\n * '.join( + f'{before} => {_strip_leading_zeros(before)}' + for before in list( + jinja2.lexer.Lexer.wrap._instances + )[:num_examples] + ) ) - ) - ) + ) - # revert the code patch - jinja2.lexer.integer_re = _integer_re - jinja2.lexer.Lexer.wrap = jinja2.lexer.Lexer.wrap.__wrapped__ + # revert the code patch + jinja2.lexer.integer_re = _integer_re + jinja2.lexer.Lexer.wrap = jinja2.lexer.Lexer.wrap.__wrapped__ - # clear any patched lexers to return Jinja2 to normal operation - jinja2.lexer._lexer_cache.clear() + # clear any patched lexers to return Jinja2 to normal operation + jinja2.lexer._lexer_cache.clear() class Parser(NativeEnvironment): From 0479c9c11ac462367a723eea06163bfe4f0fa318 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Wed, 3 Jan 2024 11:07:54 +0000 Subject: [PATCH 40/48] install/reinstall: adapt to new async interfaces * The Cylc install and reinstall interfaces are now async. * This adapts rose-stem to handle the change and adjusts the tests. --- cylc/rose/entry_points.py | 7 +- cylc/rose/fileinstall.py | 6 +- cylc/rose/stem.py | 4 +- tests/conftest.py | 175 +++++++++++++++--- tests/functional/test_ROSE_ORIG_HOST.py | 20 +- tests/functional/test_pre_configure.py | 8 +- tests/functional/test_reinstall.py | 64 ++++--- tests/functional/test_reinstall_clean.py | 25 ++- .../functional/test_reinstall_fileinstall.py | 15 +- tests/functional/test_rose_fileinstall.py | 113 ++++++----- tests/functional/test_rose_stem.py | 53 +++--- tests/functional/test_utils.py | 38 ++-- tests/unit/test_fileinstall.py | 16 +- 13 files changed, 352 insertions(+), 192 deletions(-) diff --git a/cylc/rose/entry_points.py b/cylc/rose/entry_points.py index f749cd56..5c6f1e87 100644 --- a/cylc/rose/entry_points.py +++ b/cylc/rose/entry_points.py @@ -76,7 +76,10 @@ def post_install(srcdir: Path, rundir: str, opts: 'Values') -> bool: def rose_stem(): """Implements the "rose stem" command.""" - from cylc.rose.stem import get_rose_stem_opts + import asyncio + from cylc.rose.stem import get_rose_stem_opts, rose_stem parser, opts = get_rose_stem_opts() - rose_stem(parser, opts) + asyncio.run( + rose_stem(parser, opts) + ) diff --git a/cylc/rose/fileinstall.py b/cylc/rose/fileinstall.py index c69e7ac1..963ba6c9 100644 --- a/cylc/rose/fileinstall.py +++ b/cylc/rose/fileinstall.py @@ -39,9 +39,8 @@ def rose_fileinstall( config_tree = rose_config_tree_loader(rundir, opts) if any(i.startswith('file') for i in config_tree.node.value): - startpoint = None try: - startpoint = os.getcwd() + # NOTE: Cylc will chdir back for us afterwards os.chdir(rundir) except FileNotFoundError as exc: raise exc @@ -66,8 +65,5 @@ def rose_fileinstall( # Process fileinstall. config_pm = ConfigProcessorsManager(event_handler, popen, fs_util) config_pm(config_tree, "file") - finally: - if startpoint: - os.chdir(startpoint) return config_tree.node diff --git a/cylc/rose/stem.py b/cylc/rose/stem.py index 2a556cc0..5972f4cf 100644 --- a/cylc/rose/stem.py +++ b/cylc/rose/stem.py @@ -609,13 +609,13 @@ def get_rose_stem_opts(): return parser, opts -def rose_stem(parser, opts): +async def rose_stem(parser, opts): try: # modify the CLI options to add whatever rose stem would like to add opts = StemRunner(opts).process() # call cylc install - cylc_install(opts, opts.workflow_conf_dir) + await cylc_install(opts, opts.workflow_conf_dir) except CylcError as exc: if opts.verbosity > 1: diff --git a/tests/conftest.py b/tests/conftest.py index 5905240a..731e285d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -14,19 +14,52 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . +import asyncio +from pathlib import Path +from shutil import rmtree from types import SimpleNamespace from uuid import uuid4 +import pytest + from cylc.flow import __version__ as CYLC_VERSION from cylc.flow.option_parsers import Options -from cylc.flow.pathutil import get_workflow_run_dir +from cylc.flow.pathutil import get_cylc_run_dir from cylc.flow.scripts.install import get_option_parser as install_gop from cylc.flow.scripts.install import install_cli as cylc_install from cylc.flow.scripts.reinstall import get_option_parser as reinstall_gop from cylc.flow.scripts.reinstall import reinstall_cli as cylc_reinstall -from cylc.flow.scripts.validate import _main as cylc_validate +from cylc.flow.scripts.validate import run as cylc_validate from cylc.flow.scripts.validate import get_option_parser as validate_gop -import pytest +from cylc.flow.wallclock import get_current_time_string + + +CYLC_RUN_DIR = Path(get_cylc_run_dir()) + + +@pytest.fixture(scope='module') +def event_loop(): + """This fixture defines the event loop used for each test. + + The default scoping for this fixture is "function" which means that all + async fixtures must have "function" scoping. + + Defining `event_loop` as a module scoped fixture opens the door to + module scoped fixtures but means all tests in a module will run in the same + event loop. This is fine, it's actually an efficiency win but also + something to be aware of. + + See: https://github.com/pytest-dev/pytest-asyncio/issues/171 + + """ + loop = asyncio.get_event_loop_policy().new_event_loop() + yield loop + # gracefully exit async generators + loop.run_until_complete(loop.shutdown_asyncgens()) + # cancel any tasks still running in this event loop + for task in asyncio.all_tasks(loop): + task.cancel() + loop.close() @pytest.fixture() @@ -98,15 +131,34 @@ def pytest_runtest_makereport(item, call): item.module._module_outcomes = _module_outcomes +def _rm_if_empty(path): + """Convenience wrapper for removing empty directories.""" + try: + path.rmdir() + except OSError: + return False + return True + + +def _pytest_passed(request: pytest.FixtureRequest) -> bool: + """Returns True if the test(s) a fixture was used in passed.""" + if hasattr(request.node, '_function_outcome'): + return request.node._function_outcome.outcome in {'passed', 'skipped'} + return all(( + report.outcome in {'passed', 'skipped'} + for report in request.node.obj._module_outcomes.values() + )) + + def _cylc_validate_cli(capsys, caplog): """Access the validate CLI""" - def _inner(srcpath, args=None): + async def _inner(srcpath, args=None): parser = validate_gop() options = Options(parser, args)() output = SimpleNamespace() try: - cylc_validate(parser, options, str(srcpath)) + await cylc_validate(parser, options, str(srcpath)) output.ret = 0 output.exc = '' except Exception as exc: @@ -120,24 +172,39 @@ def _inner(srcpath, args=None): return _inner -def _cylc_install_cli(capsys, caplog, workflow_name): +def _cylc_install_cli(capsys, caplog, test_dir): """Access the install CLI""" - def _inner(srcpath, args=None): + async def _inner(srcpath, workflow_name=None, opts=None): """Install a workflow. Args: srcpath: - args: Dictionary of arguments. + The workflow to install + workflow_name: + The workflow ID prefix to install this workflow as. + + If you leave this blank, it will use the module/function's + test directory as appropriate. + opts: + Dictionary of arguments for cylc install. + """ - options = Options(install_gop(), args)() + nonlocal capsys, caplog, test_dir + if not workflow_name: + workflow_name = str( + (test_dir / str(uuid4())[:4]).relative_to(CYLC_RUN_DIR) + ) + options = Options( + install_gop(), opts or {} + )(workflow_name=workflow_name) output = SimpleNamespace() if not options.workflow_name: options.workflow_name = workflow_name - if not args or not args.get('no_run_name', ''): + if not opts or not opts.get('no_run_name', ''): options.no_run_name = True try: - output.name, output.id = cylc_install(options, str(srcpath)) + output.name, output.id = await cylc_install(options, str(srcpath)) output.ret = 0 output.exc = '' except Exception as exc: @@ -145,28 +212,37 @@ def _inner(srcpath, args=None): output.exc = exc output.logging = '\n'.join([i.message for i in caplog.records]) output.out, output.err = capsys.readouterr() - output.run_dir = get_workflow_run_dir(output.id) return output return _inner -def _cylc_reinstall_cli(capsys, caplog): +def _cylc_reinstall_cli(capsys, caplog, test_dir): """Access the reinstall CLI""" - def _inner(workflow_id, opts=None): + async def _inner(workflow_id=None, opts=None): """Install a workflow. Args: - srcpath: - args: Dictionary of arguments. + workflow_id: + The workflow ID to reinstall. + + If you leave this blank, it will use the module/function's + test directory as appropriate. + args: + Dictionary of arguments for cylc reinstall. + """ - options = Options(reinstall_gop(), opts)() + nonlocal capsys, caplog, test_dir + if not workflow_id: + workflow_id = str(test_dir.relative_to(CYLC_RUN_DIR)) + options = Options(reinstall_gop(), opts or {})() output = SimpleNamespace() try: - cylc_reinstall(options, workflow_id) + await cylc_reinstall(options, workflow_id) output.ret = 0 output.exc = '' except Exception as exc: + # raise output.ret = 1 output.exc = exc output.logging = '\n'.join([i.message for i in caplog.records]) @@ -176,24 +252,23 @@ def _inner(workflow_id, opts=None): @pytest.fixture -def cylc_install_cli(capsys, caplog, workflow_name): - return _cylc_install_cli(capsys, caplog, workflow_name) +def cylc_install_cli(capsys, caplog, test_dir): + return _cylc_install_cli(capsys, caplog, test_dir) @pytest.fixture(scope='module') -def mod_cylc_install_cli(mod_capsys, mod_caplog, mod_workflow_name): - return _cylc_install_cli( - mod_capsys, mod_caplog, mod_workflow_name) +def mod_cylc_install_cli(mod_capsys, mod_caplog): + return _cylc_install_cli(mod_capsys, mod_caplog, mod_test_dir) @pytest.fixture -def cylc_reinstall_cli(capsys, caplog): - return _cylc_reinstall_cli(capsys, caplog) +def cylc_reinstall_cli(capsys, caplog, test_dir): + return _cylc_reinstall_cli(capsys, caplog, test_dir) @pytest.fixture(scope='module') -def mod_cylc_reinstall_cli(mod_capsys, mod_caplog): - return _cylc_reinstall_cli(mod_capsys, mod_caplog) +def mod_cylc_reinstall_cli(mod_capsys, mod_caplog, mod_test_dir): + return _cylc_reinstall_cli(mod_capsys, mod_caplog, mod_test_dir) @pytest.fixture @@ -204,3 +279,49 @@ 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(scope='session') +def run_dir(): + """The cylc run directory for this host.""" + CYLC_RUN_DIR.mkdir(exist_ok=True) + yield CYLC_RUN_DIR + + +@pytest.fixture(scope='session') +def ses_test_dir(request, run_dir): + """The root run dir for test flows in this test session.""" + timestamp = get_current_time_string(use_basic_format=True) + uuid = f'cylc-rose-test-{timestamp}-{str(uuid4())[:4]}' + path = Path(run_dir, uuid) + path.mkdir(exist_ok=True) + yield path + _rm_if_empty(path) + + +@pytest.fixture(scope='module') +def mod_test_dir(request, ses_test_dir): + """The root run dir for test flows in this test module.""" + path = Path(ses_test_dir, request.module.__name__) + path.mkdir(exist_ok=True) + yield path + if _pytest_passed(request): + # test passed -> remove all files + rmtree(path, ignore_errors=False) + else: + # test failed -> remove the test dir if empty + _rm_if_empty(path) + + +@pytest.fixture +def test_dir(request, mod_test_dir): + """The root run dir for test flows in this test function.""" + path = Path(mod_test_dir, request.function.__name__) + path.mkdir(parents=True, exist_ok=True) + yield path + if _pytest_passed(request): + # test passed -> remove all files + rmtree(path, ignore_errors=False) + else: + # test failed -> remove the test dir if empty + _rm_if_empty(path) diff --git a/tests/functional/test_ROSE_ORIG_HOST.py b/tests/functional/test_ROSE_ORIG_HOST.py index d97e221b..7f9af1fb 100644 --- a/tests/functional/test_ROSE_ORIG_HOST.py +++ b/tests/functional/test_ROSE_ORIG_HOST.py @@ -102,7 +102,7 @@ def fixture_provide_flow(tmp_path_factory, request): @pytest.fixture(scope='module') -def fixture_install_flow( +async def fixture_install_flow( fixture_provide_flow, monkeymodule, mod_cylc_install_cli ): """Run ``cylc install``. @@ -113,9 +113,9 @@ def fixture_install_flow( If a test fails then using ``pytest --pdb`` and ``fixture_install_flow['result'].stderr`` may help with debugging. """ - result = mod_cylc_install_cli( + result = await mod_cylc_install_cli( fixture_provide_flow['srcpath'], - {'workflow_name': fixture_provide_flow['test_flow_name']} + fixture_provide_flow['test_flow_name'], ) install_conf_path = ( fixture_provide_flow['flowpath'] / @@ -130,20 +130,26 @@ def fixture_install_flow( } -def test_cylc_validate_srcdir(fixture_install_flow, mod_cylc_validate_cli): +async def test_cylc_validate_srcdir( + fixture_install_flow, + mod_cylc_validate_cli, +): """Sanity check that workflow validates: """ srcpath = fixture_install_flow['srcpath'] - result = mod_cylc_validate_cli(srcpath) + result = await mod_cylc_validate_cli(srcpath) search = re.findall(r'ROSE_ORIG_HOST \(.*\) is: (.*)', result.logging) assert search == [HOST, HOST] -def test_cylc_validate_rundir(fixture_install_flow, mod_cylc_validate_cli): +async def test_cylc_validate_rundir( + fixture_install_flow, + mod_cylc_validate_cli, +): """Sanity check that workflow validates: """ flowpath = fixture_install_flow['flowpath'] - result = mod_cylc_validate_cli(flowpath) + result = await mod_cylc_validate_cli(flowpath) assert 'ROSE_ORIG_HOST (env) is:' in result.logging diff --git a/tests/functional/test_pre_configure.py b/tests/functional/test_pre_configure.py index 00d423b7..5b597ba3 100644 --- a/tests/functional/test_pre_configure.py +++ b/tests/functional/test_pre_configure.py @@ -45,9 +45,9 @@ ) ] ) -def test_validate_fail(srcdir, expect, cylc_validate_cli): +async def test_validate_fail(srcdir, expect, cylc_validate_cli): srcdir = Path(__file__).parent / srcdir - validate = cylc_validate_cli(srcdir) + validate = await cylc_validate_cli(srcdir) assert validate.ret == 1 if expect: assert re.findall(expect, str(validate.exc)) @@ -77,11 +77,11 @@ def test_validate_fail(srcdir, expect, cylc_validate_cli): ('09_template_vars_vanilla', {'XYZ': 'xyz'}, None), ], ) -def test_validate(monkeypatch, srcdir, envvars, args, cylc_validate_cli): +async def test_validate(monkeypatch, srcdir, envvars, args, cylc_validate_cli): for key, value in (envvars or {}).items(): monkeypatch.setenv(key, value) srcdir = Path(__file__).parent / srcdir - validate = cylc_validate_cli(str(srcdir), args) + validate = await cylc_validate_cli(str(srcdir), args) assert validate.ret == 0 diff --git a/tests/functional/test_reinstall.py b/tests/functional/test_reinstall.py index 32abc432..4ee030a1 100644 --- a/tests/functional/test_reinstall.py +++ b/tests/functional/test_reinstall.py @@ -78,7 +78,7 @@ def fixture_provide_flow(tmp_path_factory, request): @pytest.fixture(scope='module') -def fixture_install_flow( +async def fixture_install_flow( fixture_provide_flow, monkeymodule, mod_cylc_install_cli ): """Run ``cylc install``. @@ -90,11 +90,12 @@ def fixture_install_flow( ``fixture_install_flow['result'].stderr`` may help with debugging. """ monkeymodule.setenv('ROSE_SUITE_OPT_CONF_KEYS', 'b') - result = mod_cylc_install_cli( - fixture_provide_flow['srcpath'], { + result = await mod_cylc_install_cli( + fixture_provide_flow['srcpath'], + fixture_provide_flow['test_flow_name'], + { 'opt_conf_keys': ['c'], - 'workflow_name': fixture_provide_flow['test_flow_name'] - } + }, ) yield { @@ -103,11 +104,11 @@ def fixture_install_flow( } -def test_cylc_validate(fixture_provide_flow, cylc_validate_cli): +async def test_cylc_validate(fixture_provide_flow, cylc_validate_cli): """Sanity check that workflow validates: """ srcpath = fixture_provide_flow['srcpath'] - assert cylc_validate_cli(str(srcpath)).ret == 0 + assert (await cylc_validate_cli(str(srcpath))).ret == 0 def test_cylc_install_run(fixture_install_flow): @@ -140,8 +141,8 @@ def test_cylc_install_files(fixture_install_flow, file_, expect): @pytest.fixture(scope='module') -def fixture_reinstall_flow( - fixture_provide_flow, monkeymodule, mod_cylc_reinstall_cli +async def fixture_reinstall_flow( + fixture_install_flow, monkeymodule, mod_cylc_reinstall_cli ): """Run ``cylc reinstall``. @@ -152,15 +153,13 @@ def fixture_reinstall_flow( ``fixture_install_flow['result'].stderr`` may help with debugging. """ monkeymodule.delenv('ROSE_SUITE_OPT_CONF_KEYS', raising=False) - result = mod_cylc_reinstall_cli( - f'{fixture_provide_flow["test_flow_name"]}/run1', - { - 'opt_conf_keys': ['d'] - } + result = await mod_cylc_reinstall_cli( + fixture_install_flow['result'].id, + {'opt_conf_keys': ['d']}, ) yield { - 'fixture_provide_flow': fixture_provide_flow, - 'result': result + 'fixture_install_flow': fixture_install_flow, + 'result': result, } @@ -189,13 +188,18 @@ def test_cylc_reinstall_run(fixture_reinstall_flow): ] ) def test_cylc_reinstall_files(fixture_reinstall_flow, file_, expect): - fpath = fixture_reinstall_flow['fixture_provide_flow']['flowpath'] + fpath = ( + fixture_reinstall_flow + ['fixture_install_flow'] + ['fixture_provide_flow'] + ['flowpath'] + ) assert (fpath / file_).read_text() == expect @pytest.fixture(scope='module') -def fixture_reinstall_flow2( - fixture_provide_flow, monkeymodule, mod_cylc_reinstall_cli +async def fixture_reinstall_flow2( + fixture_install_flow, monkeymodule, mod_cylc_reinstall_cli ): """Run ``cylc reinstall``. @@ -210,14 +214,17 @@ def fixture_reinstall_flow2( ``fixture_install_flow['result'].stderr`` may help with debugging. """ monkeymodule.delenv('ROSE_SUITE_OPT_CONF_KEYS', raising=False) - (fixture_provide_flow['srcpath'] / 'rose-suite.conf').write_text( - 'opts=z\n' - ) - result = mod_cylc_reinstall_cli( - f'{fixture_provide_flow["test_flow_name"]}/run1' + ( + fixture_install_flow + ['fixture_provide_flow'] + ['srcpath'] + / 'rose-suite.conf' + ).write_text('opts=z\n') + result = await mod_cylc_reinstall_cli( + fixture_install_flow['result'].id, ) yield { - 'fixture_provide_flow': fixture_provide_flow, + 'fixture_install_flow': fixture_install_flow, 'result': result } @@ -247,7 +254,12 @@ def test_cylc_reinstall_run2(fixture_reinstall_flow2): ] ) def test_cylc_reinstall_files2(fixture_reinstall_flow2, file_, expect): - fpath = fixture_reinstall_flow2['fixture_provide_flow']['flowpath'] + fpath = ( + fixture_reinstall_flow2 + ['fixture_install_flow'] + ['fixture_provide_flow'] + ['flowpath'] + ) assert (fpath / file_).read_text() == expect diff --git a/tests/functional/test_reinstall_clean.py b/tests/functional/test_reinstall_clean.py index 0a207430..e75cac0a 100644 --- a/tests/functional/test_reinstall_clean.py +++ b/tests/functional/test_reinstall_clean.py @@ -76,7 +76,7 @@ def fixture_provide_flow(tmp_path_factory, request): @pytest.fixture(scope='module') -def fixture_install_flow( +async def fixture_install_flow( fixture_provide_flow, monkeymodule, mod_cylc_install_cli ): """Run ``cylc install``. @@ -87,10 +87,10 @@ def fixture_install_flow( If a test fails using ``pytest --pdb then`` ``fixture_install_flow['result'].stderr`` may help with debugging. """ - result = mod_cylc_install_cli( + result = await mod_cylc_install_cli( fixture_provide_flow['srcpath'], + fixture_provide_flow['test_flow_name'], { - 'workflow_name': fixture_provide_flow['test_flow_name'], 'opt_conf_keys': ['bar'], 'defines': ['[env]FOO=1'] } @@ -126,8 +126,8 @@ def test_cylc_install_files(fixture_install_flow, file_, expect): @pytest.fixture(scope='module') -def fixture_reinstall_flow( - fixture_provide_flow, monkeymodule, mod_cylc_reinstall_cli +async def fixture_reinstall_flow( + fixture_install_flow, monkeymodule, mod_cylc_reinstall_cli ): """Run ``cylc reinstall --clear-rose-install-options``. @@ -141,8 +141,10 @@ def fixture_reinstall_flow( ``fixture_install_flow['result'].stderr`` may help with debugging. """ monkeymodule.delenv('ROSE_SUITE_OPT_CONF_KEYS', raising=False) - result = mod_cylc_reinstall_cli( - f'{fixture_provide_flow["test_flow_name"]}/run1', + result = await mod_cylc_reinstall_cli( + ( + fixture_install_flow['fixture_provide_flow']['test_flow_name'] + ), { 'opt_conf_keys': ['baz'], 'defines': ['[env]BAR=2'], @@ -150,7 +152,7 @@ def fixture_reinstall_flow( } ) yield { - 'fixture_provide_flow': fixture_provide_flow, + 'fixture_install_flow': fixture_install_flow, 'result': result } @@ -175,5 +177,10 @@ def test_cylc_reinstall_run(fixture_reinstall_flow): ] ) def test_cylc_reinstall_files(fixture_reinstall_flow, file_, expect): - fpath = fixture_reinstall_flow['fixture_provide_flow']['flowpath'] + fpath = ( + fixture_reinstall_flow + ['fixture_install_flow'] + ['fixture_provide_flow'] + ['flowpath'] + ) assert (fpath / file_).read_text() == expect diff --git a/tests/functional/test_reinstall_fileinstall.py b/tests/functional/test_reinstall_fileinstall.py index 817500c6..b1c523f6 100644 --- a/tests/functional/test_reinstall_fileinstall.py +++ b/tests/functional/test_reinstall_fileinstall.py @@ -23,9 +23,11 @@ import shutil from uuid import uuid4 -from cylc.flow.pathutil import get_workflow_run_dir import pytest +from cylc.flow.pathutil import get_workflow_run_dir + + WORKFLOW_SRC = Path(__file__).parent / '14_reinstall_fileinstall' @@ -49,18 +51,19 @@ def fixture_provide_flow(tmp_path_factory, request): shutil.rmtree(flowpath) -def test_install_flow(fixture_provide_flow, mod_cylc_install_cli): +async def test_install_flow(fixture_provide_flow, mod_cylc_install_cli): """Run ``cylc install``. """ - result = mod_cylc_install_cli( + result = await mod_cylc_install_cli( fixture_provide_flow['srcpath'], - {'workflow_name': fixture_provide_flow['test_flow_name']}) + fixture_provide_flow['test_flow_name'], + ) assert result.ret == 0 -def test_reinstall_flow(fixture_provide_flow, mod_cylc_reinstall_cli): +async def test_reinstall_flow(fixture_provide_flow, mod_cylc_reinstall_cli): """Run ``cylc reinstall``. """ - result = mod_cylc_reinstall_cli( + result = await mod_cylc_reinstall_cli( fixture_provide_flow['test_flow_name']) assert result.ret == 0 diff --git a/tests/functional/test_rose_fileinstall.py b/tests/functional/test_rose_fileinstall.py index ac99b3da..87834d7d 100644 --- a/tests/functional/test_rose_fileinstall.py +++ b/tests/functional/test_rose_fileinstall.py @@ -17,82 +17,79 @@ """Functional tests for Rose file installation.""" from pathlib import Path -import shutil -from uuid import uuid4 +from textwrap import dedent -from cylc.flow.pathutil import get_workflow_run_dir import pytest +from cylc.flow.pathutil import get_workflow_run_dir + +# @pytest.fixture(scope='module') @pytest.fixture -def fixture_provide_flow(tmp_path): +def workflow_source_dir(tmp_path): + """A source dir with a Rose config that configures file installation.""" # Set up paths for test: srcpath = tmp_path / 'src' + srcpath.mkdir() + + # the files to install are stored in a directory alongside this test file datapath = Path(__file__).parent / 'fileinstall_data' - for path in [srcpath]: - path.mkdir() # Create a unique flow name for this test: - flow_name = f'cylc-rose-test-{str(uuid4())[:8]}' - # Create source workflow: - (srcpath / 'flow.cylc').write_text( - '[scheduling]\n' - ' initial cycle point = 2020\n' - ' [[dependencies]]\n' - ' [[[R1]]]\n' - ' graph = pointless\n' - '[runtime]\n' - ' [[pointless]]\n' - ' script = true\n' - ) - (srcpath / 'rose-suite.conf').write_text( - '[file:lib/python/lion.py]\n' - f'source={str(datapath)}/lion.py\n' - '[file:data]\n' - f'source={str(datapath)}/*.data\n' - ) - yield srcpath, datapath, flow_name - + (srcpath / 'flow.cylc').touch() + (srcpath / 'rose-suite.conf').write_text(dedent(f''' + [file:lib/python/lion.py] + source={datapath}/lion.py -@pytest.fixture -def fixture_install_flow(fixture_provide_flow, request, cylc_install_cli): - srcpath, datapath, flow_name = fixture_provide_flow - result = cylc_install_cli(str(srcpath), {'workflow_name': flow_name}) - destpath = Path(get_workflow_run_dir(flow_name)) + [file:data] + source={datapath}/*.data + ''')) + yield srcpath, datapath - yield srcpath, datapath, flow_name, result, destpath - if not request.session.testsfailed: - shutil.rmtree(destpath, ignore_errors=True) +@pytest.fixture +async def installed_workflow( + workflow_source_dir, + cylc_install_cli, +): + srcpath, datapath = workflow_source_dir + result = await cylc_install_cli(srcpath) + assert result.ret == 0 # ensure the workflow installed successfully + workflow_id = result.id + run_dir = Path(get_workflow_run_dir(workflow_id)) + yield datapath, workflow_id, result, run_dir + + +async def test_rose_fileinstall_subfolders(installed_workflow): + """It should perform file installation creating directories as needed.""" + datapath, _, _, destpath = installed_workflow + assert (destpath / 'lib/python/lion.py').read_text() == ( + (datapath / 'lion.py').read_text() + ) -def test_rose_fileinstall_validate(fixture_provide_flow, cylc_validate_cli): - """Workflow validates: - """ - srcpath, _, _ = fixture_provide_flow - assert cylc_validate_cli(str(srcpath)).ret == 0 +def test_rose_fileinstall_concatenation(installed_workflow): + """It should install multiple sources into a single file. -def test_rose_fileinstall_run(fixture_install_flow): - """Workflow installs: + Note source contains wildcard. """ - _, _, _, result, _ = fixture_install_flow - assert result.ret == 0 - + datapath, _, _, destpath = installed_workflow + assert (destpath / 'data').read_text() == ( + (datapath / 'randoms1.data').read_text() + + (datapath / 'randoms3.data').read_text() + ) -def test_rose_fileinstall_subfolders(fixture_install_flow): - """File installed into a sub directory: - """ - _, datapath, _, _, destpath = fixture_install_flow - assert ((destpath / 'lib/python/lion.py').read_text() == - (datapath / 'lion.py').read_text()) +async def test_rose_fileinstall_error(tmp_path, cylc_install_cli): + """It should capture fileinstallation errors.""" + (tmp_path / 'flow.cylc').touch() + (tmp_path / 'rose-suite.conf').write_text(dedent(''' + [file:bad] + source=no-such-file + ''')) -def test_rose_fileinstall_concatenation(fixture_install_flow): - """Multiple files concatenated on install(source contained wildcard): - """ - _, datapath, _, _, destpath = fixture_install_flow - assert ((destpath / 'data').read_text() == - ((datapath / 'randoms1.data').read_text() + - (datapath / 'randoms3.data').read_text() - )) + result = await cylc_install_cli(tmp_path) + assert ( + 'file:bad=source=no-such-file: bad or missing value' + ) in str(result.exc) diff --git a/tests/functional/test_rose_stem.py b/tests/functional/test_rose_stem.py index 85a5a5a9..0e23a732 100644 --- a/tests/functional/test_rose_stem.py +++ b/tests/functional/test_rose_stem.py @@ -252,7 +252,7 @@ def rose_stem_run_template(setup_stem_repo, pytestconfig, monkeymodule): """ verbosity = pytestconfig.getoption('verbose') - def _inner_fn(rose_stem_opts, verbosity=verbosity): + async def _inner_fn(rose_stem_opts, verbosity=verbosity): monkeymodule.setattr('sys.argv', ['stem']) monkeymodule.chdir(setup_stem_repo['workingcopy']) parser, opts = get_rose_stem_opts() @@ -261,7 +261,7 @@ def _inner_fn(rose_stem_opts, verbosity=verbosity): run_stem = SimpleNamespace() run_stem.stdout = '' try: - rose_stem(parser, opts) + await rose_stem(parser, opts) run_stem.returncode = 0 run_stem.stderr = '' except Exception as exc: @@ -281,14 +281,14 @@ def _inner_fn(rose_stem_opts, verbosity=verbosity): @pytest.fixture(scope='class') -def rose_stem_run_really_basic(rose_stem_run_template, setup_stem_repo): +async def rose_stem_run_really_basic(rose_stem_run_template, setup_stem_repo): rose_stem_opts = { 'stem_groups': [], 'stem_sources': [ str(setup_stem_repo['workingcopy']), "fcm:foo.x_tr@head" ], } - yield rose_stem_run_template(rose_stem_opts) + yield await rose_stem_run_template(rose_stem_opts) class TestReallyBasic(): @@ -299,7 +299,7 @@ def test_really_basic(self, rose_stem_run_really_basic): @pytest.fixture(scope='class') -def rose_stem_run_basic(rose_stem_run_template, setup_stem_repo): +async def rose_stem_run_basic(rose_stem_run_template, setup_stem_repo): rose_stem_opts = { 'stem_groups': ['earl_grey', 'milk,sugar', 'spoon,cup,milk'], 'stem_sources': [ @@ -307,7 +307,7 @@ def rose_stem_run_basic(rose_stem_run_template, setup_stem_repo): ], 'workflow_name': setup_stem_repo['suitename'] } - yield rose_stem_run_template(rose_stem_opts) + yield await rose_stem_run_template(rose_stem_opts) class TestBasic(): @@ -338,7 +338,7 @@ def test_basic(self, rose_stem_run_basic, expected): @pytest.fixture(scope='class') -def project_override( +async def project_override( rose_stem_run_template, setup_stem_repo ): rose_stem_opts = { @@ -348,7 +348,7 @@ def project_override( ], 'workflow_name': setup_stem_repo['suitename'] } - yield rose_stem_run_template(rose_stem_opts) + yield await rose_stem_run_template(rose_stem_opts) class TestProjectOverride(): @@ -387,7 +387,7 @@ def test_project_override(self, project_override, expected): @pytest.fixture(scope='class') -def suite_redirection( +async def suite_redirection( rose_stem_run_template, setup_stem_repo ): rose_stem_opts = { @@ -396,7 +396,7 @@ def suite_redirection( 'stem_sources': ["fcm:foo.x_tr@head"], 'workflow_name': setup_stem_repo['suitename'] } - yield rose_stem_run_template(rose_stem_opts) + yield await rose_stem_run_template(rose_stem_opts) class TestSuiteRedirection: @@ -424,7 +424,7 @@ def test_suite_redirection(self, suite_redirection, expected): @pytest.fixture(scope='class') -def subdirectory( +async def subdirectory( rose_stem_run_template, setup_stem_repo ): rose_stem_opts = { @@ -432,7 +432,7 @@ def subdirectory( 'stem_sources': [f'{setup_stem_repo["workingcopy"]}/rose-stem'], 'workflow_name': setup_stem_repo['suitename'] } - yield rose_stem_run_template(rose_stem_opts) + yield await rose_stem_run_template(rose_stem_opts) class TestSubdirectory: @@ -463,7 +463,7 @@ def test_subdirectory(self, subdirectory, expected): @pytest.fixture(scope='class') -def relative_path( +async def relative_path( rose_stem_run_template, setup_stem_repo ): rose_stem_opts = { @@ -471,7 +471,7 @@ def relative_path( 'stem_groups': ['ceylon'], 'workflow_name': setup_stem_repo['suitename'] } - yield rose_stem_run_template(rose_stem_opts) + yield await rose_stem_run_template(rose_stem_opts) class TestRelativePath: @@ -503,7 +503,7 @@ def test_relative_path(self, relative_path, expected): @pytest.fixture(scope='class') -def with_config( +async def with_config( rose_stem_run_template, setup_stem_repo, mock_global_cfg ): """test for successful execution with site/user configuration @@ -519,7 +519,7 @@ def with_config( 'cylc.rose.stem.ResourceLocator.default', '[rose-stem]\nautomatic-options = MILK=true', ) - yield rose_stem_run_template(rose_stem_opts) + yield await rose_stem_run_template(rose_stem_opts) class TestWithConfig: @@ -544,7 +544,7 @@ def test_with_config(self, with_config): @pytest.fixture(scope='class') -def with_config2( +async def with_config2( rose_stem_run_template, setup_stem_repo, mock_global_cfg ): """test for successful execution with site/user configuration @@ -559,7 +559,7 @@ def with_config2( 'cylc.rose.stem.ResourceLocator.default', '[rose-stem]\nautomatic-options = MILK=true TEA=darjeeling', ) - yield rose_stem_run_template(rose_stem_opts) + yield await rose_stem_run_template(rose_stem_opts) class TestWithConfig2: @@ -578,7 +578,12 @@ def test_with_config2(self, with_config2): assert line in with_config2['jobout_content'] -def test_incompatible_versions(setup_stem_repo, monkeymodule, caplog, capsys): +async def test_incompatible_versions( + setup_stem_repo, + monkeymodule, + caplog, + capsys, +): """It fails if trying to install an incompatible version. """ # Copy suite into working copy. @@ -607,10 +612,10 @@ def test_incompatible_versions(setup_stem_repo, monkeymodule, caplog, capsys): with pytest.raises( RoseStemVersionException, match='1 but suite is at version 0' ): - rose_stem(parser, opts) + await rose_stem(parser, opts) -def test_project_not_in_keywords(setup_stem_repo, monkeymodule, capsys): +async def test_project_not_in_keywords(setup_stem_repo, monkeymodule, capsys): """It fails if it cannot extract project name from FCM keywords. """ # Copy suite into working copy. @@ -629,12 +634,12 @@ def test_project_not_in_keywords(setup_stem_repo, monkeymodule, capsys): parser, opts = get_rose_stem_opts() [setattr(opts, key, val) for key, val in rose_stem_opts.items()] - rose_stem(parser, opts) + await rose_stem(parser, opts) assert 'ProjectNotFoundException' in capsys.readouterr().err -def test_picks_template_section(setup_stem_repo, monkeymodule, capsys): +async def test_picks_template_section(setup_stem_repo, monkeymodule, capsys): """It can cope with template variables section being either ``template variables`` or ``jinja2:suite.rc``. """ @@ -645,6 +650,6 @@ def test_picks_template_section(setup_stem_repo, monkeymodule, capsys): '[template_variables]\n' ) parser, opts = get_rose_stem_opts() - rose_stem(parser, opts) + await rose_stem(parser, opts) _, err = capsys.readouterr() assert "[jinja2:suite.rc]' is deprecated" not in err diff --git a/tests/functional/test_utils.py b/tests/functional/test_utils.py index 3642fda9..a87ec494 100644 --- a/tests/functional/test_utils.py +++ b/tests/functional/test_utils.py @@ -17,9 +17,12 @@ """Unit tests for utilities.""" from pathlib import Path +from textwrap import dedent from cylc.rose.entry_points import copy_config_file +from cylc.flow.pathutil import get_workflow_run_dir + def test_basic(tmp_path): # Create files @@ -38,7 +41,7 @@ def test_basic(tmp_path): ) -def test_global_config_environment_validate( +async def test_global_config_environment_validate( monkeypatch, tmp_path, cylc_validate_cli ): """It should reload the global config after exporting env variables. @@ -69,7 +72,7 @@ def test_global_config_environment_validate( """) # Validate the config: - output = cylc_validate_cli(tmp_path) + output = await cylc_validate_cli(tmp_path) assert output.ret == 0 # CYLC_SYMLINKS == None the first time the global.cylc @@ -77,7 +80,7 @@ def test_global_config_environment_validate( assert output.logging.split('\n')[-1] == '"Foo"' -def test_global_config_environment_validate2( +async def test_global_config_environment_validate2( monkeypatch, tmp_path, cylc_install_cli ): """It should reload the global config after exporting env variables. @@ -85,18 +88,18 @@ def test_global_config_environment_validate2( See: https://github.com/cylc/cylc-rose/issues/237 """ # Setup global config: - global_conf = ( - '#!jinja2\n' - '[install]\n' - ' [[symlink dirs]]\n' - ' [[[localhost]]]\n' - '{% set cylc_symlinks = environ.get(\'CYLC_SYMLINKS\', None) %}\n' - '{% if cylc_symlinks == "foo" %}\n' - f'log = {str(tmp_path)}/foo\n' - '{% else %}\n' - f'log = {str(tmp_path)}/bar\n' - '{% endif %}\n' - ) + global_conf = dedent(f''' + #!jinja2 + [install] + [[symlink dirs]] + [[[localhost]]] + {{% set cylc_symlinks = environ.get(\'CYLC_SYMLINKS\', None) %}} + {{% if cylc_symlinks == "foo" %}} + log = {str(tmp_path)}/foo + {{% else %}} + log = {str(tmp_path)}/bar + {{% endif %}} + ''').strip() glbl_conf_path = tmp_path / 'conf' glbl_conf_path.mkdir() (glbl_conf_path / 'global.cylc').write_text(global_conf) @@ -115,15 +118,16 @@ def test_global_config_environment_validate2( """) # Install the config: - output = cylc_install_cli(tmp_path) + output = await cylc_install_cli(tmp_path) import sys for i in output.logging.split('\n'): print(i, file=sys.stderr) assert output.ret == 0 # Assert symlink created back to test_path/foo: + run_dir = get_workflow_run_dir(output.id) expected_msg = ( - f'Symlink created: {output.run_dir}/log -> ' + f'Symlink created: {run_dir}/log -> ' f'{tmp_path}/foo/cylc-run/{output.id}/log' ) assert expected_msg in output.logging.split('\n')[0] diff --git a/tests/unit/test_fileinstall.py b/tests/unit/test_fileinstall.py index a4f70abf..4a0cb673 100644 --- a/tests/unit/test_fileinstall.py +++ b/tests/unit/test_fileinstall.py @@ -56,12 +56,15 @@ def fixture_provide_flow(tmp_path_factory): @pytest.fixture(scope='module') -def fixture_install_flow(fixture_provide_flow, request, mod_cylc_install_cli): +async def fixture_install_flow( + fixture_provide_flow, request, + mod_cylc_install_cli, +): srcpath, datapath, flow_name = fixture_provide_flow - result = mod_cylc_install_cli( + result = await mod_cylc_install_cli( srcpath, + flow_name, { - 'workflow_name': flow_name, 'no_run_name': True, 'opt_conf_keys': ['A', 'B'], 'defines': ["[env]FOO=42", "[jinja2:suite.rc]BAR=84"], @@ -75,11 +78,14 @@ def fixture_install_flow(fixture_provide_flow, request, mod_cylc_install_cli): shutil.rmtree(destpath) -def test_rose_fileinstall_validate(fixture_provide_flow, cylc_validate_cli): +async def test_rose_fileinstall_validate( + fixture_provide_flow, + cylc_validate_cli, +): """Workflow validates: """ srcpath, _, _ = fixture_provide_flow - cylc_validate_cli(srcpath) + await cylc_validate_cli(srcpath) def test_rose_fileinstall_run(fixture_install_flow): From 89f2be6b3cf04ad1b2bd02729c7919b02e2ded44 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Thu, 22 Feb 2024 16:36:46 +0000 Subject: [PATCH 41/48] setup: add missing pytest-asyncio test dependency --- setup.cfg | 2 ++ 1 file changed, 2 insertions(+) diff --git a/setup.cfg b/setup.cfg index 4041cd8c..71d44d1c 100644 --- a/setup.cfg +++ b/setup.cfg @@ -68,6 +68,8 @@ include = cylc* tests = coverage>=5.0.0 pytest + # https://github.com/pytest-dev/pytest-asyncio/issues/705 + pytest-asyncio==0.21.* pytest-cov pytest-xdist>=2 lint = From 1111aae14def65c70fb8611dfb2b72e781403f11 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Thu, 22 Feb 2024 17:06:09 +0000 Subject: [PATCH 42/48] tests/functional: fix scoping issues --- tests/functional/test_rose_stem.py | 357 ++++++++++++++--------------- 1 file changed, 173 insertions(+), 184 deletions(-) diff --git a/tests/functional/test_rose_stem.py b/tests/functional/test_rose_stem.py index 0e23a732..ba662f9c 100644 --- a/tests/functional/test_rose_stem.py +++ b/tests/functional/test_rose_stem.py @@ -65,7 +65,6 @@ """ import os from pathlib import Path -import re from shlex import split import shutil import subprocess @@ -115,7 +114,7 @@ def monkeymodule(): mpatch.undo() -@pytest.fixture(scope='class') +@pytest.fixture() def mock_global_cfg(monkeymodule): """Mock the rose ResourceLocator.default @@ -136,7 +135,7 @@ def _inner(target, conf): yield _inner -@pytest.fixture(scope='class') +@pytest.fixture() def setup_stem_repo(tmp_path_factory, monkeymodule, request): """Setup a Rose Stem Repository for the tests. @@ -173,7 +172,7 @@ def setup_stem_repo(tmp_path_factory, monkeymodule, request): """ # Set up required folders: - testname = re.findall(r'Function\s(.*?)[\[>]', str(request))[0] + testname = request.function.__name__ basetemp = tmp_path_factory.getbasetemp() / testname baseinstall = basetemp / 'baseinstall' rose_stem_dir = baseinstall / 'trunk/rose-stem' @@ -181,7 +180,7 @@ def setup_stem_repo(tmp_path_factory, monkeymodule, request): confdir = basetemp / 'conf' workingcopy = basetemp / f'cylc-rose-stem-test-{str(uuid4())[:8]}' for dir_ in [baseinstall, repo, rose_stem_dir, confdir, workingcopy]: - dir_.mkdir(parents=True) + dir_.mkdir(parents=True, exist_ok=True) # Turn repo into an svn repo: subprocess.run(['svnadmin', 'create', f'{repo}/foo']) @@ -228,7 +227,7 @@ def setup_stem_repo(tmp_path_factory, monkeymodule, request): ResourceLocator.default(reset=True) -@pytest.fixture(scope='class') +@pytest.fixture() def rose_stem_run_template(setup_stem_repo, pytestconfig, monkeymodule): """Runs rose-stem @@ -280,7 +279,7 @@ async def _inner_fn(rose_stem_opts, verbosity=verbosity): yield _inner_fn -@pytest.fixture(scope='class') +@pytest.fixture() async def rose_stem_run_really_basic(rose_stem_run_template, setup_stem_repo): rose_stem_opts = { 'stem_groups': [], @@ -291,14 +290,13 @@ async def rose_stem_run_really_basic(rose_stem_run_template, setup_stem_repo): yield await rose_stem_run_template(rose_stem_opts) -class TestReallyBasic(): - def test_really_basic(self, rose_stem_run_really_basic): - """Check that assorted variables have been exported. - """ - assert rose_stem_run_really_basic['run_stem'].returncode == 0 +def test_really_basic(rose_stem_run_really_basic): + """Check that assorted variables have been exported. + """ + assert rose_stem_run_really_basic['run_stem'].returncode == 0 -@pytest.fixture(scope='class') +@pytest.fixture() async def rose_stem_run_basic(rose_stem_run_template, setup_stem_repo): rose_stem_opts = { 'stem_groups': ['earl_grey', 'milk,sugar', 'spoon,cup,milk'], @@ -310,34 +308,33 @@ async def rose_stem_run_basic(rose_stem_run_template, setup_stem_repo): yield await rose_stem_run_template(rose_stem_opts) -class TestBasic(): - @pytest.mark.parametrize( - 'expected', - [ - "run_ok", - "RUN_NAMES=['earl_grey', 'milk', 'sugar', 'spoon', 'cup', 'milk']", - "SOURCE_FOO=\"{workingcopy} fcm:foo.x_tr@head\"", - "HOST_SOURCE_FOO=\"{hostname}:{workingcopy} fcm:foo.x_tr@head\"", - "SOURCE_FOO_BASE=\"{workingcopy}\"\n", - "SOURCE_FOO_BASE=\"{hostname}:{workingcopy}\"\n", - "SOURCE_FOO_REV=\"\"\n", - "SOURCE_FOO_MIRROR=\"fcm:foo.xm/trunk@1\"\n", - ] - ) - def test_basic(self, rose_stem_run_basic, expected): - """Check that assorted variables have been exported. - """ - if expected == 'run_ok': - assert rose_stem_run_basic['run_stem'].returncode == 0 - else: - expected = expected.format( - workingcopy=rose_stem_run_basic['workingcopy'], - hostname=HOST - ) - assert expected in rose_stem_run_basic['jobout_content'] +@pytest.mark.parametrize( + 'expected', + [ + "run_ok", + "RUN_NAMES=['earl_grey', 'milk', 'sugar', 'spoon', 'cup', 'milk']", + "SOURCE_FOO=\"{workingcopy} fcm:foo.x_tr@head\"", + "HOST_SOURCE_FOO=\"{hostname}:{workingcopy} fcm:foo.x_tr@head\"", + "SOURCE_FOO_BASE=\"{workingcopy}\"\n", + "SOURCE_FOO_BASE=\"{hostname}:{workingcopy}\"\n", + "SOURCE_FOO_REV=\"\"\n", + "SOURCE_FOO_MIRROR=\"fcm:foo.xm/trunk@1\"\n", + ] +) +def test_basic(rose_stem_run_basic, expected): + """Check that assorted variables have been exported. + """ + if expected == 'run_ok': + assert rose_stem_run_basic['run_stem'].returncode == 0 + else: + expected = expected.format( + workingcopy=rose_stem_run_basic['workingcopy'], + hostname=HOST + ) + assert expected in rose_stem_run_basic['jobout_content'] -@pytest.fixture(scope='class') +@pytest.fixture() async def project_override( rose_stem_run_template, setup_stem_repo ): @@ -351,42 +348,41 @@ async def project_override( yield await rose_stem_run_template(rose_stem_opts) -class TestProjectOverride(): - @pytest.mark.parametrize( - 'expected', - [ - "run_ok", - ( - "RUN_NAMES=[\'earl_grey\', \'milk\', \'sugar\', " - "\'spoon\', \'cup\', \'milk\']" - ), - "SOURCE_FOO=\"fcm:foo.x_tr@head\"", - "HOST_SOURCE_FOO=\"fcm:foo.x_tr@head\"", - "SOURCE_BAR=\"{workingcopy}\"", - "HOST_SOURCE_BAR=\"{hostname}:{workingcopy}\"", - "SOURCE_FOO_BASE=\"fcm:foo.x_tr\"", - "HOST_SOURCE_FOO_BASE=\"fcm:foo.x_tr\"", - "SOURCE_BAR_BASE=\"{workingcopy}\"", - "HOST_SOURCE_BAR_BASE=\"{hostname}:{workingcopy}\"", - "SOURCE_FOO_REV=\"@1\"", - "SOURCE_BAR_REV=\"\"", - "SOURCE_FOO_MIRROR=\"fcm:foo.xm/trunk@1\"", - ] - ) - def test_project_override(self, project_override, expected): - """Check that assorted variables have been exported. - """ - if expected == 'run_ok': - assert project_override['run_stem'].returncode == 0 - else: - expected = expected.format( - workingcopy=project_override['workingcopy'], - hostname=HOST - ) - assert expected in project_override['jobout_content'] +@pytest.mark.parametrize( + 'expected', + [ + "run_ok", + ( + "RUN_NAMES=[\'earl_grey\', \'milk\', \'sugar\', " + "\'spoon\', \'cup\', \'milk\']" + ), + "SOURCE_FOO=\"fcm:foo.x_tr@head\"", + "HOST_SOURCE_FOO=\"fcm:foo.x_tr@head\"", + "SOURCE_BAR=\"{workingcopy}\"", + "HOST_SOURCE_BAR=\"{hostname}:{workingcopy}\"", + "SOURCE_FOO_BASE=\"fcm:foo.x_tr\"", + "HOST_SOURCE_FOO_BASE=\"fcm:foo.x_tr\"", + "SOURCE_BAR_BASE=\"{workingcopy}\"", + "HOST_SOURCE_BAR_BASE=\"{hostname}:{workingcopy}\"", + "SOURCE_FOO_REV=\"@1\"", + "SOURCE_BAR_REV=\"\"", + "SOURCE_FOO_MIRROR=\"fcm:foo.xm/trunk@1\"", + ] +) +def test_project_override(project_override, expected): + """Check that assorted variables have been exported. + """ + if expected == 'run_ok': + assert project_override['run_stem'].returncode == 0 + else: + expected = expected.format( + workingcopy=project_override['workingcopy'], + hostname=HOST + ) + assert expected in project_override['jobout_content'] -@pytest.fixture(scope='class') +@pytest.fixture() async def suite_redirection( rose_stem_run_template, setup_stem_repo ): @@ -399,31 +395,30 @@ async def suite_redirection( yield await rose_stem_run_template(rose_stem_opts) -class TestSuiteRedirection: - @pytest.mark.parametrize( - 'expected', - [ - "run_ok", - "RUN_NAMES=[\'lapsang\']", - "SOURCE_FOO=\"fcm:foo.x_tr@head\"", - "SOURCE_FOO_BASE=\"fcm:foo.x_tr\"", - "SOURCE_FOO_REV=\"@1\"", - ] - ) - def test_suite_redirection(self, suite_redirection, expected): - """Check that assorted variables have been exported. - """ - if expected == 'run_ok': - assert suite_redirection['run_stem'].returncode == 0 - else: - expected = expected.format( - workingcopy=suite_redirection['workingcopy'], - hostname=HOST - ) - assert expected in suite_redirection['jobout_content'] +@pytest.mark.parametrize( + 'expected', + [ + "run_ok", + "RUN_NAMES=[\'lapsang\']", + "SOURCE_FOO=\"fcm:foo.x_tr@head\"", + "SOURCE_FOO_BASE=\"fcm:foo.x_tr\"", + "SOURCE_FOO_REV=\"@1\"", + ] +) +def test_suite_redirection(suite_redirection, expected): + """Check that assorted variables have been exported. + """ + if expected == 'run_ok': + assert suite_redirection['run_stem'].returncode == 0 + else: + expected = expected.format( + workingcopy=suite_redirection['workingcopy'], + hostname=HOST + ) + assert expected in suite_redirection['jobout_content'] -@pytest.fixture(scope='class') +@pytest.fixture() async def subdirectory( rose_stem_run_template, setup_stem_repo ): @@ -435,34 +430,33 @@ async def subdirectory( yield await rose_stem_run_template(rose_stem_opts) -class TestSubdirectory: - @pytest.mark.parametrize( - 'expected', - [ - "run_ok", - "RUN_NAMES=[\'assam\']", - "SOURCE_FOO=\"{workingcopy}\"", - "HOST_SOURCE_FOO=\"{hostname}:{workingcopy}\"", - "SOURCE_FOO_BASE=\"{workingcopy}\"", - "HOST_SOURCE_FOO_BASE=\"{hostname}:{workingcopy}\"", - "SOURCE_FOO_REV=\"\"", - "SOURCE_FOO_MIRROR=\"fcm:foo.xm/trunk@1\"", - ] - ) - def test_subdirectory(self, subdirectory, expected): - """Check that assorted variables have been exported. - """ - if expected == 'run_ok': - assert subdirectory['run_stem'].returncode == 0 - else: - expected = expected.format( - workingcopy=subdirectory['workingcopy'], - hostname=HOST - ) - assert expected in subdirectory['jobout_content'] +@pytest.mark.parametrize( + 'expected', + [ + "run_ok", + "RUN_NAMES=[\'assam\']", + "SOURCE_FOO=\"{workingcopy}\"", + "HOST_SOURCE_FOO=\"{hostname}:{workingcopy}\"", + "SOURCE_FOO_BASE=\"{workingcopy}\"", + "HOST_SOURCE_FOO_BASE=\"{hostname}:{workingcopy}\"", + "SOURCE_FOO_REV=\"\"", + "SOURCE_FOO_MIRROR=\"fcm:foo.xm/trunk@1\"", + ] +) +def test_subdirectory(subdirectory, expected): + """Check that assorted variables have been exported. + """ + if expected == 'run_ok': + assert subdirectory['run_stem'].returncode == 0 + else: + expected = expected.format( + workingcopy=subdirectory['workingcopy'], + hostname=HOST + ) + assert expected in subdirectory['jobout_content'] -@pytest.fixture(scope='class') +@pytest.fixture() async def relative_path( rose_stem_run_template, setup_stem_repo ): @@ -474,35 +468,32 @@ async def relative_path( yield await rose_stem_run_template(rose_stem_opts) -class TestRelativePath: - """Check relative path with src is working. +@pytest.mark.parametrize( + 'expected', + [ + "run_ok", + "RUN_NAMES=[\'ceylon\']", + "SOURCE_FOO=\"{workingcopy}\"", + "HOST_SOURCE_FOO=\"{hostname}:{workingcopy}\"", + "SOURCE_FOO_BASE=\"{workingcopy}\"", + "HOST_SOURCE_FOO_BASE=\"{hostname}:{workingcopy}\"", + "SOURCE_FOO_REV=\"\"", + ] +) +def test_relative_path(relative_path, expected): + """Check that assorted variables have been exported. """ - @pytest.mark.parametrize( - 'expected', - [ - "run_ok", - "RUN_NAMES=[\'ceylon\']", - "SOURCE_FOO=\"{workingcopy}\"", - "HOST_SOURCE_FOO=\"{hostname}:{workingcopy}\"", - "SOURCE_FOO_BASE=\"{workingcopy}\"", - "HOST_SOURCE_FOO_BASE=\"{hostname}:{workingcopy}\"", - "SOURCE_FOO_REV=\"\"", - ] - ) - def test_relative_path(self, relative_path, expected): - """Check that assorted variables have been exported. - """ - if expected == 'run_ok': - assert relative_path['run_stem'].returncode == 0 - else: - expected = expected.format( - workingcopy=relative_path['workingcopy'], - hostname=HOST - ) - assert expected in relative_path['jobout_content'] + if expected == 'run_ok': + assert relative_path['run_stem'].returncode == 0 + else: + expected = expected.format( + workingcopy=relative_path['workingcopy'], + hostname=HOST + ) + assert expected in relative_path['jobout_content'] -@pytest.fixture(scope='class') +@pytest.fixture() async def with_config( rose_stem_run_template, setup_stem_repo, mock_global_cfg ): @@ -522,28 +513,27 @@ async def with_config( yield await rose_stem_run_template(rose_stem_opts) -class TestWithConfig: - def test_with_config(self, with_config): - """test for successful execution with site/user configuration - """ - assert with_config['run_stem'].returncode == 0 - for line in [ - "RUN_NAMES=['earl_grey', 'milk', 'sugar', 'spoon', 'cup', 'milk']", - 'SOURCE_FOO="{workingcopy} fcm:foo.x_tr@head"', - 'HOST_SOURCE_FOO="{hostname}:{workingcopy} fcm:foo.x_tr@head"', - 'SOURCE_FOO_BASE="{workingcopy}"', - 'HOST_SOURCE_FOO_BASE="{hostname}:{workingcopy}"', - 'SOURCE_FOO_REV=""', - 'MILK="true"', - ]: - line = line.format( - **with_config, - hostname=HOST - ) - assert line in with_config['jobout_content'] - - -@pytest.fixture(scope='class') +def test_with_config(with_config): + """test for successful execution with site/user configuration + """ + assert with_config['run_stem'].returncode == 0 + for line in [ + "RUN_NAMES=['earl_grey', 'milk', 'sugar', 'spoon', 'cup', 'milk']", + 'SOURCE_FOO="{workingcopy} fcm:foo.x_tr@head"', + 'HOST_SOURCE_FOO="{hostname}:{workingcopy} fcm:foo.x_tr@head"', + 'SOURCE_FOO_BASE="{workingcopy}"', + 'HOST_SOURCE_FOO_BASE="{hostname}:{workingcopy}"', + 'SOURCE_FOO_REV=""', + 'MILK="true"', + ]: + line = line.format( + **with_config, + hostname=HOST + ) + assert line in with_config['jobout_content'] + + +@pytest.fixture() async def with_config2( rose_stem_run_template, setup_stem_repo, mock_global_cfg ): @@ -562,20 +552,19 @@ async def with_config2( yield await rose_stem_run_template(rose_stem_opts) -class TestWithConfig2: - def test_with_config2(self, with_config2): - """test for successful execution with site/user configuration - """ - assert with_config2['run_stem'].returncode == 0 - for line in [ - 'MILK="true"', - 'TEA="darjeeling"', - ]: - line = line.format( - **with_config2, - hostname=HOST - ) - assert line in with_config2['jobout_content'] +def test_with_config2(with_config2): + """test for successful execution with site/user configuration + """ + assert with_config2['run_stem'].returncode == 0 + for line in [ + 'MILK="true"', + 'TEA="darjeeling"', + ]: + line = line.format( + **with_config2, + hostname=HOST + ) + assert line in with_config2['jobout_content'] async def test_incompatible_versions( From 4b24c031eae74e242194f4a2963ae41c00b9a9fa Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Mon, 26 Feb 2024 12:59:23 +0000 Subject: [PATCH 43/48] tests: remove defunct cli wrappers (#297) * The install/reinstall commands used to be called via the CLI. * They were converted to run via the Python API, but some of the CLI wrapper legacy remained. * This strips away the CLI wrapper to make native Python testing easier. --- tests/conftest.py | 52 +++----- tests/functional/test_ROSE_ORIG_HOST.py | 14 +-- tests/functional/test_reinstall.py | 114 +++++++----------- tests/functional/test_reinstall_clean.py | 69 ++++------- .../functional/test_reinstall_fileinstall.py | 13 +- tests/functional/test_rose_fileinstall.py | 23 ++-- tests/functional/test_utils.py | 18 ++- tests/unit/test_fileinstall.py | 17 +-- 8 files changed, 112 insertions(+), 208 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 731e285d..65023f2a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -172,7 +172,7 @@ async def _inner(srcpath, args=None): return _inner -def _cylc_install_cli(capsys, caplog, test_dir): +def _cylc_install_cli(test_dir): """Access the install CLI""" async def _inner(srcpath, workflow_name=None, opts=None): """Install a workflow. @@ -189,7 +189,7 @@ async def _inner(srcpath, workflow_name=None, opts=None): Dictionary of arguments for cylc install. """ - nonlocal capsys, caplog, test_dir + nonlocal test_dir if not workflow_name: workflow_name = str( (test_dir / str(uuid4())[:4]).relative_to(CYLC_RUN_DIR) @@ -197,26 +197,15 @@ async def _inner(srcpath, workflow_name=None, opts=None): options = Options( install_gop(), opts or {} )(workflow_name=workflow_name) - output = SimpleNamespace() if not options.workflow_name: options.workflow_name = workflow_name if not opts or not opts.get('no_run_name', ''): options.no_run_name = True - - try: - output.name, output.id = await cylc_install(options, str(srcpath)) - output.ret = 0 - output.exc = '' - except Exception as exc: - output.ret = 1 - output.exc = exc - output.logging = '\n'.join([i.message for i in caplog.records]) - output.out, output.err = capsys.readouterr() - return output + return await cylc_install(options, str(srcpath)) return _inner -def _cylc_reinstall_cli(capsys, caplog, test_dir): +def _cylc_reinstall_cli(test_dir): """Access the reinstall CLI""" async def _inner(workflow_id=None, opts=None): """Install a workflow. @@ -231,44 +220,33 @@ async def _inner(workflow_id=None, opts=None): Dictionary of arguments for cylc reinstall. """ - nonlocal capsys, caplog, test_dir + nonlocal test_dir if not workflow_id: workflow_id = str(test_dir.relative_to(CYLC_RUN_DIR)) options = Options(reinstall_gop(), opts or {})() - output = SimpleNamespace() - - try: - await cylc_reinstall(options, workflow_id) - output.ret = 0 - output.exc = '' - except Exception as exc: - # raise - output.ret = 1 - output.exc = exc - output.logging = '\n'.join([i.message for i in caplog.records]) - output.out, output.err = capsys.readouterr() - return output + options.skip_interactive = True + return await cylc_reinstall(options, workflow_id) return _inner @pytest.fixture -def cylc_install_cli(capsys, caplog, test_dir): - return _cylc_install_cli(capsys, caplog, test_dir) +def cylc_install_cli(test_dir): + return _cylc_install_cli(test_dir) @pytest.fixture(scope='module') -def mod_cylc_install_cli(mod_capsys, mod_caplog): - return _cylc_install_cli(mod_capsys, mod_caplog, mod_test_dir) +def mod_cylc_install_cli(mod_test_dir): + return _cylc_install_cli(mod_test_dir) @pytest.fixture -def cylc_reinstall_cli(capsys, caplog, test_dir): - return _cylc_reinstall_cli(capsys, caplog, test_dir) +def cylc_reinstall_cli(test_dir): + return _cylc_reinstall_cli(test_dir) @pytest.fixture(scope='module') -def mod_cylc_reinstall_cli(mod_capsys, mod_caplog, mod_test_dir): - return _cylc_reinstall_cli(mod_capsys, mod_caplog, mod_test_dir) +def mod_cylc_reinstall_cli(mod_test_dir): + return _cylc_reinstall_cli(mod_test_dir) @pytest.fixture diff --git a/tests/functional/test_ROSE_ORIG_HOST.py b/tests/functional/test_ROSE_ORIG_HOST.py index 7f9af1fb..0d96a96e 100644 --- a/tests/functional/test_ROSE_ORIG_HOST.py +++ b/tests/functional/test_ROSE_ORIG_HOST.py @@ -113,7 +113,7 @@ async def fixture_install_flow( If a test fails then using ``pytest --pdb`` and ``fixture_install_flow['result'].stderr`` may help with debugging. """ - result = await mod_cylc_install_cli( + await mod_cylc_install_cli( fixture_provide_flow['srcpath'], fixture_provide_flow['test_flow_name'], ) @@ -126,7 +126,6 @@ async def fixture_install_flow( install_conf_path.write_text(text) yield { **fixture_provide_flow, - 'result': result } @@ -134,8 +133,7 @@ async def test_cylc_validate_srcdir( fixture_install_flow, mod_cylc_validate_cli, ): - """Sanity check that workflow validates: - """ + """Sanity check that workflow validates.""" srcpath = fixture_install_flow['srcpath'] result = await mod_cylc_validate_cli(srcpath) search = re.findall(r'ROSE_ORIG_HOST \(.*\) is: (.*)', result.logging) @@ -146,13 +144,7 @@ async def test_cylc_validate_rundir( fixture_install_flow, mod_cylc_validate_cli, ): - """Sanity check that workflow validates: - """ + """Sanity check that workflow validates.""" flowpath = fixture_install_flow['flowpath'] result = await mod_cylc_validate_cli(flowpath) assert 'ROSE_ORIG_HOST (env) is:' in result.logging - - -def test_cylc_install_run(fixture_install_flow): - """install flow works.""" - assert fixture_install_flow['result'].ret == 0 diff --git a/tests/functional/test_reinstall.py b/tests/functional/test_reinstall.py index 4ee030a1..7c7b06bc 100644 --- a/tests/functional/test_reinstall.py +++ b/tests/functional/test_reinstall.py @@ -90,7 +90,7 @@ async def fixture_install_flow( ``fixture_install_flow['result'].stderr`` may help with debugging. """ monkeymodule.setenv('ROSE_SUITE_OPT_CONF_KEYS', 'b') - result = await mod_cylc_install_cli( + name, id_ = await mod_cylc_install_cli( fixture_provide_flow['srcpath'], fixture_provide_flow['test_flow_name'], { @@ -100,7 +100,8 @@ async def fixture_install_flow( yield { 'fixture_provide_flow': fixture_provide_flow, - 'result': result + 'name': name, + 'id': id_, } @@ -111,10 +112,6 @@ async def test_cylc_validate(fixture_provide_flow, cylc_validate_cli): assert (await cylc_validate_cli(str(srcpath))).ret == 0 -def test_cylc_install_run(fixture_install_flow): - assert fixture_install_flow['result'].ret == 0 - - @pytest.mark.parametrize( 'file_, expect', [ @@ -140,31 +137,45 @@ def test_cylc_install_files(fixture_install_flow, file_, expect): assert (fpath / file_).read_text() == expect -@pytest.fixture(scope='module') -async def fixture_reinstall_flow( - fixture_install_flow, monkeymodule, mod_cylc_reinstall_cli +@pytest.mark.parametrize( + 'file_, expect', + [ + ( + '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' + ) + ), + ( + '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' + f'\n[template variables]\n#{ROHIOS}\nROSE_ORIG_HOST={HOST}\n' + ) + ) + ] +) +async def test_cylc_reinstall_files( + fixture_install_flow, + monkeymodule, + mod_cylc_reinstall_cli, + file_, + expect, ): """Run ``cylc reinstall``. By running in a fixture with modular scope we can run tests on different aspects of its output as separate tests. - - If a test fails using ``pytest --pdb then`` - ``fixture_install_flow['result'].stderr`` may help with debugging. """ monkeymodule.delenv('ROSE_SUITE_OPT_CONF_KEYS', raising=False) - result = await mod_cylc_reinstall_cli( - fixture_install_flow['result'].id, + assert await mod_cylc_reinstall_cli( + fixture_install_flow['id'], {'opt_conf_keys': ['d']}, ) - yield { - 'fixture_install_flow': fixture_install_flow, - 'result': result, - } - - -def test_cylc_reinstall_run(fixture_reinstall_flow): - assert fixture_reinstall_flow['result'].ret == 0 + fpath = fixture_install_flow['fixture_provide_flow']['flowpath'] + assert (fpath / file_).read_text() == expect @pytest.mark.parametrize( @@ -174,7 +185,7 @@ def test_cylc_reinstall_run(fixture_reinstall_flow): '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' + 'opts=z b c d (cylc-install)\n' ) ), ( @@ -187,19 +198,12 @@ def test_cylc_reinstall_run(fixture_reinstall_flow): ) ] ) -def test_cylc_reinstall_files(fixture_reinstall_flow, file_, expect): - fpath = ( - fixture_reinstall_flow - ['fixture_install_flow'] - ['fixture_provide_flow'] - ['flowpath'] - ) - assert (fpath / file_).read_text() == expect - - -@pytest.fixture(scope='module') -async def fixture_reinstall_flow2( - fixture_install_flow, monkeymodule, mod_cylc_reinstall_cli +async def test_cylc_reinstall_files2( + fixture_install_flow, + monkeymodule, + mod_cylc_reinstall_cli, + file_, + expect, ): """Run ``cylc reinstall``. @@ -220,43 +224,11 @@ async def fixture_reinstall_flow2( ['srcpath'] / 'rose-suite.conf' ).write_text('opts=z\n') - result = await mod_cylc_reinstall_cli( - fixture_install_flow['result'].id, + assert await mod_cylc_reinstall_cli( + fixture_install_flow['id'], ) - yield { - 'fixture_install_flow': fixture_install_flow, - 'result': result - } - - -def test_cylc_reinstall_run2(fixture_reinstall_flow2): - assert fixture_reinstall_flow2['result'].ret == 0 - - -@pytest.mark.parametrize( - 'file_, expect', - [ - ( - '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' - ) - ), - ( - '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' - f'\n[template variables]\n#{ROHIOS}\nROSE_ORIG_HOST={HOST}\n' - ) - ) - ] -) -def test_cylc_reinstall_files2(fixture_reinstall_flow2, file_, expect): fpath = ( - fixture_reinstall_flow2 - ['fixture_install_flow'] + fixture_install_flow ['fixture_provide_flow'] ['flowpath'] ) diff --git a/tests/functional/test_reinstall_clean.py b/tests/functional/test_reinstall_clean.py index e75cac0a..e42617bc 100644 --- a/tests/functional/test_reinstall_clean.py +++ b/tests/functional/test_reinstall_clean.py @@ -87,7 +87,7 @@ async def fixture_install_flow( If a test fails using ``pytest --pdb then`` ``fixture_install_flow['result'].stderr`` may help with debugging. """ - result = await mod_cylc_install_cli( + await mod_cylc_install_cli( fixture_provide_flow['srcpath'], fixture_provide_flow['test_flow_name'], { @@ -97,14 +97,9 @@ async def fixture_install_flow( ) yield { 'fixture_provide_flow': fixture_provide_flow, - 'result': result } -def test_cylc_install_run(fixture_install_flow): - assert fixture_install_flow['result'].ret == 0 - - @pytest.mark.parametrize( 'file_, expect', [ @@ -125,9 +120,27 @@ def test_cylc_install_files(fixture_install_flow, file_, expect): assert (fpath / file_).read_text() == expect -@pytest.fixture(scope='module') -async def fixture_reinstall_flow( - fixture_install_flow, monkeymodule, mod_cylc_reinstall_cli +@pytest.mark.parametrize( + 'file_, expect', + [ + ( + 'opt/rose-suite-cylc-install.conf', ( + '# This file records CLI Options.\n\n' + '!opts=baz\n\n' + '[env]\n' + f'BAR=2\n#{ROHIOS}\n' + f'ROSE_ORIG_HOST={HOST}\n' + f'\n[template variables]\n#{ROHIOS}\nROSE_ORIG_HOST={HOST}\n' + ) + ) + ] +) +async def test_cylc_reinstall_files( + fixture_install_flow, + monkeymodule, + mod_cylc_reinstall_cli, + file_, + expect, ): """Run ``cylc reinstall --clear-rose-install-options``. @@ -136,12 +149,9 @@ async def fixture_reinstall_flow( By running in a fixture with modular scope we can run tests on different aspects of its output as separate tests. - - If a test fails using ``pytest --pdb then`` - ``fixture_install_flow['result'].stderr`` may help with debugging. """ monkeymodule.delenv('ROSE_SUITE_OPT_CONF_KEYS', raising=False) - result = await mod_cylc_reinstall_cli( + assert await mod_cylc_reinstall_cli( ( fixture_install_flow['fixture_provide_flow']['test_flow_name'] ), @@ -151,36 +161,5 @@ async def fixture_reinstall_flow( 'clear_rose_install_opts': True } ) - yield { - 'fixture_install_flow': fixture_install_flow, - 'result': result - } - - -def test_cylc_reinstall_run(fixture_reinstall_flow): - assert fixture_reinstall_flow['result'].ret == 0 - - -@pytest.mark.parametrize( - 'file_, expect', - [ - ( - 'opt/rose-suite-cylc-install.conf', ( - '# This file records CLI Options.\n\n' - '!opts=baz\n\n' - '[env]\n' - f'BAR=2\n#{ROHIOS}\n' - f'ROSE_ORIG_HOST={HOST}\n' - f'\n[template variables]\n#{ROHIOS}\nROSE_ORIG_HOST={HOST}\n' - ) - ) - ] -) -def test_cylc_reinstall_files(fixture_reinstall_flow, file_, expect): - fpath = ( - fixture_reinstall_flow - ['fixture_install_flow'] - ['fixture_provide_flow'] - ['flowpath'] - ) + fpath = fixture_install_flow['fixture_provide_flow']['flowpath'] assert (fpath / file_).read_text() == expect diff --git a/tests/functional/test_reinstall_fileinstall.py b/tests/functional/test_reinstall_fileinstall.py index b1c523f6..5592c491 100644 --- a/tests/functional/test_reinstall_fileinstall.py +++ b/tests/functional/test_reinstall_fileinstall.py @@ -52,18 +52,13 @@ def fixture_provide_flow(tmp_path_factory, request): async def test_install_flow(fixture_provide_flow, mod_cylc_install_cli): - """Run ``cylc install``. - """ - result = await mod_cylc_install_cli( + """Run ``cylc install``.""" + await mod_cylc_install_cli( fixture_provide_flow['srcpath'], fixture_provide_flow['test_flow_name'], ) - assert result.ret == 0 async def test_reinstall_flow(fixture_provide_flow, mod_cylc_reinstall_cli): - """Run ``cylc reinstall``. - """ - result = await mod_cylc_reinstall_cli( - fixture_provide_flow['test_flow_name']) - assert result.ret == 0 + """Run ``cylc reinstall``.""" + assert await mod_cylc_reinstall_cli(fixture_provide_flow['test_flow_name']) diff --git a/tests/functional/test_rose_fileinstall.py b/tests/functional/test_rose_fileinstall.py index 87834d7d..d22e124c 100644 --- a/tests/functional/test_rose_fileinstall.py +++ b/tests/functional/test_rose_fileinstall.py @@ -21,6 +21,7 @@ import pytest +from cylc.flow.exceptions import PluginError from cylc.flow.pathutil import get_workflow_run_dir @@ -54,16 +55,14 @@ async def installed_workflow( cylc_install_cli, ): srcpath, datapath = workflow_source_dir - result = await cylc_install_cli(srcpath) - assert result.ret == 0 # ensure the workflow installed successfully - workflow_id = result.id - run_dir = Path(get_workflow_run_dir(workflow_id)) - yield datapath, workflow_id, result, run_dir + _, id_ = await cylc_install_cli(srcpath) + run_dir = Path(get_workflow_run_dir(id_)) + yield datapath, run_dir async def test_rose_fileinstall_subfolders(installed_workflow): """It should perform file installation creating directories as needed.""" - datapath, _, _, destpath = installed_workflow + datapath, destpath = installed_workflow assert (destpath / 'lib/python/lion.py').read_text() == ( (datapath / 'lion.py').read_text() ) @@ -74,7 +73,7 @@ def test_rose_fileinstall_concatenation(installed_workflow): Note source contains wildcard. """ - datapath, _, _, destpath = installed_workflow + datapath, destpath = installed_workflow assert (destpath / 'data').read_text() == ( (datapath / 'randoms1.data').read_text() + (datapath / 'randoms3.data').read_text() @@ -88,8 +87,8 @@ async def test_rose_fileinstall_error(tmp_path, cylc_install_cli): [file:bad] source=no-such-file ''')) - - result = await cylc_install_cli(tmp_path) - assert ( - 'file:bad=source=no-such-file: bad or missing value' - ) in str(result.exc) + with pytest.raises( + PluginError, + match='file:bad=source=no-such-file: bad or missing value', + ): + await cylc_install_cli(tmp_path) diff --git a/tests/functional/test_utils.py b/tests/functional/test_utils.py index a87ec494..c83fb7ec 100644 --- a/tests/functional/test_utils.py +++ b/tests/functional/test_utils.py @@ -56,7 +56,7 @@ async def test_global_config_environment_validate( """ conf_path = tmp_path / 'conf' conf_path.mkdir() - monkeypatch.setenv('CYLC_CONF_PATH', conf_path) + monkeypatch.setenv('CYLC_CONF_PATH', str(conf_path)) # Setup workflow config: (conf_path / 'global.cylc').write_text(global_conf) @@ -81,7 +81,7 @@ async def test_global_config_environment_validate( async def test_global_config_environment_validate2( - monkeypatch, tmp_path, cylc_install_cli + caplog, monkeypatch, tmp_path, cylc_install_cli ): """It should reload the global config after exporting env variables. @@ -103,7 +103,7 @@ async def test_global_config_environment_validate2( glbl_conf_path = tmp_path / 'conf' glbl_conf_path.mkdir() (glbl_conf_path / 'global.cylc').write_text(global_conf) - monkeypatch.setenv('CYLC_CONF_PATH', glbl_conf_path) + monkeypatch.setenv('CYLC_CONF_PATH', str(glbl_conf_path)) # Setup workflow config: (tmp_path / 'rose-suite.conf').write_text( @@ -118,16 +118,12 @@ async def test_global_config_environment_validate2( """) # Install the config: - output = await cylc_install_cli(tmp_path) - import sys - for i in output.logging.split('\n'): - print(i, file=sys.stderr) - assert output.ret == 0 + _, id_ = await cylc_install_cli(tmp_path) # Assert symlink created back to test_path/foo: - run_dir = get_workflow_run_dir(output.id) + run_dir = get_workflow_run_dir(id_) expected_msg = ( f'Symlink created: {run_dir}/log -> ' - f'{tmp_path}/foo/cylc-run/{output.id}/log' + f'{tmp_path}/foo/cylc-run/{id_}/log' ) - assert expected_msg in output.logging.split('\n')[0] + assert expected_msg in caplog.messages diff --git a/tests/unit/test_fileinstall.py b/tests/unit/test_fileinstall.py index 4a0cb673..1df20148 100644 --- a/tests/unit/test_fileinstall.py +++ b/tests/unit/test_fileinstall.py @@ -61,7 +61,7 @@ async def fixture_install_flow( mod_cylc_install_cli, ): srcpath, datapath, flow_name = fixture_provide_flow - result = await mod_cylc_install_cli( + await mod_cylc_install_cli( srcpath, flow_name, { @@ -73,7 +73,7 @@ async def fixture_install_flow( ) destpath = Path(get_workflow_run_dir(flow_name)) - yield srcpath, datapath, flow_name, result, destpath + yield destpath if not request.session.testsfailed: shutil.rmtree(destpath) @@ -82,20 +82,13 @@ async def test_rose_fileinstall_validate( fixture_provide_flow, cylc_validate_cli, ): - """Workflow validates: - """ + """Workflow validates.""" srcpath, _, _ = fixture_provide_flow await cylc_validate_cli(srcpath) -def test_rose_fileinstall_run(fixture_install_flow): - """Workflow installs: - """ - pass # this tests the fixture itself - - def test_rose_fileinstall_rose_conf(fixture_install_flow): - _, _, _, result, destpath = fixture_install_flow + destpath = fixture_install_flow assert (destpath / 'rose-suite.conf').read_text() == ( "# Config Options 'A B (cylc-install)' from CLI appended to options " "already in `rose-suite.conf`.\n" @@ -104,7 +97,7 @@ def test_rose_fileinstall_rose_conf(fixture_install_flow): def test_rose_fileinstall_rose_suite_cylc_install_conf(fixture_install_flow): - _, _, _, result, destpath = fixture_install_flow + destpath = fixture_install_flow host = get_host() assert (destpath / 'opt/rose-suite-cylc-install.conf').read_text() == ( "# This file records CLI Options.\n\n" From 3936594a6eca5f209e51a60fe0b63750cae8a424 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 12 Mar 2024 15:30:50 +0000 Subject: [PATCH 44/48] build(deps): bump pypa/gh-action-pypi-publish from 1.8.11 to 1.8.14 (#301) [skip ci] --- .github/workflows/2_auto_publish_release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/2_auto_publish_release.yml b/.github/workflows/2_auto_publish_release.yml index 4b3ca585..7ab0d2d4 100644 --- a/.github/workflows/2_auto_publish_release.yml +++ b/.github/workflows/2_auto_publish_release.yml @@ -37,7 +37,7 @@ jobs: uses: cylc/release-actions/build-python-package@v1 - name: Publish distribution to PyPI - uses: pypa/gh-action-pypi-publish@v1.8.11 + uses: pypa/gh-action-pypi-publish@v1.8.14 with: user: __token__ # uses the API token feature of PyPI - least permissions possible password: ${{ secrets.PYPI_TOKEN }} From 60260cb4cb4414b93296b4098ab8b560a2a14197 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Fri, 19 Apr 2024 13:00:43 +0100 Subject: [PATCH 45/48] add developer docs --- DEVELOPING.md | 67 ++++ README.md | 4 + etc/cylc-rose-opts.svg | 782 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 853 insertions(+) create mode 100644 DEVELOPING.md create mode 100644 etc/cylc-rose-opts.svg diff --git a/DEVELOPING.md b/DEVELOPING.md new file mode 100644 index 00000000..7ac54fd5 --- /dev/null +++ b/DEVELOPING.md @@ -0,0 +1,67 @@ +# Cylc Rose Dev Docs + +Cylc Rose provides integration between Cylc and Rose. + +Its functionality is defined in the +[cylc-rose proposal](https://github.com/cylc/cylc-admin/blob/master/docs/proposal-cylc-rose-installing-rose-configs.md). + +## Cylc Rose Options + +### Rose Config Files + +Rose suite configurations are Cylc workflows which contain a `rose-suite.conf` +file at the top level. This file configures: + +* Template variables. +* File installation. +* Environment variables for workflow configuration. + +Rose suite configurations may additionally contain optional configuration files +in the `opts/` directory. Note that optional configurations may be turned on by +default via the `opt` configuration in the `rose-suite.conf` file. + +### The Options + +The Rose suite configuration may be extended or overridden via the Cylc Rose +CLI options. + +Cylc Rose adds three options to Cylc Flow. + +* `-S` - template variables +* `-D` - any arbitrary rose configuration (includes template variables) +* `-O` - optional configs + +These options are currently hardcoded in the cylc-flow source code as we +do not yet have the ability for Cylc plugins (such as cylc-rose) to inject +options into cylc-flow commands. + +There is also the `ROSE_SUITE_OPT_CONF_KEYS` environment variable which compliments +the `-O` option. + +### Storage + +Cylc Rose options passed in on the CLI are written to the +`~/cylc-run//opt/rose-suite-cylc-install.conf` file by +the `post_install` plugin. + +This preserves these options so that they are inherited by subsequent commands +avoiding the need for users to remember which Cylc Rose options they have used +and specify them with all future commands. + +### Reinstallation + +On reinstallation, additional Cylc Rose CLI options may be provided. + +If specified, these will override those previously specified on the CLI. + +### Option Lifecycle + +In order to support Cylc's compound commands (single commands that may perform +multiple individual operations as a transaction), the `post_install` plugin +deletes the Cylc Rose CLI options (from the object it was passed) after it has +written them to the filesystem. + +This is necessary to avoid the same options being reenacted in later stages of +compound commands. See https://github.com/cylc/cylc-flow/issues/5968 + +![lifecycle-diagram](./etc/cylc-rose-opts.svg) diff --git a/README.md b/README.md index d1d92143..60e3c215 100644 --- a/README.md +++ b/README.md @@ -56,6 +56,10 @@ This plugin provides support for the `rose-suite.conf` file, namely: the new Cylc `symlink dirs` functionality. * Graphical configuration editors. +### How It Works + +For developer documentation, see [DEVELOPING](DEVELOPING.md). + ### Contributing [![Contributors](https://img.shields.io/github/contributors/cylc/cylc-rose.svg?color=9cf)](https://github.com/cylc/cylc-rose/graphs/contributors) diff --git a/etc/cylc-rose-opts.svg b/etc/cylc-rose-opts.svg new file mode 100644 index 00000000..77a4b92f --- /dev/null +++ b/etc/cylc-rose-opts.svg @@ -0,0 +1,782 @@ + + + + + + + + + + image/svg+xml + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Rose Opts + Command + -S & -D & -O + Validate + Install + Play + Validate + Reinstall + Reload + Clean + pre_configure + post_install + + pre_configure + pre_configure + + + + + + + + + + + + pre_configure + pre_configure + post_install + pre_configure + Reload + pre_configure + File System + opts/rose-suite-cylc-install.conf + + + + + + + + + + + + + + + + + + + + + + + + + + + + + vip + + + + vr + reload + UserCommand + + From 0bf524012228258cf720826724d7653b3020e011 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Tue, 23 Apr 2024 15:25:43 +0100 Subject: [PATCH 46/48] Apply suggestions from code review Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> --- DEVELOPING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DEVELOPING.md b/DEVELOPING.md index 7ac54fd5..47acbd16 100644 --- a/DEVELOPING.md +++ b/DEVELOPING.md @@ -35,7 +35,7 @@ These options are currently hardcoded in the cylc-flow source code as we do not yet have the ability for Cylc plugins (such as cylc-rose) to inject options into cylc-flow commands. -There is also the `ROSE_SUITE_OPT_CONF_KEYS` environment variable which compliments +There is also the `ROSE_SUITE_OPT_CONF_KEYS` environment variable which complements the `-O` option. ### Storage From 69f321cef6ef2ef601f73b84319bb922a4d23f89 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Thu, 25 Apr 2024 12:51:09 +0100 Subject: [PATCH 47/48] simplify rose stem tests (#314) * stem tests: move monkeymodule the top level conftest * stem tests: remove intermediate fixtures * Remove fixtures specific to individual tests. * Move parametrization into tests where possible. * stem tests: remove outdated docs and unused code * stem tests: remove subprocess styled wrapper for rose stem execution * We used to use subprocess to run the tests. * This was removed but the logic for returncodes etc was kept. * stem tests: add descriptions to rose-stem tests. * Link each test back to the original source (metomi/rose). * Try to determine the purpose of each test and hint it in comments. * stem tests: use the rose config to check template vars * Rather than testing the config file line by line, read the config in and test the variables exist in the section we expect them to be in. * Add installed template vars into the rose stem fixture's return result. * stem tests: remove workflow name parameter * Don't explicitly speciy the workflow name, `cylc install` sets this to the basename of the source dir automatically. * stem tests: always use fixture to run rose_stem * Some tests were using `rose_stem_runner`, other's weren't and were replicating its logic locally. * stem tests: sort out fixture scopes * Convert the Rose Stem project fixture to module scope to avoid re-creating it for every test. * Moved the cleanup logic out of the Rose Stem project fixture into to fixture that runs `rose stem` itself. * Niceified the fixture interfaces. * Converted the function-scoped fixtures to request only function-scoped fixtures themselves. * stem tests: unify rose_stem teardown logic * Move `rose_stem` test fixture into the conftest.py with the others * Unify with the `test_dir` fixture (for provisioning the Cylc run dir) that all the other fixtures use. * Remove the resource teardown logic from the `rose_stem` fixture, fixing a bug where the source directories from successful tests could be left behind. * review feedback --- setup.cfg | 2 +- tests/conftest.py | 93 +++- tests/functional/test_rose_stem.py | 750 ++++++++++------------------- 3 files changed, 350 insertions(+), 495 deletions(-) diff --git a/setup.cfg b/setup.cfg index 71d44d1c..82498d59 100644 --- a/setup.cfg +++ b/setup.cfg @@ -67,7 +67,7 @@ include = cylc* [options.extras_require] tests = coverage>=5.0.0 - pytest + pytest>=6.2.0 # https://github.com/pytest-dev/pytest-asyncio/issues/705 pytest-asyncio==0.21.* pytest-cov diff --git a/tests/conftest.py b/tests/conftest.py index 65023f2a..be321628 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -15,6 +15,7 @@ # along with this program. If not, see . import asyncio +from io import StringIO from pathlib import Path from shutil import rmtree from types import SimpleNamespace @@ -33,6 +34,14 @@ from cylc.flow.scripts.validate import get_option_parser as validate_gop from cylc.flow.wallclock import get_current_time_string +from metomi.rose.resource import ResourceLocator +from metomi.rose.config import ConfigLoader + +from cylc.rose.stem import ( + get_rose_stem_opts, + rose_stem as _rose_stem, +) + CYLC_RUN_DIR = Path(get_cylc_run_dir()) @@ -62,6 +71,13 @@ def event_loop(): loop.close() +@pytest.fixture(scope='module') +def monkeymodule(): + """Make monkeypatching available in a module scope.""" + with pytest.MonkeyPatch.context() as mp: + yield mp + + @pytest.fixture() def workflow_name(): return 'cylc-rose-test-' + str(uuid4())[:8] @@ -97,8 +113,7 @@ def mod_caplog(request): @pytest.fixture(scope='package', autouse=True) def set_cylc_version(): - from _pytest.monkeypatch import MonkeyPatch - mpatch = MonkeyPatch() + mpatch = pytest.MonkeyPatch() yield mpatch.setenv('CYLC_VERSION', CYLC_VERSION) mpatch.undo() @@ -259,6 +274,80 @@ def mod_cylc_validate_cli(mod_capsys, mod_caplog): return _cylc_validate_cli(mod_capsys, mod_caplog) +@pytest.fixture +def rose_stem(test_dir, monkeypatch, request): + """The Rose Stem command. + + Wraps the "rose_stem" async function for use in tests. + + Cleans up afterwards if the test was successful. + """ + run_dir = test_dir / str(uuid4())[:4] + + async def _inner(source_dir, cwd=None, **rose_stem_opts): + nonlocal monkeypatch, request, run_dir + + # point rose-stem at the desired run directory + rose_stem_opts['no_run_name'] = True + rose_stem_opts['workflow_name'] = str( + run_dir.relative_to(CYLC_RUN_DIR) + ) + + # make it look like we're running the "rose stem" CLI + monkeypatch.setattr('sys.argv', ['stem']) + + # cd into the rose stem project directory (unless overridden) + monkeypatch.chdir(cwd or source_dir) + + # merge the opts in with the defaults + parser, opts = get_rose_stem_opts() + for key, val in rose_stem_opts.items(): + setattr(opts, key, val) + + # run rose stem + await _rose_stem(parser, opts) + + # return a dictionary of template variables found in the + # cylc-install optional configuration + cylc_install_opt_conf = Path( + run_dir, + 'opt/rose-suite-cylc-install.conf', + ) + if cylc_install_opt_conf.exists(): + opt_conf = ConfigLoader().load(str(cylc_install_opt_conf)) + return { + key: node.value # noqa B035 (false positive) + for [_, key], node in opt_conf.get( + ('template variables',) + ).walk() + } + else: + return {} + + return _inner + + +@pytest.fixture() +def mock_global_cfg(monkeypatch): + """Mock the rose ResourceLocator.default + + Args: + target: The module to patch. + conf: A fake rose global config as a string. + + """ + def _inner(target, conf): + """Get the ResourceLocator.default and patch its get_conf method.""" + obj = ResourceLocator.default() + monkeypatch.setattr( + obj, 'get_conf', lambda: ConfigLoader().load(StringIO(conf)) + ) + + monkeypatch.setattr(target, lambda *_, **__: obj) + + yield _inner + + @pytest.fixture(scope='session') def run_dir(): """The cylc run directory for this host.""" diff --git a/tests/functional/test_rose_stem.py b/tests/functional/test_rose_stem.py index ba662f9c..878ff1a7 100644 --- a/tests/functional/test_rose_stem.py +++ b/tests/functional/test_rose_stem.py @@ -18,73 +18,21 @@ # along with Rose. If not, see . # ----------------------------------------------------------------------------- -""" -Tests for cylc.rose.stem -======================== - -These tests are based on the Rose 2019 tests for Rose Stem. - -Structure ---------- - -#. ``setup_stem_repo`` is a module scoped fixture which creates a Rose Stem - repository which is used for all the tests. -#. ``rose_stem_run_template`` is a class scoped fixture, which runs a rose - stem command. Most of the tests are encapsulated in classes to allow - this expensive fixture to be run only once per class. Most of the tests - check that the ``rose stem`` has returned 0, and then check that variables - have been written to a job file. -#. For each test class there is a fixture encapsulating the test to be run. - -.. code:: - - ┌───────┬──────────┬───────────┬───────────┬───────────────┐ - │ │ │ │ │Test rose stem │ - │ │ test │ rose stem │ Class │returned 0 │ - │set-up │ specific │ runner │ container ├───────────────┤ - │repo │ fixture │ fixture │ │Test for output│ - │fixture│ (set up │ │ Only run │strings "foo" │ - │ │ rose stem│ (run │ class ├───────────────┤ - │ │ command) │ rose stem)│ fixture │Test for output│ - │ │ │ │ once │strings "bar" │ - │ ├──────────┼─ ── ── ── ├───────────┼───────────────┤ - │ │ │ │ │ │ - │ │ │ │ ├───────────────┤ - │ │ │ │ │ │ - │ │ │ │ ├───────────────┤ - │ │ │ │ │ │ - -Debugging ---------- -Because of the tasks being run in subprocesses debugging can be a little -tricky. As a result there is a commented ``breakpoint`` in -``rose_stem_run_template`` indicating a location where it might be useful -to investigate failing tests. - - -""" +"""Tests for cylc.rose.stem""" + import os from pathlib import Path from shlex import split import shutil import subprocess -from io import StringIO -from types import SimpleNamespace from uuid import uuid4 +from typing import Dict - -from metomi.rose.config import ConfigLoader from metomi.rose.host_select import HostSelector -from cylc.flow.pathutil import get_workflow_run_dir -from metomi.rose.resource import ResourceLocator -import pytest -from cylc.rose.stem import ( - RoseStemVersionException, - get_rose_stem_opts, - rose_stem, -) +import pytest +from cylc.rose.stem import RoseStemVersionException # We want to test Rose-Stem's insertion of the hostname, # not Rose's method of getting the hostname, so it doesn't @@ -93,10 +41,6 @@ HOST = HostSelector().get_local_host() -class SubprocessesError(Exception): - ... - - # Check that FCM is present on system, skipping checks elsewise: try: subprocess.run(['fcm', '--version']) @@ -105,80 +49,33 @@ class SubprocessesError(Exception): @pytest.fixture(scope='module') -def monkeymodule(): - """Make monkeypatching available in a module scope. - """ - from _pytest.monkeypatch import MonkeyPatch - mpatch = MonkeyPatch() - yield mpatch - mpatch.undo() - +def rose_stem_project(tmp_path_factory, monkeymodule, request): + """A Rose Stem project's root directory. -@pytest.fixture() -def mock_global_cfg(monkeymodule): - """Mock the rose ResourceLocator.default - - Args (To _inner): - target: The module to patch. - conf: A fake rose global config as a string. - """ - def _inner(target, conf): - """Get the ResourceLocator.default and patch its get_conf method - """ - obj = ResourceLocator.default() - monkeymodule.setattr( - obj, 'get_conf', lambda: ConfigLoader().load(StringIO(conf)) - ) + The project has the following structure:: - monkeymodule.setattr(target, lambda *_, **__: obj) - - yield _inner - - -@pytest.fixture() -def setup_stem_repo(tmp_path_factory, monkeymodule, request): - """Setup a Rose Stem Repository for the tests. - - creates the following repo structure: - - .. code:: - - |-- baseinstall - | `-- trunk + / + |-- baseinstall/ + | `-- trunk/ | `-- rose-stem - |-- conf + |-- conf/ | `-- keyword.cfg - |-- cylc-rose-stem-test-1df3e028 - | `-- rose-stem + |-- cylc-rose-stem-test-project-1df3e028/ + | `-- rose-stem/ | |-- flow.cylc | `-- rose-suite.conf - `-- rose-test-battery-stemtest-repo - `-- foo - - - Yields: - dictionary: - basetemp: - The location of the base temporary file, which allows tests - to modify any part of the rose-stem suite. - workingcopy: - Path to the location of the working copy. - suitename: - The name of the suite, which will be name of the suite/workflow - installed in ``~/cylc-run``. - suite_install_directory: - The path to the installed suite/workflow. Handy for cleaning - up after tests. + `-- rose-test-battery-stemtest-repo/ + `-- foo/ + `- """ # Set up required folders: - testname = request.function.__name__ - basetemp = tmp_path_factory.getbasetemp() / testname + basetemp = tmp_path_factory.getbasetemp() / request.module.__name__ baseinstall = basetemp / 'baseinstall' rose_stem_dir = baseinstall / 'trunk/rose-stem' repo = basetemp / 'rose-test-battery-stemtest-repo' confdir = basetemp / 'conf' - workingcopy = basetemp / f'cylc-rose-stem-test-{str(uuid4())[:8]}' + workingcopy = basetemp / f'cylc-rose-stem-test-project-{str(uuid4())[:8]}' for dir_ in [baseinstall, repo, rose_stem_dir, confdir, workingcopy]: dir_.mkdir(parents=True, exist_ok=True) @@ -197,7 +94,6 @@ def setup_stem_repo(tmp_path_factory, monkeymodule, request): ) monkeymodule.setenv('FCM_CONF_PATH', str(confdir)) # Check out a working copy of the repo: - suitename = workingcopy.parts[-1] subprocess.run(split(f'fcm checkout -q fcm:foo.x_tr {workingcopy}')) # Copy suite into working copy. test_src_dir = Path(__file__).parent / '12_rose_stem' @@ -205,440 +101,310 @@ def setup_stem_repo(tmp_path_factory, monkeymodule, request): src = str(test_src_dir / file) dest = str(workingcopy / 'rose-stem') shutil.copy2(src, dest) - suite_install_dir = get_workflow_run_dir(suitename) monkeymodule.setattr( 'cylc.flow.pathutil.make_symlink_dir', lambda *_, **__: {} ) - yield { - 'basetemp': basetemp, - 'workingcopy': workingcopy, - 'suitename': suitename, - 'suite_install_dir': suite_install_dir - } - # Only clean up if all went well. - if ( - not request.session.testsfailed - and Path(suite_install_dir).exists() - ): - shutil.rmtree(suite_install_dir) - ResourceLocator.default(reset=True) - - -@pytest.fixture() -def rose_stem_run_template(setup_stem_repo, pytestconfig, monkeymodule): - """Runs rose-stem - - Uses an inner function to allow inheriting fixtures to run different - cylc-run commands. - - Args: - setup_stem_repo (function): - Pytest fixture function setting up the structure of a rose-stem - suite - - Yields: - function which can be give a rose-stem command. This function returns: - dict: - 'run_stem': subprocess.CompletedProcess - The results of running rose-stem. - 'jobout_content': str - Content of test job output file. - ``**setup_stem_repo``: - Unpack all yields from setup_stem_repo. - """ - verbosity = pytestconfig.getoption('verbose') - - async def _inner_fn(rose_stem_opts, verbosity=verbosity): - monkeymodule.setattr('sys.argv', ['stem']) - monkeymodule.chdir(setup_stem_repo['workingcopy']) - parser, opts = get_rose_stem_opts() - [setattr(opts, key, val) for key, val in rose_stem_opts.items()] - - run_stem = SimpleNamespace() - run_stem.stdout = '' - try: - await rose_stem(parser, opts) - run_stem.returncode = 0 - run_stem.stderr = '' - except Exception as exc: - run_stem.returncode = 1 - run_stem.stderr = exc - - return { - 'run_stem': run_stem, - 'jobout_content': ( - Path(setup_stem_repo['suite_install_dir']) / - 'runN/opt/rose-suite-cylc-install.conf' - ).read_text(), - **setup_stem_repo - } - - yield _inner_fn - - -@pytest.fixture() -async def rose_stem_run_really_basic(rose_stem_run_template, setup_stem_repo): - rose_stem_opts = { - 'stem_groups': [], - 'stem_sources': [ - str(setup_stem_repo['workingcopy']), "fcm:foo.x_tr@head" - ], - } - yield await rose_stem_run_template(rose_stem_opts) + return workingcopy + + +def check_template_variables( + expected: Dict[str, str], got: Dict[str, str] +) -> None: + """Check template variable dictionaries. + Check the template vars in "expected" are present and match those in "got". + + Raises: + AssertionError -def test_really_basic(rose_stem_run_really_basic): - """Check that assorted variables have been exported. """ - assert rose_stem_run_really_basic['run_stem'].returncode == 0 + for key in expected: + assert key in got, f'template var {key} missing from config' + assert ( + expected[key] == got[key] + ), f'template var {key}={got[key]}, expected {expected[key]}' + + +async def test_hello_world(rose_stem, rose_stem_project): + """It should run a hello-world example without erroring.""" + await rose_stem( + rose_stem_project, + stem_groups=[], + stem_sources=[str(rose_stem_project), "fcm:foo.x_tr@head"], + ) -@pytest.fixture() -async def rose_stem_run_basic(rose_stem_run_template, setup_stem_repo): - rose_stem_opts = { - 'stem_groups': ['earl_grey', 'milk,sugar', 'spoon,cup,milk'], - 'stem_sources': [ - str(setup_stem_repo['workingcopy']), "fcm:foo.x_tr@head" - ], - 'workflow_name': setup_stem_repo['suitename'] - } - yield await rose_stem_run_template(rose_stem_opts) - - -@pytest.mark.parametrize( - 'expected', - [ - "run_ok", - "RUN_NAMES=['earl_grey', 'milk', 'sugar', 'spoon', 'cup', 'milk']", - "SOURCE_FOO=\"{workingcopy} fcm:foo.x_tr@head\"", - "HOST_SOURCE_FOO=\"{hostname}:{workingcopy} fcm:foo.x_tr@head\"", - "SOURCE_FOO_BASE=\"{workingcopy}\"\n", - "SOURCE_FOO_BASE=\"{hostname}:{workingcopy}\"\n", - "SOURCE_FOO_REV=\"\"\n", - "SOURCE_FOO_MIRROR=\"fcm:foo.xm/trunk@1\"\n", - ] -) -def test_basic(rose_stem_run_basic, expected): - """Check that assorted variables have been exported. +async def test_template_variables(rose_stem, rose_stem_project): + """It should set various template variables. + + https://github.com/metomi/rose/blob/2c8956a9464bd277c8eb24d38af4803cba4c1243/t/rose-stem/00-run-basic.t#L56-L78 """ - if expected == 'run_ok': - assert rose_stem_run_basic['run_stem'].returncode == 0 - else: - expected = expected.format( - workingcopy=rose_stem_run_basic['workingcopy'], - hostname=HOST - ) - assert expected in rose_stem_run_basic['jobout_content'] + template_vars = await rose_stem( + rose_stem_project, + stem_groups=['earl_grey', 'milk,sugar', 'spoon,cup,milk'], + stem_sources=[str(rose_stem_project), "fcm:foo.x_tr@head"], + ) + check_template_variables( + { + "RUN_NAMES": + "['earl_grey', 'milk', 'sugar', 'spoon', 'cup', 'milk']", + "SOURCE_FOO": + f'"{rose_stem_project} fcm:foo.x_tr@head"', + "HOST_SOURCE_FOO": + f'"{HOST}:{rose_stem_project} ' + 'fcm:foo.x_tr@head"', + "SOURCE_FOO_BASE": + f'"{rose_stem_project}"', + "HOST_SOURCE_FOO_BASE": + f'"{HOST}:{rose_stem_project}"', + "SOURCE_FOO_REV": + '""', + "SOURCE_FOO_MIRROR": + '"fcm:foo.xm/trunk@1"', + }, + template_vars, + ) -@pytest.fixture() -async def project_override( - rose_stem_run_template, setup_stem_repo -): - rose_stem_opts = { - 'stem_groups': ['earl_grey', 'milk,sugar', 'spoon,cup,milk'], - 'stem_sources': [ - f'bar={str(setup_stem_repo["workingcopy"])}', "fcm:foo.x_tr@head" - ], - 'workflow_name': setup_stem_repo['suitename'] - } - yield await rose_stem_run_template(rose_stem_opts) - - -@pytest.mark.parametrize( - 'expected', - [ - "run_ok", - ( - "RUN_NAMES=[\'earl_grey\', \'milk\', \'sugar\', " - "\'spoon\', \'cup\', \'milk\']" - ), - "SOURCE_FOO=\"fcm:foo.x_tr@head\"", - "HOST_SOURCE_FOO=\"fcm:foo.x_tr@head\"", - "SOURCE_BAR=\"{workingcopy}\"", - "HOST_SOURCE_BAR=\"{hostname}:{workingcopy}\"", - "SOURCE_FOO_BASE=\"fcm:foo.x_tr\"", - "HOST_SOURCE_FOO_BASE=\"fcm:foo.x_tr\"", - "SOURCE_BAR_BASE=\"{workingcopy}\"", - "HOST_SOURCE_BAR_BASE=\"{hostname}:{workingcopy}\"", - "SOURCE_FOO_REV=\"@1\"", - "SOURCE_BAR_REV=\"\"", - "SOURCE_FOO_MIRROR=\"fcm:foo.xm/trunk@1\"", - ] -) -def test_project_override(project_override, expected): - """Check that assorted variables have been exported. + +async def test_manual_project_override(rose_stem, rose_stem_project): + """It should accept named project sources. + + https://github.com/metomi/rose/blob/2c8956a9464bd277c8eb24d38af4803cba4c1243/t/rose-stem/00-run-basic.t#L80-L112 """ - if expected == 'run_ok': - assert project_override['run_stem'].returncode == 0 - else: - expected = expected.format( - workingcopy=project_override['workingcopy'], - hostname=HOST - ) - assert expected in project_override['jobout_content'] + template_vars = await rose_stem( + rose_stem_project, + stem_groups=["earl_grey", "milk,sugar", "spoon,cup,milk"], + stem_sources=[ + # specify a named source called "bar" + f'bar={rose_stem_project}', + "fcm:foo.x_tr@head", + ], + ) + check_template_variables( + { + "RUN_NAMES": + "['earl_grey', 'milk', 'sugar', " "'spoon', 'cup', 'milk']", + "SOURCE_FOO": '"fcm:foo.x_tr@head"', + "HOST_SOURCE_FOO": '"fcm:foo.x_tr@head"', + "SOURCE_BAR": f'"{rose_stem_project}"', + "HOST_SOURCE_BAR": f'"{HOST}:{rose_stem_project}"', + "SOURCE_FOO_BASE": '"fcm:foo.x_tr"', + "HOST_SOURCE_FOO_BASE": '"fcm:foo.x_tr"', + "SOURCE_BAR_BASE": f'"{rose_stem_project}"', + "HOST_SOURCE_BAR_BASE": f'"{HOST}:{rose_stem_project}"', + "SOURCE_FOO_REV": '"@1"', + "SOURCE_BAR_REV": '""', + "SOURCE_FOO_MIRROR": '"fcm:foo.xm/trunk@1"', + }, + template_vars, + ) -@pytest.fixture() -async def suite_redirection( - rose_stem_run_template, setup_stem_repo -): - rose_stem_opts = { - 'workflow_conf_dir': f'{setup_stem_repo["workingcopy"]}/rose-stem', - 'stem_groups': ['lapsang'], - 'stem_sources': ["fcm:foo.x_tr@head"], - 'workflow_name': setup_stem_repo['suitename'] - } - yield await rose_stem_run_template(rose_stem_opts) - - -@pytest.mark.parametrize( - 'expected', - [ - "run_ok", - "RUN_NAMES=[\'lapsang\']", - "SOURCE_FOO=\"fcm:foo.x_tr@head\"", - "SOURCE_FOO_BASE=\"fcm:foo.x_tr\"", - "SOURCE_FOO_REV=\"@1\"", - ] -) -def test_suite_redirection(suite_redirection, expected): - """Check that assorted variables have been exported. + +async def test_config_dir_absolute(rose_stem, rose_stem_project): + """It should allow you to specify the config directory as an absolute path. + + This is an alternative approach to running the "rose stem" command + in the directory itelf. + + https://github.com/metomi/rose/blob/2c8956a9464bd277c8eb24d38af4803cba4c1243/t/rose-stem/00-run-basic.t#L114-L128 """ - if expected == 'run_ok': - assert suite_redirection['run_stem'].returncode == 0 - else: - expected = expected.format( - workingcopy=suite_redirection['workingcopy'], - hostname=HOST - ) - assert expected in suite_redirection['jobout_content'] + template_vars = await rose_stem( + rose_stem_project, + workflow_conf_dir=f'{rose_stem_project}/rose-stem', + stem_groups=['lapsang'], + stem_sources=["fcm:foo.x_tr@head"], + # don't CD into the project directory first + cwd=Path.cwd(), + ) + check_template_variables( + { + "RUN_NAMES": "['lapsang']", + "SOURCE_FOO": '"fcm:foo.x_tr@head"', + "SOURCE_FOO_BASE": '"fcm:foo.x_tr"', + "SOURCE_FOO_REV": '"@1"', + }, + template_vars, + ) -@pytest.fixture() -async def subdirectory( - rose_stem_run_template, setup_stem_repo -): - rose_stem_opts = { - 'stem_groups': ['assam'], - 'stem_sources': [f'{setup_stem_repo["workingcopy"]}/rose-stem'], - 'workflow_name': setup_stem_repo['suitename'] - } - yield await rose_stem_run_template(rose_stem_opts) - - -@pytest.mark.parametrize( - 'expected', - [ - "run_ok", - "RUN_NAMES=[\'assam\']", - "SOURCE_FOO=\"{workingcopy}\"", - "HOST_SOURCE_FOO=\"{hostname}:{workingcopy}\"", - "SOURCE_FOO_BASE=\"{workingcopy}\"", - "HOST_SOURCE_FOO_BASE=\"{hostname}:{workingcopy}\"", - "SOURCE_FOO_REV=\"\"", - "SOURCE_FOO_MIRROR=\"fcm:foo.xm/trunk@1\"", - ] -) -def test_subdirectory(subdirectory, expected): - """Check that assorted variables have been exported. +async def test_config_dir_relative(rose_stem, rose_stem_project): + """It should allow you to specify the config directory as a relative path. + + This is an alternative approach to running the "rose stem" command + in the directory itelf. + + https://github.com/metomi/rose/blob/2c8956a9464bd277c8eb24d38af4803cba4c1243/t/rose-stem/00-run-basic.t#L152-L171 """ - if expected == 'run_ok': - assert subdirectory['run_stem'].returncode == 0 - else: - expected = expected.format( - workingcopy=subdirectory['workingcopy'], - hostname=HOST - ) - assert expected in subdirectory['jobout_content'] + template_vars = await rose_stem( + rose_stem_project, + workflow_conf_dir='./rose-stem', + stem_groups=['ceylon'], + ) + check_template_variables( + { + "RUN_NAMES": "['ceylon']", + "SOURCE_FOO": f'"{rose_stem_project}"', + "HOST_SOURCE_FOO": f'"{HOST}:{rose_stem_project}"', + "SOURCE_FOO_BASE": f'"{rose_stem_project}"', + "HOST_SOURCE_FOO_BASE": f'"{HOST}:{rose_stem_project}"', + "SOURCE_FOO_REV": '""', + }, + template_vars, + ) -@pytest.fixture() -async def relative_path( - rose_stem_run_template, setup_stem_repo -): - rose_stem_opts = { - 'workflow_conf_dir': './rose-stem', - 'stem_groups': ['ceylon'], - 'workflow_name': setup_stem_repo['suitename'] - } - yield await rose_stem_run_template(rose_stem_opts) - - -@pytest.mark.parametrize( - 'expected', - [ - "run_ok", - "RUN_NAMES=[\'ceylon\']", - "SOURCE_FOO=\"{workingcopy}\"", - "HOST_SOURCE_FOO=\"{hostname}:{workingcopy}\"", - "SOURCE_FOO_BASE=\"{workingcopy}\"", - "HOST_SOURCE_FOO_BASE=\"{hostname}:{workingcopy}\"", - "SOURCE_FOO_REV=\"\"", - ] -) -def test_relative_path(relative_path, expected): - """Check that assorted variables have been exported. +async def test_source_in_a_subdirectory(rose_stem, rose_stem_project): + """It should accept a source in a subdirectory. + + https://github.com/metomi/rose/blob/2c8956a9464bd277c8eb24d38af4803cba4c1243/t/rose-stem/00-run-basic.t#L130-L150 """ - if expected == 'run_ok': - assert relative_path['run_stem'].returncode == 0 - else: - expected = expected.format( - workingcopy=relative_path['workingcopy'], - hostname=HOST - ) - assert expected in relative_path['jobout_content'] + template_vars = await rose_stem( + rose_stem_project, + stem_groups=['assam'], + # stem source in a sub directory + stem_sources=[f'{rose_stem_project / "rose-stem"}'], + ) + check_template_variables( + { + "RUN_NAMES": "['assam']", + "SOURCE_FOO": f'"{rose_stem_project}"', + "HOST_SOURCE_FOO": f'"{HOST}:{rose_stem_project}"', + "SOURCE_FOO_BASE": f'"{rose_stem_project}"', + "HOST_SOURCE_FOO_BASE": f'"{HOST}:{rose_stem_project}"', + "SOURCE_FOO_REV": '""', + "SOURCE_FOO_MIRROR": '"fcm:foo.xm/trunk@1"', + }, + template_vars, + ) -@pytest.fixture() -async def with_config( - rose_stem_run_template, setup_stem_repo, mock_global_cfg +async def test_automatic_options( + rose_stem, + rose_stem_project, + mock_global_cfg, ): - """test for successful execution with site/user configuration - """ - rose_stem_opts = { - 'stem_groups': ['earl_grey', 'milk,sugar', 'spoon,cup,milk'], - 'stem_sources': [ - f'{setup_stem_repo["workingcopy"]}', 'fcm:foo.x_tr@head'], - 'workflow_name': setup_stem_repo['suitename'] - } + """It should use automatic options from the site/user configuration. + https://github.com/metomi/rose/blob/2c8956a9464bd277c8eb24d38af4803cba4c1243/t/rose-stem/00-run-basic.t#L182-L204 + """ mock_global_cfg( 'cylc.rose.stem.ResourceLocator.default', + # automatic options defined in the site/user config '[rose-stem]\nautomatic-options = MILK=true', ) - yield await rose_stem_run_template(rose_stem_opts) - - -def test_with_config(with_config): - """test for successful execution with site/user configuration - """ - assert with_config['run_stem'].returncode == 0 - for line in [ - "RUN_NAMES=['earl_grey', 'milk', 'sugar', 'spoon', 'cup', 'milk']", - 'SOURCE_FOO="{workingcopy} fcm:foo.x_tr@head"', - 'HOST_SOURCE_FOO="{hostname}:{workingcopy} fcm:foo.x_tr@head"', - 'SOURCE_FOO_BASE="{workingcopy}"', - 'HOST_SOURCE_FOO_BASE="{hostname}:{workingcopy}"', - 'SOURCE_FOO_REV=""', - 'MILK="true"', - ]: - line = line.format( - **with_config, - hostname=HOST - ) - assert line in with_config['jobout_content'] + template_vars = await rose_stem( + rose_stem_project, + stem_groups=['earl_grey', 'milk,sugar', 'spoon,cup,milk'], + stem_sources=[f'{rose_stem_project}', 'fcm:foo.x_tr@head'], + ) + check_template_variables( + { + "RUN_NAMES": + "['earl_grey', 'milk', 'sugar', 'spoon', 'cup', 'milk']", + "SOURCE_FOO": + f'"{rose_stem_project} fcm:foo.x_tr@head"', + "HOST_SOURCE_FOO": + f'"{HOST}:{rose_stem_project} fcm:foo.x_tr@head"', + "SOURCE_FOO_BASE": + f'"{rose_stem_project}"', + "HOST_SOURCE_FOO_BASE": + f'"{HOST}:{rose_stem_project}"', + "SOURCE_FOO_REV": + '""', + "MILK": + '"true"', + }, + template_vars, + ) -@pytest.fixture() -async def with_config2( - rose_stem_run_template, setup_stem_repo, mock_global_cfg +async def test_automatic_options_multi( + rose_stem, rose_stem_project, mock_global_cfg ): - """test for successful execution with site/user configuration + """It should use MULTIPLE automatic options from the site/user conf. + + https://github.com/metomi/rose/blob/2c8956a9464bd277c8eb24d38af4803cba4c1243/t/rose-stem/00-run-basic.t#L206-L221 """ - rose_stem_opts = { - 'stem_groups': ['assam'], - 'stem_sources': [ - f'{setup_stem_repo["workingcopy"]}'], - 'workflow_name': setup_stem_repo['suitename'] - } mock_global_cfg( 'cylc.rose.stem.ResourceLocator.default', + # *multiple* automatic options defined in the site/user config '[rose-stem]\nautomatic-options = MILK=true TEA=darjeeling', ) - yield await rose_stem_run_template(rose_stem_opts) + template_vars = await rose_stem( + rose_stem_project, + stem_groups=['assam'], + stem_sources=[f'{rose_stem_project}'], + ) + check_template_variables( + { + "MILK": '"true"', + "TEA": '"darjeeling"', + }, + template_vars, + ) -def test_with_config2(with_config2): - """test for successful execution with site/user configuration - """ - assert with_config2['run_stem'].returncode == 0 - for line in [ - 'MILK="true"', - 'TEA="darjeeling"', - ]: - line = line.format( - **with_config2, - hostname=HOST - ) - assert line in with_config2['jobout_content'] +async def test_incompatible_rose_stem_versions(rose_stem_project, rose_stem): + """It should fail if trying to install an incompatible rose-stem config. + Rose Stem configurations must specify a Rose Stem version, it should fail + if the versions don't match. -async def test_incompatible_versions( - setup_stem_repo, - monkeymodule, - caplog, - capsys, -): - """It fails if trying to install an incompatible version. + https://github.com/metomi/rose/blob/2c8956a9464bd277c8eb24d38af4803cba4c1243/t/rose-stem/00-run-basic.t#L223-L232 """ # Copy suite into working copy. test_src_dir = Path(__file__).parent / '12_rose_stem' src = str(test_src_dir / 'rose-suite2.conf') dest = str( - setup_stem_repo['workingcopy'] / 'rose-stem/rose-suite.conf' + rose_stem_project / 'rose-stem/rose-suite.conf' ) shutil.copy2(src, dest) - rose_stem_opts = { - 'stem_groups': ['earl_grey', 'milk,sugar', 'spoon,cup,milk'], - 'stem_sources': [ - str(setup_stem_repo['workingcopy']), - "fcm:foo.x_tr@head", - ], - 'workflow_name': str(setup_stem_repo['suitename']), - 'verbosity': 2, - } - - monkeymodule.setattr('sys.argv', ['stem']) - monkeymodule.chdir(setup_stem_repo['workingcopy']) - parser, opts = get_rose_stem_opts() - [setattr(opts, key, val) for key, val in rose_stem_opts.items()] - with pytest.raises( RoseStemVersionException, match='1 but suite is at version 0' ): - await rose_stem(parser, opts) + await rose_stem( + rose_stem_project, + stem_groups=['earl_grey', 'milk,sugar', 'spoon,cup,milk'], + stem_sources=[str(rose_stem_project), "fcm:foo.x_tr@head"], + verbosity=2, + ) -async def test_project_not_in_keywords(setup_stem_repo, monkeymodule, capsys): - """It fails if it cannot extract project name from FCM keywords. - """ +async def test_project_not_in_keywords( + rose_stem_project, + rose_stem, + monkeypatch, + capsys, +): + """It fails if it cannot extract project name from FCM keywords.""" # Copy suite into working copy. - monkeymodule.delenv('FCM_CONF_PATH') - rose_stem_opts = { - 'stem_groups': ['earl_grey', 'milk,sugar', 'spoon,cup,milk'], - 'stem_sources': [ - str(setup_stem_repo['workingcopy']), - "fcm:foo.x_tr@head", - ], - 'workflow_name': str(setup_stem_repo['suitename']) - } - - monkeymodule.setattr('sys.argv', ['stem']) - monkeymodule.chdir(setup_stem_repo['workingcopy']) - parser, opts = get_rose_stem_opts() - [setattr(opts, key, val) for key, val in rose_stem_opts.items()] - - await rose_stem(parser, opts) - + monkeypatch.delenv('FCM_CONF_PATH') + await rose_stem( + rose_stem_project, + stem_groups=['earl_grey', 'milk,sugar', 'spoon,cup,milk'], + stem_sources=[str(rose_stem_project), "fcm:foo.x_tr@head"], + ) assert 'ProjectNotFoundException' in capsys.readouterr().err -async def test_picks_template_section(setup_stem_repo, monkeymodule, capsys): - """It can cope with template variables section being either +async def test_picks_template_section(rose_stem_project, rose_stem, capsys): + """test error message when project not in keywords + + Note, it can cope with template variables section being either ``template variables`` or ``jinja2:suite.rc``. + + https://github.com/metomi/rose/blob/2c8956a9464bd277c8eb24d38af4803cba4c1243/t/rose-stem/00-run-basic.t#L234-L245 """ - monkeymodule.setattr('sys.argv', ['stem']) - monkeymodule.chdir(setup_stem_repo['workingcopy']) - (setup_stem_repo['workingcopy'] / 'rose-stem/rose-suite.conf').write_text( + (rose_stem_project / 'rose-stem/rose-suite.conf').write_text( 'ROSE_STEM_VERSION=1\n' '[template_variables]\n' ) - parser, opts = get_rose_stem_opts() - await rose_stem(parser, opts) + await rose_stem(rose_stem_project) _, err = capsys.readouterr() assert "[jinja2:suite.rc]' is deprecated" not in err From f33490d1f5aee5172128498817b64e2a05470cd8 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Thu, 2 May 2024 13:17:05 +0100 Subject: [PATCH 48/48] Update tests/functional/test_reinstall_overrides.py --- tests/functional/test_reinstall_overrides.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/test_reinstall_overrides.py b/tests/functional/test_reinstall_overrides.py index be108a35..4a0ff7f9 100644 --- a/tests/functional/test_reinstall_overrides.py +++ b/tests/functional/test_reinstall_overrides.py @@ -74,4 +74,4 @@ async def test_reinstall_overrides( file_poll(config_log) assert 'cylc message -- CLIreinstall' in config_log.read_text() - purge_workflow(wid) \ No newline at end of file + purge_workflow(wid)