Fix extension ignoring between Windows and Linux

Signed-off-by: sweeneys <sweeneys@amazon.com>

Tools/LyTestTools/ly_test_tools/environment/process_utils.py

@@ -31,45 +31,59 @@ def kill_processes_named(names, ignore_extensions=False):
     Kills all processes with a given name
 
     :param names: string process name, or list of strings of process name
-    :param ignore_extensions: ignore trailing file extension
+    :param ignore_extensions: ignore trailing file extensions. By default 'abc.exe' will not match 'abc'. Note that
+        enabling this will cause 'abc.exe' to match 'abc', 'abc.bat', and 'abc.sh', though 'abc.GameLauncher.exe'
+        will not match 'abc.DedicatedServer'
     """
     if not names:
         return
 
-    names = [names] if isinstance(names, str) else names
+    name_set = set()
+    if isinstance(names, str):
+        name_set.add(names)
+    else:
+        name_set.update(names)
 
     if ignore_extensions:
-        names = [_remove_extension(name) for name in names]
+        # both exact matches and extensionless
+        stripped_names = set()
+        for name in name_set:
+            stripped_names.add(_remove_extension(name))
+        name_set.update(stripped_names)
 
     # remove any blank names, which may empty the list
-    names = list(filter(lambda x: not x.isspace(), names))
-    if not names:
+    name_set = set(filter(lambda x: not x.isspace(), name_set))
+    if not name_set:
         return
 
-    logger.info(f"Killing all processes named {names}")
-    process_list_to_kill = []
+    logger.info(f"Killing all processes named {name_set}")
+    process_set_to_kill = set()
     for process in _safe_get_processes(['name', 'pid']):
         try:
             proc_name = process.name()
         except psutil.AccessDenied:
-            logger.info(f"Process {process} permissions error during kill_processes_named()", exc_info=True)
+            logger.warning(f"Process {process} permissions error during kill_processes_named()", exc_info=True)
             continue
         except psutil.ProcessLookupError:
-            logger.debug(f"Process {process} could not be killed during kill_processes_named() and was likely already stopped", exc_info=True)
+            logger.debug(f"Process {process} could not be killed during kill_processes_named() and was likely already "
+                         f"stopped", exc_info=True)
             continue
         except psutil.NoSuchProcess:
             logger.debug(f"Process '{process}' was active when list of processes was requested but it was not found "
                          f"during kill_processes_named()", exc_info=True)
             continue
 
+        if proc_name in name_set:
+            logger.debug(f"Found process with name {proc_name}.")
+            process_set_to_kill.add(process)
+
         if ignore_extensions:
-            proc_name = _remove_extension(proc_name)
+            extensionless_name = _remove_extension(proc_name)
+            if extensionless_name in name_set:
+                process_set_to_kill.add(process)
 
-        if proc_name in names:
-            logger.debug(f"Found process with name {proc_name}.  Attempting to kill...")
-            process_list_to_kill.append(process)
-
-    _safe_kill_process_list(process_list_to_kill)
+    if process_set_to_kill:
+        _safe_kill_processes(process_set_to_kill)
 
 
 def kill_processes_started_from(path):
@@ -90,7 +104,7 @@ def kill_processes_started_from(path):
             if process_path.lower().startswith(path.lower()):
                 process_list.append(process)
 
-        _safe_kill_process_list(process_list)
+        _safe_kill_processes(process_list)
     else:
         logger.warning(f"Path:'{path}' not found")
 
@@ -118,7 +132,7 @@ def kill_processes_with_name_not_started_from(name, path):
                 logger.info("%s -> %s" % (os.path.dirname(process_path.lower()), path))
                 proccesses_to_kill.append(process)
 
-        _safe_kill_process_list(proccesses_to_kill)
+        _safe_kill_processes(proccesses_to_kill)
     else:
         logger.warning(f"Path:'{path}' not found")
 
@@ -151,10 +165,12 @@ def process_exists(name, ignore_extensions=False):
     :return: A boolean determining whether the process is alive or not
     """
     name = name.lower()
-    if ignore_extensions:
-        name = _remove_extension(name)
     if name.isspace():
         return False
+
+    if ignore_extensions:
+        name_extensionless = _remove_extension(name)
+
     for process in _safe_get_processes(["name"]):
         try:
             proc_name = process.name().lower()
@@ -165,10 +181,17 @@ def process_exists(name, ignore_extensions=False):
         except psutil.AccessDenied as e:
             logger.info(f"Permissions issue on {process} during process_exists check", exc_info=True)
             continue
