From a1edb2c4ac0e8b56e150be02277a091c27c6fffc Mon Sep 17 00:00:00 2001 From: sweeneys Date: Thu, 21 Oct 2021 11:51:47 -0700 Subject: [PATCH 1/2] Update platform defaults to return a launcher instead of a string Signed-off-by: sweeneys --- .../launchers/launcher_helper.py | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/Tools/LyTestTools/ly_test_tools/launchers/launcher_helper.py b/Tools/LyTestTools/ly_test_tools/launchers/launcher_helper.py index d37623f352..9a75bbe3ee 100755 --- a/Tools/LyTestTools/ly_test_tools/launchers/launcher_helper.py +++ b/Tools/LyTestTools/ly_test_tools/launchers/launcher_helper.py @@ -9,14 +9,15 @@ Main launchers module, provides a facade for creating launchers. import logging -import ly_test_tools._internal.managers.workspace import ly_test_tools +import ly_test_tools._internal.managers.workspace as workspace_manager +import ly_test_tools.launchers.platforms.base as base_launcher log = logging.getLogger(__name__) def create_launcher(workspace, launcher_platform=ly_test_tools.HOST_OS_PLATFORM, args=None): - # type: (ly_test_tools.managers.workspace.WorkspaceManager, str, List[str]) -> Launcher + # type: (workspace_manager.AbstractWorkspaceManager, str, list[str]) -> base_launcher.Launcher """ Create a launcher compatible with the specified workspace, if no specific launcher is found return a generic one. @@ -25,12 +26,16 @@ def create_launcher(workspace, launcher_platform=ly_test_tools.HOST_OS_PLATFORM, :param args: List of arguments to pass to the launcher's 'args' argument during construction :return: Launcher instance """ - launcher_class = ly_test_tools.LAUNCHERS.get(launcher_platform, ly_test_tools.HOST_OS_PLATFORM) + launcher_class = ly_test_tools.LAUNCHERS.get(launcher_platform) + if not launcher_class: + log.warning(f"Using default launcher for '{ly_test_tools.HOST_OS_PLATFORM}' " + f"as no option is available for '{launcher_platform}'") + launcher_class = ly_test_tools.LAUNCHERS.get(ly_test_tools.HOST_OS_PLATFORM) return launcher_class(workspace, args) def create_dedicated_launcher(workspace, launcher_platform=ly_test_tools.HOST_OS_DEDICATED_SERVER, args=None): - # type: (ly_test_tools.managers.workspace.WorkspaceManager, str, List[str]) -> Launcher + # type: (workspace_manager.AbstractWorkspaceManager, str, list[str]) -> base_launcher.Launcher """ Create a dedicated launcher compatible with the specified workspace. Dedicated Launcher is only supported on the Linux and Windows Platform @@ -41,11 +46,15 @@ def create_dedicated_launcher(workspace, launcher_platform=ly_test_tools.HOST_OS :return: Launcher instance """ launcher_class = ly_test_tools.LAUNCHERS.get(launcher_platform, ly_test_tools.HOST_OS_DEDICATED_SERVER) + if not launcher_class: + log.warning(f"Using default dedicated launcher for '{ly_test_tools.HOST_OS_PLATFORM}' " + f"as no option is available for '{launcher_platform}'") + launcher_class = ly_test_tools.LAUNCHERS.get(ly_test_tools.HOST_OS_DEDICATED_SERVER) return launcher_class(workspace, args) def create_editor(workspace, launcher_platform=ly_test_tools.HOST_OS_EDITOR, args=None): - # type: (ly_test_tools.managers.workspace.WorkspaceManager, str, List[str]) -> Launcher + # type: (workspace_manager.AbstractWorkspaceManager, str, list[str]) -> base_launcher.Launcher """ Create an Editor compatible with the specified workspace. Editor is only officially supported on the Windows Platform. @@ -56,11 +65,15 @@ def create_editor(workspace, launcher_platform=ly_test_tools.HOST_OS_EDITOR, arg :return: Editor instance """ launcher_class = ly_test_tools.LAUNCHERS.get(launcher_platform, ly_test_tools.HOST_OS_EDITOR) + if not launcher_class: + log.warning(f"Using default editor launcher for '{ly_test_tools.HOST_OS_PLATFORM}' " + f"as no option is available for '{launcher_platform}'") + launcher_class = ly_test_tools.LAUNCHERS.get(ly_test_tools.HOST_OS_EDITOR) return launcher_class(workspace, args) def create_generic_launcher(workspace, launcher_platform, exe_file_name, args=None): - # type: (ly_test_tools.managers.workspace.WorkspaceManager, str, str, List[str]) -> Launcher + # type: (workspace_manager.AbstractWorkspaceManager, str, str, list[str]) -> base_launcher.Launcher """ Create a generic launcher compatible with the specified workspace. Allows custom .exe files to serve as the launcher instead of ones listed in the ly_test_tools.LAUNCHERS constant @@ -72,4 +85,8 @@ def create_generic_launcher(workspace, launcher_platform, exe_file_name, args=No :return: Launcher instance. """ launcher_class = ly_test_tools.LAUNCHERS.get(launcher_platform, ly_test_tools.HOST_OS_GENERIC_EXECUTABLE) + if not launcher_class: + log.warning(f"Using default generic executable launcher for '{ly_test_tools.HOST_OS_PLATFORM}' " + f"as no option is available for '{launcher_platform}'") + launcher_class = ly_test_tools.LAUNCHERS.get(ly_test_tools.HOST_OS_GENERIC_EXECUTABLE) return launcher_class(workspace, exe_file_name, args) From ea325d356c736a7cf5544a1b18b42252720c91ae Mon Sep 17 00:00:00 2001 From: sweeneys Date: Thu, 21 Oct 2021 12:05:22 -0700 Subject: [PATCH 2/2] updated defaults in all launchers, with additional unit test Signed-off-by: sweeneys --- .../ly_test_tools/launchers/launcher_helper.py | 12 ++++++------ Tools/LyTestTools/tests/unit/test_launcher_base.py | 7 +++++++ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/Tools/LyTestTools/ly_test_tools/launchers/launcher_helper.py b/Tools/LyTestTools/ly_test_tools/launchers/launcher_helper.py index 9a75bbe3ee..ac4ad621da 100755 --- a/Tools/LyTestTools/ly_test_tools/launchers/launcher_helper.py +++ b/Tools/LyTestTools/ly_test_tools/launchers/launcher_helper.py @@ -45,9 +45,9 @@ def create_dedicated_launcher(workspace, launcher_platform=ly_test_tools.HOST_OS :param args: List of arguments to pass to the launcher's 'args' argument during construction :return: Launcher instance """ - launcher_class = ly_test_tools.LAUNCHERS.get(launcher_platform, ly_test_tools.HOST_OS_DEDICATED_SERVER) + launcher_class = ly_test_tools.LAUNCHERS.get(launcher_platform) if not launcher_class: - log.warning(f"Using default dedicated launcher for '{ly_test_tools.HOST_OS_PLATFORM}' " + log.warning(f"Using default dedicated launcher for '{ly_test_tools.HOST_OS_DEDICATED_SERVER}' " f"as no option is available for '{launcher_platform}'") launcher_class = ly_test_tools.LAUNCHERS.get(ly_test_tools.HOST_OS_DEDICATED_SERVER) return launcher_class(workspace, args) @@ -64,9 +64,9 @@ def create_editor(workspace, launcher_platform=ly_test_tools.HOST_OS_EDITOR, arg :param args: List of arguments to pass to the launcher's 'args' argument during construction :return: Editor instance """ - launcher_class = ly_test_tools.LAUNCHERS.get(launcher_platform, ly_test_tools.HOST_OS_EDITOR) + launcher_class = ly_test_tools.LAUNCHERS.get(launcher_platform) if not launcher_class: - log.warning(f"Using default editor launcher for '{ly_test_tools.HOST_OS_PLATFORM}' " + log.warning(f"Using default editor launcher for '{ly_test_tools.HOST_OS_EDITOR}' " f"as no option is available for '{launcher_platform}'") launcher_class = ly_test_tools.LAUNCHERS.get(ly_test_tools.HOST_OS_EDITOR) return launcher_class(workspace, args) @@ -84,9 +84,9 @@ def create_generic_launcher(workspace, launcher_platform, exe_file_name, args=No :param args: List of arguments to pass to the launcher's 'args' argument during construction :return: Launcher instance. """ - launcher_class = ly_test_tools.LAUNCHERS.get(launcher_platform, ly_test_tools.HOST_OS_GENERIC_EXECUTABLE) + launcher_class = ly_test_tools.LAUNCHERS.get(launcher_platform) if not launcher_class: - log.warning(f"Using default generic executable launcher for '{ly_test_tools.HOST_OS_PLATFORM}' " + log.warning(f"Using default generic executable launcher for '{ly_test_tools.HOST_OS_GENERIC_EXECUTABLE}' " f"as no option is available for '{launcher_platform}'") launcher_class = ly_test_tools.LAUNCHERS.get(ly_test_tools.HOST_OS_GENERIC_EXECUTABLE) return launcher_class(workspace, exe_file_name, args) diff --git a/Tools/LyTestTools/tests/unit/test_launcher_base.py b/Tools/LyTestTools/tests/unit/test_launcher_base.py index 1ab9129a7d..5264e1bc28 100755 --- a/Tools/LyTestTools/tests/unit/test_launcher_base.py +++ b/Tools/LyTestTools/tests/unit/test_launcher_base.py @@ -221,3 +221,10 @@ class TestLauncherBuilder(object): under_test = ly_test_tools.launchers.launcher_helper.create_editor( dummy_workspace, ly_test_tools.HOST_OS_GENERIC_EXECUTABLE) assert isinstance(under_test, ly_test_tools.launchers.Launcher) + + def test_CreateEditor_InvalidPlatform_ValidLauncherStillReturned(self): + dummy_workspace = mock.MagicMock() + dummy_workspace.paths.build_directory.return_value = 'dummy' + under_test = ly_test_tools.launchers.launcher_helper.create_editor( + dummy_workspace, 'does not exist') + assert isinstance(under_test, ly_test_tools.launchers.Launcher)