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', (