-        if ignore_extensions:
-            proc_name = _remove_extension(proc_name)
-        if proc_name == name:
+
+        if proc_name == name:  # abc.exe matches abc.exe
             return True
+        if ignore_extensions:
+            proc_name_extensionless = _remove_extension(proc_name)
+            if proc_name_extensionless == name:  # abc matches abc.exe
+                return True
+            if proc_name == name_extensionless:  # abc.exe matches abc
+                return True
+            # don't check proc_name_extensionless against name_extensionless: abc.exe and abc.exe are already tested,
+            # however xyz.Gamelauncher should not match xyz.DedicatedServer
     return False
 
 
@@ -341,17 +364,14 @@ def _safe_kill_process(proc):
     except Exception:  # purposefully broad
         logger.warning("Unexpected exception while terminating process", exc_info=True)
 
-def _safe_kill_process_list(proc_list):
+
+def _safe_kill_processes(processes):
     """
     Kills a given process without raising an error
 
-    :param proc_list: The process list to kill
+    :param processes: An iterable of processes to kill
     """
-
-    def on_terminate(proc):
-        print(f"process '{proc.name()}' with id '{proc.pid}' terminated with exit code {proc.returncode}")
-
-    for proc in proc_list:
+    for proc in processes:
         try:
             logger.info(f"Terminating process '{proc.name()}' with id '{proc.pid}'")
             proc.kill()
@@ -360,12 +380,14 @@ def _safe_kill_process_list(proc_list):
         except psutil.NoSuchProcess:
             logger.debug("Termination request ignored, process was already terminated during iteration", exc_info=True)
         except Exception:  # purposefully broad
-            logger.warning("Unexpected exception while terminating process", exc_info=True)
+            logger.warning("Unexpected exception ignored while terminating process", exc_info=True)
 
+    def on_terminate(proc):
+        logger.info(f"process '{proc.name()}' with id '{proc.pid}' terminated with exit code {proc.returncode}")
     try:
-        psutil.wait_procs(proc_list, timeout=30, callback=on_terminate)
+        psutil.wait_procs(processes, timeout=30, callback=on_terminate)
     except Exception:  # purposefully broad
-        logger.warning("Unexpected exception while waiting for process to terminate", exc_info=True)
+        logger.warning("Unexpected exception while waiting for processes to terminate", exc_info=True)
 
 
 def _terminate_and_confirm_dead(proc):
