diff --git a/doc/changelog.d/3678.added.md b/doc/changelog.d/3678.added.md new file mode 100644 index 0000000000..dd6b7532ad --- /dev/null +++ b/doc/changelog.d/3678.added.md @@ -0,0 +1 @@ +refactor: make cli testing not depending on MAPDL. \ No newline at end of file diff --git a/src/ansys/mapdl/core/cli/list_instances.py b/src/ansys/mapdl/core/cli/list_instances.py index 6256996848..7ad94d1bf1 100644 --- a/src/ansys/mapdl/core/cli/list_instances.py +++ b/src/ansys/mapdl/core/cli/list_instances.py @@ -36,7 +36,7 @@ flag_value=True, type=bool, default=False, - help="Print only instances", + help="Do not print the child process, only the main processes (instances).", ) @click.option( "--long", @@ -72,6 +72,15 @@ def list_instances(instances, long, cmd, location) -> None: # Assuming all ansys processes have -grpc flag mapdl_instances = [] + def is_grpc_based(proc): + cmdline = proc.cmdline() + return "-grpc" in cmdline + + def get_port(proc): + cmdline = proc.cmdline() + ind_grpc = cmdline.index("-port") + return cmdline[ind_grpc + 1] + def is_valid_process(proc): valid_status = proc.status() in [ psutil.STATUS_RUNNING, @@ -81,12 +90,7 @@ def is_valid_process(proc): valid_ansys_process = ("ansys" in proc.name().lower()) or ( "mapdl" in proc.name().lower() ) - # Early exit to avoid checking 'cmdline' of a protected process (raises psutil.AccessDenied) - if not valid_ansys_process: - return False - - grpc_is_active = "-grpc" in proc.cmdline() - return valid_status and valid_ansys_process and grpc_is_active + return valid_status and valid_ansys_process and is_grpc_based(proc) for proc in psutil.process_iter(): # Check if the process is running and not suspended @@ -104,8 +108,6 @@ def is_valid_process(proc): continue # printing - table = [] - if long: cmd = True location = True @@ -120,18 +122,10 @@ def is_valid_process(proc): if location: headers.append("Working directory") - def get_port(proc): - cmdline = proc.cmdline() - ind_grpc = cmdline.index("-port") - return cmdline[ind_grpc + 1] - - def is_grpc_based(proc): - cmdline = proc.cmdline() - return "-grpc" in cmdline - table = [] for each_p in mapdl_instances: - if not each_p.ansys_instance or not is_grpc_based(each_p): + if instances and not each_p.ansys_instance: + # Skip child processes if only printing instances continue proc_line = [] diff --git a/src/ansys/mapdl/core/cli/start.py b/src/ansys/mapdl/core/cli/start.py index c34b3e7bbc..e038333673 100644 --- a/src/ansys/mapdl/core/cli/start.py +++ b/src/ansys/mapdl/core/cli/start.py @@ -281,7 +281,6 @@ def start( additional_switches=additional_switches, start_timeout=start_timeout, port=port, - license_server_check=license_server_check, license_type=license_type, version=version, ) diff --git a/src/ansys/mapdl/core/cli/stop.py b/src/ansys/mapdl/core/cli/stop.py index e49ebbff0c..c3513683df 100644 --- a/src/ansys/mapdl/core/cli/stop.py +++ b/src/ansys/mapdl/core/cli/stop.py @@ -68,8 +68,6 @@ def stop(port: int, pid: Optional[int], all: bool) -> None: """ import psutil - from ansys.mapdl.core.launcher import is_ansys_process - PROCESS_OK_STATUS = [ # List of all process status, comment out the ones that means that # process is not OK. @@ -97,15 +95,11 @@ def stop(port: int, pid: Optional[int], all: bool) -> None: killed_ = False for proc in psutil.process_iter(): try: - if ( - psutil.pid_exists(proc.pid) - and proc.status() in PROCESS_OK_STATUS - and is_ansys_process(proc) - ): + if _is_valid_ansys_process(PROCESS_OK_STATUS, proc): # Killing "all" if all: try: - proc.kill() + _kill_process(proc) killed_ = True except psutil.NoSuchProcess: pass @@ -114,7 +108,7 @@ def stop(port: int, pid: Optional[int], all: bool) -> None: # Killing by ports if str(port) in proc.cmdline(): try: - proc.kill() + _kill_process(proc) killed_ = True except psutil.NoSuchProcess: pass @@ -154,8 +148,9 @@ def stop(port: int, pid: Optional[int], all: bool) -> None: p = psutil.Process(pid) for child in p.children(recursive=True): - child.kill() - p.kill() + _kill_process(child) + + _kill_process(p) if p.status == "running": click.echo( @@ -168,3 +163,19 @@ def stop(port: int, pid: Optional[int], all: bool) -> None: + f"The process with PID {pid} and its children have been stopped." ) return + + +def _kill_process(proc): + proc.kill() + + +def _is_valid_ansys_process(PROCESS_OK_STATUS, proc): + import psutil + + from ansys.mapdl.core.launcher import is_ansys_process + + return ( + psutil.pid_exists(proc.pid) + and proc.status() in PROCESS_OK_STATUS + and is_ansys_process(proc) + ) diff --git a/tests/test_cli.py b/tests/test_cli.py index a59b957038..41e03705c5 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -22,7 +22,9 @@ import re import subprocess +from unittest.mock import MagicMock, patch +import numpy as np import psutil import pytest @@ -34,6 +36,26 @@ PORT1 = 50090 +def make_fake_process(pid, name, port=PORT1, ansys_process=False, n_children=0): + mock_process = MagicMock(spec=psutil.Process) + mock_process.pid = pid + mock_process.name.return_value = name + mock_process.status.return_value = psutil.STATUS_RUNNING + mock_process.children.side_effect = lambda *arg, **kwargs: [ + i for i in range(n_children) + ] + mock_process.cwd.return_value = f"/cwd/of/{name}" + + if ansys_process: + mock_process.cmdline.return_value = ( + f"/ansys_inc/v251/ansys/bin/ansys251 -grpc -port {port}".split(" ") + ) + else: + mock_process.cmdline.return_value = f"/path/to/process".split(" ") + + return mock_process + + @pytest.fixture @requires("click") @requires("nostudent") @@ -44,7 +66,7 @@ def do_run(arguments=""): from ansys.mapdl.core.cli import main if arguments: - args = list(arguments.split(" ")) + args = list(arguments.strip().split(" ")) else: args = [] @@ -58,8 +80,6 @@ def do_run(arguments=""): @requires("click") -@requires("local") -@requires("nostudent") @pytest.mark.parametrize("start_instance", [None, True, False]) def test_launch_mapdl_cli(monkeypatch, run_cli, start_instance): if start_instance is not None: @@ -67,25 +87,188 @@ def test_launch_mapdl_cli(monkeypatch, run_cli, start_instance): else: monkeypatch.delenv("PYMAPDL_START_INSTANCE", raising=False) - # Setting a port so it does not collide with the already running instance for testing - output = run_cli(f"start --port {PORT1}") + with ( + patch("ansys.mapdl.core.launcher.launch_mapdl") as mock_launch, + patch("ansys.mapdl.core.launcher.submitter") as mock_submitter, + ): # test we are not calling Popen + + mock_launch.side_effect = lambda *args, **kwargs: ( + "123.45.67.89", + str(PORT1), + "123245", + ) + + # Setting a port so it does not collide with the already running instance for testing + output = run_cli(f"start --port {PORT1}") assert "Success: Launched an MAPDL instance " in output assert str(PORT1) in output # grab ips and port + pid = re.search(r"\(PID=(\d+)\)", output) + assert pid pid = int(re.search(r"\(PID=(\d+)\)", output).groups()[0]) + assert pid != 0 + + +@pytest.mark.parametrize( + "mapping", + ((False, True), (False, True, False), (False, False, False), (True, True, False)), +) +def test_pymapdl_stop_instances(run_cli, mapping): + + fake_process_ = [ + { + "pid": np.random.randint(10000, 100000), + "name": f"ansys251_{ind}" if each else "process", + "port": str(50052 + ind), + "ansys_process": each, + } + for ind, each in enumerate(mapping) + ] - output = run_cli(f"stop --port {PORT1}") - assert "success" in output.lower() + fake_processes = [make_fake_process(**each) for each in fake_process_] + + with ( + patch("ansys.mapdl.core.cli.stop._kill_process") as mock_kill, + patch("psutil.pid_exists") as mock_pid, + patch("psutil.process_iter", return_value=iter(fake_processes)), + ): + + mock_pid.return_value = lambda *args, **kwargs: True # All process exists + mock_kill.side_effect = lambda *args, **kwargs: None # avoid kill nothing + + if sum(mapping) == 0: + output = run_cli(f"stop --port {PORT1}") + assert ( + f"error: no ansys instances running on port {PORT1}" in output.lower() + ) + mock_kill.assert_not_called() + + output = run_cli(f"stop --all") + assert f"error: no ansys instances have been found." in output.lower() + mock_kill.assert_not_called() + + elif sum(mapping) == 1: + # Port + process, process_mock = [ + (each, each_mock) + for each, each_mock in zip(fake_process_, fake_processes) + if "ansys251" in each["name"] + ][0] + port = process["port"] + + output = run_cli(f"stop --port {port}") + assert ( + f"success: ansys instances running on port {port} have been stopped" + in output.lower() + ) + + # PID + pid = process["pid"] + with patch("psutil.Process") as mock_process: + mock_process.return_value = process_mock + + output = run_cli(f"stop --pid {pid}") + assert ( + f"the process with pid {pid} and its children have been stopped." + in output.lower() + ) + + mock_kill.assert_called() + assert mock_kill.call_count == 2 + + else: + output = run_cli(f"stop --all") + assert "success: ansys instances have been stopped." in output.lower() + assert mock_kill.call_count == sum(mapping) @requires("click") -@requires("local") -@requires("nostudent") -def test_launch_mapdl_cli_config(run_cli): - cmds_ = ["start", f"--port {PORT1}", "--jobname myjob"] - cmd_warnings = [ +@pytest.mark.parametrize( + "arg,check", + ( + ("", {}), # default + ("-i", {"Is Instance": False}), + ("-c", {"Command line": True}), + ("-l", {"Command line": True, "Working directory": True}), + ("-cwd", {"Working directory": True}), + ("--instances", {"Is Instance": False}), + ("--cmd", {"Command line": True}), + ("--long", {"Command line": True, "Working directory": True}), + ("--location", {"Working directory": True}), + ), +) +@patch("psutil.pid_exists", lambda *args, **kwargs: True) +def test_launch_mapdl_cli_list(run_cli, arg, check): + + mapping = (False, True, False, True, False, False) + is_instance = (False, False, False, True, False, False) + + fake_process_ = [ + { + "pid": np.random.randint(10000, 100000), + "name": f"ansys251_{ind}" if each else "process", + "port": str(50052 + ind), + "ansys_process": each, + "n_children": 4 if each_ins else 1, + } + for ind, (each, each_ins) in enumerate(zip(mapping, is_instance)) + ] + + fake_processes = [make_fake_process(**each) for each in fake_process_] + + checks_defaults = { + "Is Instance": True, + "Command line": False, + "Working directory": False, + } + + with patch("psutil.process_iter", return_value=iter(fake_processes)): + output = run_cli(" ".join(["list", arg])) + + assert "running" in output or "sleeping" in output + checks_defaults.update(check) + + for each, value in checks_defaults.items(): + if value: + assert each in output + else: + assert each not in output + + assert len(output.splitlines()) > 2 + + if arg in ["-c", "--cmd"]: + assert "/ansys_inc/v251/ansys/bin/ansys251" in output + + if arg in ["-cwd", "--location"]: + assert "/cwd/of/ansys251" in output + + if arg in ["-i", "--instances"]: + assert len(output.splitlines()) == sum(is_instance) + 2 + else: + assert len(output.splitlines()) == sum(mapping) + 2 + + if arg in ["-l", "--long"]: + assert "/ansys_inc/v251/ansys/bin/ansys251" in output.lower() + + for ind, (each, each_ins) in enumerate(zip(mapping, is_instance)): + proc_options = fake_process_[ind] + # is ansys + if not each or (not each_ins and arg in ["-i", "--instances"]): + assert proc_options["name"] not in output + assert str(proc_options["pid"]) not in output + assert proc_options["port"] not in output + else: + assert proc_options["name"] in output + assert str(proc_options["pid"]) in output + assert proc_options["port"] in output + + +@requires("click") +@pytest.mark.parametrize( + "arg", + [ "ip", "license_server_check", "mode", @@ -98,93 +281,35 @@ def test_launch_mapdl_cli_config(run_cli): "print_com", "add_env_vars", "replace_env_vars", - ] - - cmd = " ".join(cmds_) - cmd_warnings_ = ["--" + each + " True" for each in cmd_warnings] - - cmd = cmd + " " + " ".join(cmd_warnings_) + ], +) +def test_launch_mapdl_cli_config(run_cli, arg): + cmd = " ".join(["start", f"--port {PORT1}", "--jobname myjob", f"--{arg} True"]) + + with ( + patch("ansys.mapdl.core.launcher.launch_mapdl") as mock_launch, + patch("ansys.mapdl.core.launcher.submitter") as mock_submitter, + ): # test we are not calling Popen + mock_launch.side_effect = lambda *args, **kwargs: ( + "123.45.67.89", + str(PORT1), + "123245", + ) - try: output = run_cli(cmd) + mock_submitter.assert_not_called() + kwargs = mock_launch.call_args_list[0].kwargs + assert str(kwargs["port"]) == str(PORT1) + assert "Launched an MAPDL instance" in output assert str(PORT1) in output # assert warnings - for each in cmd_warnings: - assert ( - f"The following argument is not allowed in CLI: '{each}'" in output - ), f"Warning about '{each}' not printed" - - # grab ips and port - pid = int(re.search(r"\(PID=(\d+)\)", output).groups()[0]) - p = psutil.Process(pid) - cmdline = " ".join(p.cmdline()) - - assert str(PORT1) in cmdline - assert "myjob" in cmdline - - finally: - output = run_cli(f"stop --port {PORT1}") - assert "Success" in output + assert arg not in kwargs assert ( - f"Success: Ansys instances running on port {PORT1} have been stopped" - in output - ) - - -@requires("click") -@requires("local") -@requires("nostudent") -@pytest.mark.xfail(reason="Flaky test. See #2435") -def test_launch_mapdl_cli_list(run_cli): - - output = run_cli(f"start --port {PORT1}") - - assert "Success: Launched an MAPDL instance " in output - assert str(PORT1) in output - - output = run_cli("list") - assert "running" in output or "sleeping" in output - assert "Is Instance" in output - assert len(output.splitlines()) > 2 - assert "ansys" in output.lower() or "mapdl" in output.lower() - - output = run_cli("list -i") - assert "running" in output or "sleeping" in output - assert "Is Instance" not in output - assert len(output.splitlines()) > 2 - assert "ansys" in output.lower() or "mapdl" in output.lower() - - output = run_cli("list -c") - assert "running" in output or "sleeping" in output - assert "Command line" in output - assert "Is Instance" in output - assert len(output.splitlines()) > 2 - assert "ansys" in output.lower() or "mapdl" in output.lower() - - output = run_cli("list -cwd") - assert "running" in output or "sleeping" in output - assert "Command line" not in output - assert "Working directory" in output - assert "Is Instance" in output - assert len(output.splitlines()) > 2 - assert "ansys" in output.lower() or "mapdl" in output.lower() - - output = run_cli("list -l") - assert "running" in output or "sleeping" in output - assert "Is Instance" in output - assert "Command line" in output - assert len(output.splitlines()) > 2 - assert "ansys" in output.lower() or "mapdl" in output.lower() - - output = run_cli(f"stop --port {PORT1}") - assert "Success" in output - assert str(PORT1) in output - assert ( - f"Success: Ansys instances running on port {PORT1} have been stopped" in output - ) + f"The following argument is not allowed in CLI: '{arg}'" in output + ), f"Warning about '{arg}' not printed" @requires("click")