diff --git a/changes.d/6102.fix.md b/changes.d/6102.fix.md new file mode 100644 index 00000000000..cbc8ecc2bd8 --- /dev/null +++ b/changes.d/6102.fix.md @@ -0,0 +1 @@ +Fixed bug introduced in 8.2.6 in `cylc vip` & `cylc vr` when using cylc-rose options (`-S`, `-D`, `-O`). diff --git a/cylc/flow/option_parsers.py b/cylc/flow/option_parsers.py index 6402641934e..9f416d0c0e8 100644 --- a/cylc/flow/option_parsers.py +++ b/cylc/flow/option_parsers.py @@ -33,7 +33,7 @@ import sys from textwrap import dedent -from typing import Any, Dict, Iterable, Optional, List, Tuple, Union +from typing import Any, Dict, Iterable, Optional, List, Set, Tuple, Union from cylc.flow import LOG from cylc.flow.terminal import supports_color, DIM @@ -62,7 +62,13 @@ class OptionSettings(): cylc.flow.option_parsers(thismodule).combine_options_pair. """ - def __init__(self, argslist, sources=None, useif=None, **kwargs): + def __init__( + self, + argslist: List[str], + sources: Optional[Set[str]] = None, + useif: str = '', + **kwargs + ): """Init function: Args: @@ -71,13 +77,10 @@ def __init__(self, argslist, sources=None, useif=None, **kwargs): useif: badge for use by Cylc optionparser. **kwargs: kwargs for optparse.option. """ - self.kwargs: Dict[str, str] = {} - self.sources: set = sources if sources is not None else set() - self.useif: str = useif if useif is not None else '' - - self.args: list[str] = argslist - for kwarg, value in kwargs.items(): - self.kwargs.update({kwarg: value}) + self.args: List[str] = argslist + self.kwargs: Dict[str, Any] = kwargs + self.sources: Set[str] = sources if sources is not None else set() + self.useif: str = useif def __eq__(self, other): """Args and Kwargs, but not other props equal. @@ -862,7 +865,7 @@ def cleanup_sysargv( # Get a list of unwanted args: unwanted_compound: List[str] = [] unwanted_simple: List[str] = [] - for unwanted_dest in (set(options.__dict__)) - set(script_opts_by_dest): + for unwanted_dest in set(options.__dict__) - set(script_opts_by_dest): for unwanted_arg in compound_opts_by_dest[unwanted_dest].args: if ( compound_opts_by_dest[unwanted_dest].kwargs.get('action', None) diff --git a/cylc/flow/scripts/validate_install_play.py b/cylc/flow/scripts/validate_install_play.py index 4ad66b5c454..5f3bdc45c2a 100644 --- a/cylc/flow/scripts/validate_install_play.py +++ b/cylc/flow/scripts/validate_install_play.py @@ -105,10 +105,7 @@ def main(parser: COP, options: 'Values', workflow_id: Optional[str] = None): workflow_id, options, compound_script_opts=VIP_OPTIONS, - script_opts=( - PLAY_OPTIONS + CYLC_ROSE_OPTIONS - + parser.get_std_options() - ), + script_opts=(*PLAY_OPTIONS, *parser.get_std_options()), source=orig_source, ) diff --git a/cylc/flow/scripts/validate_reinstall.py b/cylc/flow/scripts/validate_reinstall.py index 7a4472901ef..b82b7bfdc73 100644 --- a/cylc/flow/scripts/validate_reinstall.py +++ b/cylc/flow/scripts/validate_reinstall.py @@ -190,10 +190,7 @@ def vro_cli(parser: COP, options: 'Values', workflow_id: str): unparsed_wid, options, compound_script_opts=VR_OPTIONS, - script_opts=( - PLAY_OPTIONS + CYLC_ROSE_OPTIONS - + parser.get_std_options() - ), + script_opts=(*PLAY_OPTIONS, *parser.get_std_options()), source='', # Intentionally blank ) log_subcommand(*sys.argv[1:]) diff --git a/tests/unit/test_option_parsers.py b/tests/unit/test_option_parsers.py index 788aa391389..5a37d0bdc3d 100644 --- a/tests/unit/test_option_parsers.py +++ b/tests/unit/test_option_parsers.py @@ -322,7 +322,7 @@ def test_combine_options(inputs, expect): 'argv_before, kwargs, expect', [ param( - 'vip myworkflow -f something -b something_else --baz'.split(), + 'vip myworkflow -f something -b something_else --baz', { 'script_name': 'play', 'workflow_id': 'myworkflow', @@ -335,11 +335,11 @@ def test_combine_options(inputs, expect): OptionSettings(['--foo', '-f']), ] }, - 'play myworkflow -f something'.split(), + 'play myworkflow -f something', id='remove some opts' ), param( - 'vip myworkflow'.split(), + 'vip myworkflow', { 'script_name': 'play', 'workflow_id': 'myworkflow', @@ -350,11 +350,11 @@ def test_combine_options(inputs, expect): ], 'script_opts': [] }, - 'play myworkflow'.split(), + 'play myworkflow', id='no opts to keep' ), param( - 'vip ./myworkflow --foo something'.split(), + 'vip ./myworkflow --foo something', { 'script_name': 'play', 'workflow_id': 'myworkflow', @@ -365,11 +365,11 @@ def test_combine_options(inputs, expect): ], 'source': './myworkflow', }, - 'play --foo something myworkflow'.split(), + 'play --foo something myworkflow', id='replace path' ), param( - 'vip --foo something'.split(), + 'vip --foo something', { 'script_name': 'play', 'workflow_id': 'myworkflow', @@ -380,11 +380,11 @@ def test_combine_options(inputs, expect): ], 'source': './myworkflow', }, - 'play --foo something myworkflow'.split(), + 'play --foo something myworkflow', id='no path given' ), param( - 'vip -n myworkflow --no-run-name'.split(), + 'vip -n myworkflow --no-run-name', { 'script_name': 'play', 'workflow_id': 'myworkflow', @@ -396,17 +396,22 @@ def test_combine_options(inputs, expect): OptionSettings(['--not-used']), ] }, - 'play myworkflow'.split(), + 'play myworkflow', id='workflow-id-added' ), ] ) -def test_cleanup_sysargv(monkeypatch, argv_before, kwargs, expect): +def test_cleanup_sysargv( + monkeypatch: pytest.MonkeyPatch, + argv_before: str, + kwargs: dict, + expect: str +): """It replaces the contents of sysargv with Cylc Play argv items. """ # Fake up sys.argv: for this test. dummy_cylc_path = ['/pathto/my/cylc/bin/cylc'] - monkeypatch.setattr(sys, 'argv', dummy_cylc_path + argv_before) + monkeypatch.setattr(sys, 'argv', dummy_cylc_path + argv_before.split()) # Fake options too: opts = SimpleNamespace(**{ i.args[0].replace('--', ''): i for i in kwargs['compound_script_opts'] @@ -418,7 +423,7 @@ def test_cleanup_sysargv(monkeypatch, argv_before, kwargs, expect): # Test the script: cleanup_sysargv(**kwargs) - assert sys.argv == dummy_cylc_path + expect + assert sys.argv == dummy_cylc_path + expect.split() @pytest.mark.parametrize(