@@ -383,7 +405,7 @@ def _terminate_and_confirm_dead(proc):
 
 def _remove_extension(filename):
     """
-    Returns a file name without its extension
+    Returns a file name without its extension, if any is present
 
     :param filename: The name of a file
     :return: The name of the file without the extension

Tools/LyTestTools/tests/integ/sanity_tests.py

@@ -58,7 +58,7 @@ class TestAutomatedTestingProject(object):
             # Call the game client executable
             with launcher.start():
                 # Wait for the process to exist
-                waiter.wait_for(lambda: process_utils.process_exists(f"{project}.GameLauncher", ignore_extensions=True))
+                waiter.wait_for(lambda: process_utils.process_exists(f"{project}.GameLauncher.exe", ignore_extensions=True))
         finally:
             # Clean up processes after the test is finished
             process_utils.kill_processes_named(names=process_utils.LY_PROCESS_KILL_LIST, ignore_extensions=True)
@@ -85,7 +85,7 @@ class TestAutomatedTestingProject(object):
             # Call the Editor executable
             with editor.start():
                 # Wait for the process to exist
-                waiter.wait_for(lambda: process_utils.process_exists("Editor", ignore_extensions=True))
+                waiter.wait_for(lambda: process_utils.process_exists("Editor.exe", ignore_extensions=True))
         finally:
             # Clean up processes after the test is finished
             process_utils.kill_processes_named(names=process_utils.LY_PROCESS_KILL_LIST, ignore_extensions=True)

Tools/LyTestTools/tests/unit/test_process_utils.py

@@ -223,7 +223,7 @@ class TestCloseWindowsProcess(unittest.TestCase):
         mock_enum.assert_called_once()
 
 
-class Test(unittest.TestCase):
+class TestProcessMatching(unittest.TestCase):
 
     @mock.patch("ly_test_tools.environment.process_utils._safe_get_processes")
     def test_ProcExists_HasExtension_Found(self, mock_get_proc):
@@ -261,18 +261,55 @@ class Test(unittest.TestCase):
         self.assertTrue(result)
         proc_mock.name.assert_called()
 
-    @mock.patch('ly_test_tools.environment.process_utils._safe_kill_process', mock.MagicMock)
+    @mock.patch('ly_test_tools.environment.process_utils._safe_kill_processes')
     @mock.patch('ly_test_tools.environment.process_utils._safe_get_processes')
-    def test_KillProcNamed_MockKill_SilentSuccess(self, mock_get_proc):
+    def test_KillProcNamed_ExactMatch_Killed(self, mock_get_proc, mock_kill_proc):
+        name = "dummy.exe"
+        proc_mock = mock.MagicMock()
+        proc_mock.name.return_value = name
+        mock_get_proc.return_value = [proc_mock]
+
+        process_utils.kill_processes_named("dummy.exe", ignore_extensions=False)
+        mock_kill_proc.assert_called()
+        proc_mock.name.assert_called()
+
+    @mock.patch('ly_test_tools.environment.process_utils._safe_kill_processes')
+    @mock.patch('ly_test_tools.environment.process_utils._safe_get_processes')
+    def test_KillProcNamed_NearMatch_Ignore(self, mock_get_proc, mock_kill_proc):
+        name = "dummy.exe"
+        proc_mock = mock.MagicMock()
+        proc_mock.name.return_value = name
+        mock_get_proc.return_value = [proc_mock]
+
+        process_utils.kill_processes_named("dummy", ignore_extensions=False)
+        mock_kill_proc.assert_not_called()
+        proc_mock.name.assert_called()
+
+    @mock.patch('ly_test_tools.environment.process_utils._safe_kill_processes')
+    @mock.patch('ly_test_tools.environment.process_utils._safe_get_processes')
+    def test_KillProcNamed_NearMatchIgnoreExtension_Kill(self, mock_get_proc, mock_kill_proc):
         name = "dummy.exe"
         proc_mock = mock.MagicMock()
         proc_mock.name.return_value = name
         mock_get_proc.return_value = [proc_mock]
 
         process_utils.kill_processes_named("dummy", ignore_extensions=True)
+        mock_kill_proc.assert_called()
         proc_mock.name.assert_called()
 
-    @mock.patch('ly_test_tools.environment.process_utils._safe_kill_process', mock.MagicMock)
+    @mock.patch('ly_test_tools.environment.process_utils._safe_kill_processes')
+    @mock.patch('ly_test_tools.environment.process_utils._safe_get_processes')
+    def test_KillProcNamed_ExactMatchIgnoreExtension_Killed(self, mock_get_proc, mock_kill_proc):
+        name = "dummy.exe"
+        proc_mock = mock.MagicMock()
+        proc_mock.name.return_value = name
+        mock_get_proc.return_value = [proc_mock]
+
+        process_utils.kill_processes_named("dummy.exe", ignore_extensions=True)
+        mock_kill_proc.assert_called()
+        proc_mock.name.assert_called()
+
+    @mock.patch('ly_test_tools.environment.process_utils._safe_kill_processes', mock.MagicMock)
     @mock.patch('ly_test_tools.environment.process_utils._safe_get_processes')
     @mock.patch('os.path.exists')
     def test_KillProcFrom_MockKill_SilentSuccess(self, mock_path, mock_get_proc):
@@ -293,7 +330,7 @@ class Test(unittest.TestCase):
 
         mock_kill.assert_called()
 
-    @mock.patch('ly_test_tools.environment.process_utils._safe_kill_process', mock.MagicMock)
+    @mock.patch('ly_test_tools.environment.process_utils._safe_kill_processes', mock.MagicMock)
     @mock.patch('psutil.Process')
     def test_KillProcPid_NoProc_SilentPass(self, mock_psutil):
         mock_proc = mock.MagicMock()
@@ -302,7 +339,7 @@ class Test(unittest.TestCase):
 
         process_utils.kill_process_with_pid(1)
 
-    @mock.patch('ly_test_tools.environment.process_utils._safe_kill_process', mock.MagicMock)
+    @mock.patch('ly_test_tools.environment.process_utils._safe_kill_processes', mock.MagicMock)
     @mock.patch('psutil.Process')
     def test_KillProcPidRaiseOnMissing_NoProc_Raises(self, mock_psutil):
         mock_proc = mock.MagicMock()
@@ -339,7 +376,7 @@ class Test(unittest.TestCase):
         mock_wait_procs.side_effect = psutil.PermissionError()
         proc_mock = mock.MagicMock()
 
-        process_utils._safe_kill_process_list(proc_mock)
+        process_utils._safe_kill_processes(proc_mock)
 
         mock_wait_procs.assert_called()
         mock_log_warn.assert_called()