From 45e7b710a3497b2d6ee98c4e0bd72e79e29d57d2 Mon Sep 17 00:00:00 2001 From: Hilary Oliver Date: Mon, 10 Oct 2022 15:19:09 +1300 Subject: [PATCH 01/11] Scan workflow name during install. --- cylc/flow/scripts/install.py | 47 ++++++++++++++++++++++++++++++------ 1 file changed, 40 insertions(+), 7 deletions(-) diff --git a/cylc/flow/scripts/install.py b/cylc/flow/scripts/install.py index 761bc7dec23..bdf339fcc38 100755 --- a/cylc/flow/scripts/install.py +++ b/cylc/flow/scripts/install.py @@ -80,11 +80,19 @@ multiple workflow run directories that link to the same workflow definition. """ +from ansimarkup import ansiprint as cprint +import asyncio +from optparse import Values from pathlib import Path -from typing import Optional, TYPE_CHECKING, Dict, Any +from typing import Optional, Dict, Any +from cylc.flow.scripts.scan import ( + get_pipe, + _format_plain, +) from cylc.flow import iter_entry_points from cylc.flow.exceptions import PluginError, InputError +from cylc.flow.loggingutil import CylcLogFormatter from cylc.flow.option_parsers import CylcOptionParser as COP from cylc.flow.pathutil import EXPLICIT_RELATIVE_PATH_REGEX, expand_path from cylc.flow.workflow_files import ( @@ -92,9 +100,6 @@ ) from cylc.flow.terminal import cli_function -if TYPE_CHECKING: - from optparse import Values - def get_option_parser() -> COP: parser = COP( @@ -171,14 +176,40 @@ def get_source_location(path: Optional[str]) -> Path: return search_install_source_dirs(expanded_path) +async def scan(wf_name: str) -> None: + """Print any instances of wf_name that are already active.""" + opts = Values({ + 'name': [f'{wf_name}/*'], + 'states': {'running', 'paused', 'stopping'}, + 'source': False, + 'ping': False, + }) + active = [ + item async for item in get_pipe(opts, None, scan_dir=None) + ] + if active: + print( + CylcLogFormatter.COLORS['WARNING'].format( + f'Instance(s) of "{wf_name}" are already active:' + ) + ) + for item in active: + cprint( + _format_plain(item, opts) + ) + + @cli_function(get_option_parser) def main(parser, opts, reg=None): - install(parser, opts, reg) + wf_name = install(parser, opts, reg) + asyncio.run( + scan(wf_name) + ) def install( parser: COP, opts: 'Values', reg: Optional[str] = None -) -> None: +) -> str: if opts.no_run_name and opts.run_name: raise InputError( "options --no-run-name and --run-name are mutually exclusive." @@ -204,7 +235,7 @@ def install( elif opts.symlink_dirs: cli_symdirs = parse_cli_sym_dirs(opts.symlink_dirs) - source_dir, rundir, _workflow_name = install_workflow( + source_dir, rundir, workflow_name = install_workflow( source=source, workflow_name=opts.workflow_name, run_name=opts.run_name, @@ -229,3 +260,5 @@ def install( entry_point.name, exc ) from None + + return workflow_name From a7285d1b3b6c9f465693a64be1bad2b80f987684 Mon Sep 17 00:00:00 2001 From: Hilary James Oliver Date: Thu, 13 Oct 2022 00:04:07 +1300 Subject: [PATCH 02/11] Add integration test. --- cylc/flow/scripts/install.py | 75 +++++++++++++++++++++----- tests/integration/test_install.py | 88 +++++++++++++++++++++++++++++++ tests/integration/test_scan.py | 26 ++++++--- 3 files changed, 169 insertions(+), 20 deletions(-) create mode 100644 tests/integration/test_install.py diff --git a/cylc/flow/scripts/install.py b/cylc/flow/scripts/install.py index bdf339fcc38..0edfbb758ef 100755 --- a/cylc/flow/scripts/install.py +++ b/cylc/flow/scripts/install.py @@ -89,11 +89,16 @@ from cylc.flow.scripts.scan import ( get_pipe, _format_plain, + FLOW_STATE_SYMBOLS, + FLOW_STATE_CMAP ) from cylc.flow import iter_entry_points from cylc.flow.exceptions import PluginError, InputError from cylc.flow.loggingutil import CylcLogFormatter -from cylc.flow.option_parsers import CylcOptionParser as COP +from cylc.flow.option_parsers import ( + CylcOptionParser as COP, + Options +) from cylc.flow.pathutil import EXPLICIT_RELATIVE_PATH_REGEX, expand_path from cylc.flow.workflow_files import ( install_workflow, search_install_source_dirs, parse_cli_sym_dirs @@ -155,6 +160,16 @@ def get_option_parser() -> COP: default=False, dest="no_run_name") + parser.add_option( + "--no-ping", + help=( + "When scanning for active instances of the workflow, " + "do not attempt to contact the schedulers to get status." + ), + action="store_true", + default=False, + dest="no_ping") + parser.add_cylc_rose_options() return parser @@ -167,7 +182,7 @@ def get_source_location(path: Optional[str]) -> Path: """ if path is None: return Path.cwd() - path = path.strip() + path = str(path).strip() expanded_path = Path(expand_path(path)) if expanded_path.is_absolute(): return expanded_path @@ -176,39 +191,75 @@ def get_source_location(path: Optional[str]) -> Path: return search_install_source_dirs(expanded_path) -async def scan(wf_name: str) -> None: +async def scan(wf_name: str, ping: bool = True) -> None: """Print any instances of wf_name that are already active.""" opts = Values({ 'name': [f'{wf_name}/*'], 'states': {'running', 'paused', 'stopping'}, 'source': False, - 'ping': False, + 'ping': ping, # get status of scanned workflows }) active = [ item async for item in get_pipe(opts, None, scan_dir=None) ] if active: + n = len(active) + grammar = ( + ["s", "are", "them"] + if n > 1 else + ["", "is", "it"] + ) print( CylcLogFormatter.COLORS['WARNING'].format( - f'Instance(s) of "{wf_name}" are already active:' + f'WARNING: {n} run%s of "{wf_name}"' + ' %s already active:' % tuple(grammar[:2]) ) ) for item in active: - cprint( - _format_plain(item, opts) - ) + if opts.ping: + status = item['status'] + tag = FLOW_STATE_CMAP[status] + symbol = f" <{tag}>{FLOW_STATE_SYMBOLS[status]}" + else: + symbol = " " + cprint(symbol, _format_plain(item, opts)) + pattern = ( + f"'{wf_name}/*'" + if n > 1 else + f"{item['name']}" + ) + print( + f'You can stop %s with "cylc stop [options] {pattern}".\n' + 'See "cylc stop --help" for options.' % grammar[-1] + ) + + +InstallOptions = Options(get_option_parser()) @cli_function(get_option_parser) -def main(parser, opts, reg=None): - wf_name = install(parser, opts, reg) +def main( + _parser: COP, + opts: 'Values', + reg: Optional[str] = None +) -> None: + """CLI wrapper.""" + install_cli(opts, reg) + + +def install_cli( + opts: 'Values', + reg: Optional[str] = None +) -> None: + """Install workflow and scan for already-running instances.""" + wf_name = install(opts, reg) asyncio.run( - scan(wf_name) + scan(wf_name, not opts.no_ping) ) def install( - parser: COP, opts: 'Values', reg: Optional[str] = None + opts: 'Values', reg: Optional[str] = None ) -> str: if opts.no_run_name and opts.run_name: raise InputError( diff --git a/tests/integration/test_install.py b/tests/integration/test_install.py new file mode 100644 index 00000000000..f90f8bfc31b --- /dev/null +++ b/tests/integration/test_install.py @@ -0,0 +1,88 @@ +# THIS FILE IS PART OF 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 cylc install.""" + +from pathlib import Path +from shutil import rmtree +from tempfile import TemporaryDirectory + +import pytest + +from .test_scan import init_flows + +from cylc.flow.workflow_files import WorkflowFiles +from cylc.flow.scripts.install import ( + InstallOptions, + install_cli +) + + +SRV_DIR = Path(WorkflowFiles.Service.DIRNAME) +CONTACT = Path(WorkflowFiles.Service.CONTACT) +RUN_N = Path(WorkflowFiles.RUN_N) +INSTALL = Path(WorkflowFiles.Install.DIRNAME) + + +@pytest.fixture() +def src_run_dirs(mock_glbl_cfg, monkeypatch): + """Create some workflow source and run dirs for testing. + + Source dirs: + /w1 + /w2 + + Run dir: + /w1/run1 + + """ + tmp_src_path = Path(TemporaryDirectory().name) + tmp_run_path = Path(TemporaryDirectory().name) + tmp_src_path.mkdir() + tmp_run_path.mkdir() + + init_flows( + tmp_run_path=tmp_run_path, + running=('w1/run1',), + tmp_src_path=tmp_src_path, + src=('w1', 'w2') + ) + mock_glbl_cfg( + 'cylc.flow.workflow_files.glbl_cfg', + f''' + [install] + source dirs = {tmp_src_path} + ''' + ) + monkeypatch.setattr('cylc.flow.pathutil._CYLC_RUN_DIR', tmp_run_path) + + yield tmp_src_path, tmp_run_path + rmtree(tmp_src_path) + rmtree(tmp_run_path) + + +def test_install_scan(src_run_dirs, capsys): + """At install, any running intances should be reported.""" + + opts = InstallOptions() + # Don't ping the scheduler: it's not really running here. + opts.no_ping = True + + install_cli(opts, reg='w1') + assert '1 run of "w1" is already active:' in capsys.readouterr().out + + install_cli(opts, reg='w2') + assert '1 run of "w2" is already active:' not in capsys.readouterr().out diff --git a/tests/integration/test_scan.py b/tests/integration/test_scan.py index 79d8ac01475..f4c6837067d 100644 --- a/tests/integration/test_scan.py +++ b/tests/integration/test_scan.py @@ -42,17 +42,20 @@ INSTALL = Path(WorkflowFiles.Install.DIRNAME) -def init_flows(tmp_path, running=None, registered=None, un_registered=None): +def init_flows(tmp_run_path=None, running=None, registered=None, + un_registered=None, tmp_src_path=None, src=None): """Create some dummy workflows for scan to discover. Assume "run1, run2, ..., runN" structure if flow name constains "run". + Optionally create workflow source dirs in a give location too. + """ def make_registered(name, running=False): - run_d = Path(tmp_path, name) + run_d = Path(tmp_run_path, name) run_d.mkdir(parents=True, exist_ok=True) (run_d / "flow.cylc").touch() if "run" in name: - root = Path(tmp_path, name).parent + root = Path(tmp_run_path, name).parent with suppress(FileExistsError): (root / "runN").symlink_to(run_d, target_is_directory=True) else: @@ -63,12 +66,19 @@ def make_registered(name, running=False): if running: (srv_d / CONTACT).touch() + def make_src(name): + src_d = Path(tmp_src_path, name) + src_d.mkdir(parents=True, exist_ok=True) + (src_d / "flow.cylc").touch() + for name in (running or []): make_registered(name, running=True) for name in (registered or []): make_registered(name) for name in (un_registered or []): - Path(tmp_path, name).mkdir(parents=True, exist_ok=True) + Path(tmp_run_path, name).mkdir(parents=True, exist_ok=True) + for name in (src or []): + make_src(name) @pytest.fixture(scope='session') @@ -157,14 +167,14 @@ def source_dirs(mock_glbl_cfg): src1 = src / '1' src1.mkdir() init_flows( - src1, - registered=('a', 'b/c') + tmp_src_path=src1, + src=('a', 'b/c') ) src2 = src / '2' src2.mkdir() init_flows( - src2, - registered=('d', 'e/f') + tmp_src_path=src2, + src=('d', 'e/f') ) mock_glbl_cfg( 'cylc.flow.scripts.scan.glbl_cfg', From 5aa5fc0ca77a65e5670ce73dc7cc7098914f097b Mon Sep 17 00:00:00 2001 From: Hilary James Oliver Date: Tue, 18 Oct 2022 18:28:17 +1300 Subject: [PATCH 03/11] Apply suggestions from code review Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> --- cylc/flow/scripts/install.py | 2 +- tests/integration/test_install.py | 12 ++++-------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/cylc/flow/scripts/install.py b/cylc/flow/scripts/install.py index 0edfbb758ef..c983bbc0b92 100755 --- a/cylc/flow/scripts/install.py +++ b/cylc/flow/scripts/install.py @@ -211,7 +211,7 @@ async def scan(wf_name: str, ping: bool = True) -> None: ) print( CylcLogFormatter.COLORS['WARNING'].format( - f'WARNING: {n} run%s of "{wf_name}"' + f'NOTE: {n} run%s of "{wf_name}"' ' %s already active:' % tuple(grammar[:2]) ) ) diff --git a/tests/integration/test_install.py b/tests/integration/test_install.py index f90f8bfc31b..54a036cc2f6 100644 --- a/tests/integration/test_install.py +++ b/tests/integration/test_install.py @@ -17,8 +17,6 @@ """Test cylc install.""" from pathlib import Path -from shutil import rmtree -from tempfile import TemporaryDirectory import pytest @@ -38,7 +36,7 @@ @pytest.fixture() -def src_run_dirs(mock_glbl_cfg, monkeypatch): +def src_run_dirs(mock_glbl_cfg, monkeypatch, tmp_path: Path): """Create some workflow source and run dirs for testing. Source dirs: @@ -49,8 +47,8 @@ def src_run_dirs(mock_glbl_cfg, monkeypatch): /w1/run1 """ - tmp_src_path = Path(TemporaryDirectory().name) - tmp_run_path = Path(TemporaryDirectory().name) + tmp_src_path = tmp_path / 'cylc-src' + tmp_run_path = tmp_path / 'cylc-run' tmp_src_path.mkdir() tmp_run_path.mkdir() @@ -69,9 +67,7 @@ def src_run_dirs(mock_glbl_cfg, monkeypatch): ) monkeypatch.setattr('cylc.flow.pathutil._CYLC_RUN_DIR', tmp_run_path) - yield tmp_src_path, tmp_run_path - rmtree(tmp_src_path) - rmtree(tmp_run_path) + return tmp_src_path, tmp_run_path def test_install_scan(src_run_dirs, capsys): From 85df31a23d7f33d4f1ec3b17f54e2bb93cc114c4 Mon Sep 17 00:00:00 2001 From: Hilary Oliver Date: Thu, 20 Oct 2022 12:52:27 +1300 Subject: [PATCH 04/11] install: scan only target workflow --- cylc/flow/scripts/install.py | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/cylc/flow/scripts/install.py b/cylc/flow/scripts/install.py index c983bbc0b92..c157a49d4a8 100755 --- a/cylc/flow/scripts/install.py +++ b/cylc/flow/scripts/install.py @@ -99,9 +99,15 @@ CylcOptionParser as COP, Options ) -from cylc.flow.pathutil import EXPLICIT_RELATIVE_PATH_REGEX, expand_path +from cylc.flow.pathutil import ( + EXPLICIT_RELATIVE_PATH_REGEX, + expand_path, + get_workflow_run_dir +) from cylc.flow.workflow_files import ( - install_workflow, search_install_source_dirs, parse_cli_sym_dirs + install_workflow, + parse_cli_sym_dirs, + search_install_source_dirs ) from cylc.flow.terminal import cli_function @@ -200,12 +206,15 @@ async def scan(wf_name: str, ping: bool = True) -> None: 'ping': ping, # get status of scanned workflows }) active = [ - item async for item in get_pipe(opts, None, scan_dir=None) + item async for item in get_pipe( + opts, None, + scan_dir=get_workflow_run_dir(wf_name) # restricted scan + ) ] if active: n = len(active) grammar = ( - ["s", "are", "them"] + ["s", "are", "them all"] if n > 1 else ["", "is", "it"] ) @@ -229,8 +238,8 @@ async def scan(wf_name: str, ping: bool = True) -> None: f"{item['name']}" ) print( - f'You can stop %s with "cylc stop [options] {pattern}".\n' - 'See "cylc stop --help" for options.' % grammar[-1] + f'You can stop %s with:\n cylc stop {pattern}' + '\nSee "cylc stop --help" for options.' % grammar[-1] ) From 8ad9ceb85d3edb890ec0d17591c10974c69469c9 Mon Sep 17 00:00:00 2001 From: Hilary Oliver Date: Thu, 20 Oct 2022 12:52:44 +1300 Subject: [PATCH 05/11] Extend new test coverage. --- tests/integration/test_install.py | 57 +++++++++++++++++++++++++++---- 1 file changed, 50 insertions(+), 7 deletions(-) diff --git a/tests/integration/test_install.py b/tests/integration/test_install.py index 54a036cc2f6..a405c3460b5 100644 --- a/tests/integration/test_install.py +++ b/tests/integration/test_install.py @@ -16,12 +16,13 @@ """Test cylc install.""" -from pathlib import Path - import pytest +from pathlib import Path from .test_scan import init_flows +from cylc.flow.async_util import pipe +from cylc.flow.scripts import scan from cylc.flow.workflow_files import WorkflowFiles from cylc.flow.scripts.install import ( InstallOptions, @@ -70,15 +71,57 @@ def src_run_dirs(mock_glbl_cfg, monkeypatch, tmp_path: Path): return tmp_src_path, tmp_run_path -def test_install_scan(src_run_dirs, capsys): - """At install, any running intances should be reported.""" +INSTALLED_MSG = "INSTALLED {wfrun} from" +WF_ACTIVE_MSG = '1 run of "{wf}" is already active:' +BAD_CONTACT_MSG = "Bad contact file:" + + +def test_install_scan_no_ping(src_run_dirs, capsys, caplog): + """At install, running intances should be reported. + + Ping = False case: don't query schedulers. + """ opts = InstallOptions() - # Don't ping the scheduler: it's not really running here. opts.no_ping = True install_cli(opts, reg='w1') - assert '1 run of "w1" is already active:' in capsys.readouterr().out + out = capsys.readouterr().out + assert INSTALLED_MSG.format(wfrun='w1/run2') in out + assert WF_ACTIVE_MSG.format(wf='w1') in out + assert f"{BAD_CONTACT_MSG} w1/run1" in caplog.text + + install_cli(opts, reg='w2') + out = capsys.readouterr().out + assert WF_ACTIVE_MSG.format(wf='w2') not in out + assert INSTALLED_MSG.format(wfrun='w2/run1') in out + + +def test_install_scan_ping(src_run_dirs, capsys, caplog): + """At install, running intances should be reported. + + Ping = True case: but mock scan's scheduler query method. + """ + + @pipe + async def mock_graphql_query(flow, fields, filters=None): + """Mock cylc.flow.network.scan.graphql_query.""" + flow.update({"status": "running"}) + return flow + + scan.graphql_query = mock_graphql_query + + opts = InstallOptions() + opts.no_ping = False + + install_cli(opts, reg='w1') + out = capsys.readouterr().out + assert INSTALLED_MSG.format(wfrun='w1/run2') in out + assert WF_ACTIVE_MSG.format(wf='w1') in out + assert scan.FLOW_STATE_SYMBOLS["running"] in out + assert f"{BAD_CONTACT_MSG} w1/run1" in caplog.text install_cli(opts, reg='w2') - assert '1 run of "w2" is already active:' not in capsys.readouterr().out + out = capsys.readouterr().out + assert INSTALLED_MSG.format(wfrun='w2/run1') in out + assert WF_ACTIVE_MSG.format(wf='w2') not in out From fe35caa4ac8626e3398ea1d1f4bffb34233dc274 Mon Sep 17 00:00:00 2001 From: Hilary Oliver Date: Thu, 20 Oct 2022 13:45:55 +1300 Subject: [PATCH 06/11] Update change log. --- CHANGES.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 929f845f55d..bee5a27ef54 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -16,6 +16,9 @@ ones in. --> ### Enhancements +[#5284](https://github.com/cylc/cylc-flow/pull/5184) - scan for active +workflows at install time. + [#5032](https://github.com/cylc/cylc-flow/pull/5032) - set a default limit of 100 for the "default" queue. From 1bb56b9342faf8abc700826afca1054404fe1798 Mon Sep 17 00:00:00 2001 From: Hilary Oliver Date: Thu, 20 Oct 2022 14:31:09 +1300 Subject: [PATCH 07/11] Use a context manager in new test. --- tests/integration/test_install.py | 67 ++++++++++++++++++------------- 1 file changed, 40 insertions(+), 27 deletions(-) diff --git a/tests/integration/test_install.py b/tests/integration/test_install.py index a405c3460b5..d58dd0abeab 100644 --- a/tests/integration/test_install.py +++ b/tests/integration/test_install.py @@ -35,6 +35,31 @@ RUN_N = Path(WorkflowFiles.RUN_N) INSTALL = Path(WorkflowFiles.Install.DIRNAME) +INSTALLED_MSG = "INSTALLED {wfrun} from" +WF_ACTIVE_MSG = '1 run of "{wf}" is already active:' +BAD_CONTACT_MSG = "Bad contact file:" + + +class graphql_query_mock: + """Context manager to mock network.scan.graphql_query(). + + (In the scripts.scan namepace, where it is used below). + + """ + def __enter__(self): + """Replace the original function.""" + @pipe + async def mock_graphql_query(flow, fields, filters=None): + """The fake function.""" + flow.update({"status": "running"}) + return flow + self.orig_func = scan.graphql_query + scan.graphql_query = mock_graphql_query + + def __exit__(self, *args, **kwargs): + """Restore the original function.""" + scan.graphql_query = self.orig_func + @pytest.fixture() def src_run_dirs(mock_glbl_cfg, monkeypatch, tmp_path: Path): @@ -71,11 +96,6 @@ def src_run_dirs(mock_glbl_cfg, monkeypatch, tmp_path: Path): return tmp_src_path, tmp_run_path -INSTALLED_MSG = "INSTALLED {wfrun} from" -WF_ACTIVE_MSG = '1 run of "{wf}" is already active:' -BAD_CONTACT_MSG = "Bad contact file:" - - def test_install_scan_no_ping(src_run_dirs, capsys, caplog): """At install, running intances should be reported. @@ -103,25 +123,18 @@ def test_install_scan_ping(src_run_dirs, capsys, caplog): Ping = True case: but mock scan's scheduler query method. """ - @pipe - async def mock_graphql_query(flow, fields, filters=None): - """Mock cylc.flow.network.scan.graphql_query.""" - flow.update({"status": "running"}) - return flow - - scan.graphql_query = mock_graphql_query - - opts = InstallOptions() - opts.no_ping = False - - install_cli(opts, reg='w1') - out = capsys.readouterr().out - assert INSTALLED_MSG.format(wfrun='w1/run2') in out - assert WF_ACTIVE_MSG.format(wf='w1') in out - assert scan.FLOW_STATE_SYMBOLS["running"] in out - assert f"{BAD_CONTACT_MSG} w1/run1" in caplog.text - - install_cli(opts, reg='w2') - out = capsys.readouterr().out - assert INSTALLED_MSG.format(wfrun='w2/run1') in out - assert WF_ACTIVE_MSG.format(wf='w2') not in out + with graphql_query_mock(): + opts = InstallOptions() + opts.no_ping = False + + install_cli(opts, reg='w1') + out = capsys.readouterr().out + assert INSTALLED_MSG.format(wfrun='w1/run2') in out + assert WF_ACTIVE_MSG.format(wf='w1') in out + assert scan.FLOW_STATE_SYMBOLS["running"] in out + assert f"{BAD_CONTACT_MSG} w1/run1" in caplog.text + + install_cli(opts, reg='w2') + out = capsys.readouterr().out + assert INSTALLED_MSG.format(wfrun='w2/run1') in out + assert WF_ACTIVE_MSG.format(wf='w2') not in out From ba288039804a9e117d356e35b1f7b701d3c9d44e Mon Sep 17 00:00:00 2001 From: Hilary James Oliver Date: Fri, 21 Oct 2022 12:47:35 +1300 Subject: [PATCH 08/11] Update CHANGES.md [skip ci] Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> --- CHANGES.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index bee5a27ef54..b8d7b3ac993 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -16,8 +16,8 @@ ones in. --> ### Enhancements -[#5284](https://github.com/cylc/cylc-flow/pull/5184) - scan for active -workflows at install time. +[#5184](https://github.com/cylc/cylc-flow/pull/5184) - scan for active +runs of the same workflow at install time. [#5032](https://github.com/cylc/cylc-flow/pull/5032) - set a default limit of 100 for the "default" queue. From 23cb4d6bc9eae93e797f617444e36cb513900f77 Mon Sep 17 00:00:00 2001 From: Hilary Oliver Date: Fri, 21 Oct 2022 12:51:06 +1300 Subject: [PATCH 09/11] Add explanatory comments. [skip ci] --- tests/integration/test_install.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/integration/test_install.py b/tests/integration/test_install.py index d58dd0abeab..78eccf103b3 100644 --- a/tests/integration/test_install.py +++ b/tests/integration/test_install.py @@ -109,6 +109,7 @@ def test_install_scan_no_ping(src_run_dirs, capsys, caplog): out = capsys.readouterr().out assert INSTALLED_MSG.format(wfrun='w1/run2') in out assert WF_ACTIVE_MSG.format(wf='w1') in out + # Empty contact file faked with "touch": assert f"{BAD_CONTACT_MSG} w1/run1" in caplog.text install_cli(opts, reg='w2') @@ -132,6 +133,7 @@ def test_install_scan_ping(src_run_dirs, capsys, caplog): assert INSTALLED_MSG.format(wfrun='w1/run2') in out assert WF_ACTIVE_MSG.format(wf='w1') in out assert scan.FLOW_STATE_SYMBOLS["running"] in out + # Empty contact file faked with "touch": assert f"{BAD_CONTACT_MSG} w1/run1" in caplog.text install_cli(opts, reg='w2') From e00cfe23f225966a41ba9f5b4d4850bfef5f8162 Mon Sep 17 00:00:00 2001 From: Hilary Oliver Date: Fri, 21 Oct 2022 13:36:12 +1300 Subject: [PATCH 10/11] Switch to better monkeypatching approach. [skip ci] --- tests/integration/test_install.py | 83 +++++++++++++++++-------------- 1 file changed, 45 insertions(+), 38 deletions(-) diff --git a/tests/integration/test_install.py b/tests/integration/test_install.py index 78eccf103b3..c3eb2525908 100644 --- a/tests/integration/test_install.py +++ b/tests/integration/test_install.py @@ -40,29 +40,29 @@ BAD_CONTACT_MSG = "Bad contact file:" -class graphql_query_mock: - """Context manager to mock network.scan.graphql_query(). - - (In the scripts.scan namepace, where it is used below). - - """ - def __enter__(self): - """Replace the original function.""" - @pipe - async def mock_graphql_query(flow, fields, filters=None): - """The fake function.""" - flow.update({"status": "running"}) - return flow - self.orig_func = scan.graphql_query - scan.graphql_query = mock_graphql_query - - def __exit__(self, *args, **kwargs): - """Restore the original function.""" - scan.graphql_query = self.orig_func +@pytest.fixture() +def patch_graphql_query( + monkeypatch +): + # Define a mocked graphql_query pipe function. + @pipe + async def _graphql_query(flow, fields, filters=None): + flow.update({"status": "running"}) + return flow + + # Swap out the function that cylc.flow.scripts.scan. + monkeypatch.setattr( + 'cylc.flow.scripts.scan.graphql_query', + _graphql_query, + ) @pytest.fixture() -def src_run_dirs(mock_glbl_cfg, monkeypatch, tmp_path: Path): +def src_run_dirs( + mock_glbl_cfg, + monkeypatch, + tmp_path: Path +): """Create some workflow source and run dirs for testing. Source dirs: @@ -96,7 +96,11 @@ def src_run_dirs(mock_glbl_cfg, monkeypatch, tmp_path: Path): return tmp_src_path, tmp_run_path -def test_install_scan_no_ping(src_run_dirs, capsys, caplog): +def test_install_scan_no_ping( + src_run_dirs, + capsys, + caplog +): """At install, running intances should be reported. Ping = False case: don't query schedulers. @@ -118,25 +122,28 @@ def test_install_scan_no_ping(src_run_dirs, capsys, caplog): assert INSTALLED_MSG.format(wfrun='w2/run1') in out -def test_install_scan_ping(src_run_dirs, capsys, caplog): +def test_install_scan_ping( + src_run_dirs, + capsys, + caplog, + patch_graphql_query +): """At install, running intances should be reported. Ping = True case: but mock scan's scheduler query method. """ + opts = InstallOptions() + opts.no_ping = False + + install_cli(opts, reg='w1') + out = capsys.readouterr().out + assert INSTALLED_MSG.format(wfrun='w1/run2') in out + assert WF_ACTIVE_MSG.format(wf='w1') in out + assert scan.FLOW_STATE_SYMBOLS["running"] in out + # Empty contact file faked with "touch": + assert f"{BAD_CONTACT_MSG} w1/run1" in caplog.text - with graphql_query_mock(): - opts = InstallOptions() - opts.no_ping = False - - install_cli(opts, reg='w1') - out = capsys.readouterr().out - assert INSTALLED_MSG.format(wfrun='w1/run2') in out - assert WF_ACTIVE_MSG.format(wf='w1') in out - assert scan.FLOW_STATE_SYMBOLS["running"] in out - # Empty contact file faked with "touch": - assert f"{BAD_CONTACT_MSG} w1/run1" in caplog.text - - install_cli(opts, reg='w2') - out = capsys.readouterr().out - assert INSTALLED_MSG.format(wfrun='w2/run1') in out - assert WF_ACTIVE_MSG.format(wf='w2') not in out + install_cli(opts, reg='w2') + out = capsys.readouterr().out + assert INSTALLED_MSG.format(wfrun='w2/run1') in out + assert WF_ACTIVE_MSG.format(wf='w2') not in out From d64c0d9cb0e66b9d78aae70a7ef2f27c0c2aa14d Mon Sep 17 00:00:00 2001 From: Hilary Oliver Date: Fri, 21 Oct 2022 14:29:54 +1300 Subject: [PATCH 11/11] Add type hints. --- tests/integration/test_install.py | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/tests/integration/test_install.py b/tests/integration/test_install.py index c3eb2525908..22ff88a55ba 100644 --- a/tests/integration/test_install.py +++ b/tests/integration/test_install.py @@ -29,6 +29,7 @@ install_cli ) +from typing import Callable, Tuple SRV_DIR = Path(WorkflowFiles.Service.DIRNAME) CONTACT = Path(WorkflowFiles.Service.CONTACT) @@ -42,7 +43,7 @@ @pytest.fixture() def patch_graphql_query( - monkeypatch + monkeypatch: pytest.MonkeyPatch ): # Define a mocked graphql_query pipe function. @pipe @@ -59,10 +60,10 @@ async def _graphql_query(flow, fields, filters=None): @pytest.fixture() def src_run_dirs( - mock_glbl_cfg, - monkeypatch, + mock_glbl_cfg: Callable, + monkeypatch: pytest.MonkeyPatch, tmp_path: Path -): +) -> Tuple[Path, Path]: """Create some workflow source and run dirs for testing. Source dirs: @@ -97,10 +98,10 @@ def src_run_dirs( def test_install_scan_no_ping( - src_run_dirs, - capsys, - caplog -): + src_run_dirs: Callable, + capsys: pytest.CaptureFixture, + caplog: pytest.LogCaptureFixture +) -> None: """At install, running intances should be reported. Ping = False case: don't query schedulers. @@ -123,11 +124,11 @@ def test_install_scan_no_ping( def test_install_scan_ping( - src_run_dirs, - capsys, - caplog, - patch_graphql_query -): + src_run_dirs: Callable, + capsys: pytest.CaptureFixture, + caplog: pytest.LogCaptureFixture, + patch_graphql_query: Callable +) -> None: """At install, running intances should be reported. Ping = True case: but mock scan's scheduler query method.