Removed system_<platform>_<asset>.cfg uses from LyTestTools

The test tools were setting the `log_RemoteConsoleAllowedAddresses` console variable, through those settings files and they have been updated to specify those values through the command line arguments. For Android test tools launcher, that console variable is set through the Settings Registry '/O3DE/Autoexec/ConsoleCommands/log_RemoteConsoleAllowedAddresses' mechanism. Also removed the validation of the `system_<platform>_<asset>.cfg` from the CMake layout tool `verify_layout` method as it is no longer needed. Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com>
o3de · Jan 9, 2023 · aec52ae · aec52ae
1 parent 55faa40
commit aec52ae
Show file tree

Hide file tree

Showing 14 changed files with 11 additions and 154 deletions.
diff --git a/Tools/LyTestTools/ly_test_tools/_internal/managers/abstract_resource_locator.py b/Tools/LyTestTools/ly_test_tools/_internal/managers/abstract_resource_locator.py
@@ -325,17 +325,6 @@ def material_canvas_log(self):
     #   The following are OS specific paths and must be defined by an override
     #
 
-    @abstractmethod
-    def platform_config_file(self):
-        """
-        Return the path to the platform config file.
-        :return: path to the platform config file (i.e. engine_root/dev/system_windows_pc.cfg)
-        """
-        raise NotImplementedError(
-            "platform_config_file() is not implemented on the base AbstractResourceLocator() class. "
-            "It must be defined by the inheriting class - "
-            "i.e. _WindowsResourceLocator(AbstractResourceLocator).platform_config_file()")
-
     @abstractmethod
     def platform_cache(self):
         """

diff --git a/Tools/LyTestTools/ly_test_tools/_internal/managers/platforms/linux.py b/Tools/LyTestTools/ly_test_tools/_internal/managers/platforms/linux.py
@@ -16,18 +16,12 @@
 logger = logging.getLogger(__name__)
 
 CACHE_DIR = 'linux'
-CONFIG_FILE = 'system_linux_pc.cfg'
 
 
 class _LinuxResourceManager(AbstractResourceLocator):
     """
     Override for locating resources in a Linux operating system running LyTestTools.
     """
-    def platform_config_file(self):
-        """
-        :return: path to the platform config file
-        """
-        return os.path.join(self.engine_root(), CONFIG_FILE)
 
     def platform_cache(self):
         """

diff --git a/Tools/LyTestTools/ly_test_tools/_internal/managers/platforms/mac.py b/Tools/LyTestTools/ly_test_tools/_internal/managers/platforms/mac.py
@@ -18,22 +18,13 @@
 logger = logging.getLogger(__name__)
 
 CACHE_DIR = 'mac'
-CONFIG_FILE = 'system_osx_mac.cfg'
 
 
 class _MacResourceLocator(AbstractResourceLocator):
     """
     Override for locating resources in a Mac operating system running LyTestTools.
     """
 
-    def platform_config_file(self):
-        """
-        Return the path to the platform config file.
-        ex. engine_root/dev/system_osx_mac.cfg
-        :return: path to the platform config file
-        """
-        return os.path.join(self.engine_root(), CONFIG_FILE)
-
     def platform_cache(self):
         """
         Return path to the cache for the Mac operating system.

diff --git a/Tools/LyTestTools/ly_test_tools/_internal/managers/platforms/windows.py b/Tools/LyTestTools/ly_test_tools/_internal/managers/platforms/windows.py
@@ -24,22 +24,13 @@
 logger = logging.getLogger(__name__)
 
 CACHE_DIR = 'pc'
-CONFIG_FILE = 'system_windows_pc.cfg'
 
 
 class _WindowsResourceLocator(AbstractResourceLocator):
     """
     Override for locating resources in a Windows operating system running LyTestTools.
     """
 
-    def platform_config_file(self):
-        """
-        Return the path to the platform config file.
-        ex. engine_root/dev/system_osx_mac.cfg
-        :return: path to the platform config file
-        """
-        return os.path.join(self.engine_root(), CONFIG_FILE)
-
     def platform_cache(self):
         """
         Return path to the cache for the Windows operating system.

diff --git a/Tools/LyTestTools/ly_test_tools/launchers/platforms/android/launcher.py b/Tools/LyTestTools/ly_test_tools/launchers/platforms/android/launcher.py
@@ -213,18 +213,18 @@ def configure_settings(self):
 
         # Create a python dictionary that can serialize to a json file with JSON pointer of
         # /Amazon/AzCore/Bootstrap/<settings>
-        vfs_settings = { 'Amazon' : { 'AzCore' : { 'Bootstrap' : {} }}}
+        vfs_settings = { 'Amazon' : { 'AzCore' : { 'Bootstrap' : {} }},
+            'O3DE': { 'Autoexec' : { 'ConsoleCommands' : {}}}}
         vfs_settings['Amazon']['AzCore']['Bootstrap']['connect_to_remote'] = 1
         vfs_settings['Amazon']['AzCore']['Bootstrap']['android_connect_to_remote'] = 1
         vfs_settings['Amazon']['AzCore']['Bootstrap']['wait_for_connect'] = 1
         vfs_settings['Amazon']['AzCore']['Bootstrap']['remote_ip'] = '127.0.0.1'
         vfs_settings['Amazon']['AzCore']['Bootstrap']['remote_port'] = 45643
+        vfs_settings['O3DE']['Autoexec']['ConsoleComands']['log_RemoteConsoleAllowedAddresses'] = '127.0.0.1'
         self.android_vfs_setreg_path = user_registry_path / 'test_android_vfs_settings.android.setreg'
         with self.android_vfs_setreg_path.open('w') as android_vfs_setreg:
             json.dump(vfs_settings, android_vfs_setreg, indent=4)
 
-        self.workspace.settings.modify_platform_setting("log_RemoteConsoleAllowedAddresses", '127.0.0.1')
-
     def launch(self):
         """
         Launches the APK matching self.package_name to the device that matches self._device_id.

diff --git a/Tools/LyTestTools/ly_test_tools/launchers/platforms/base.py b/Tools/LyTestTools/ly_test_tools/launchers/platforms/base.py
@@ -121,14 +121,13 @@ def backup_settings(self):
         """
         backup_path = self.workspace.settings.get_temp_path()
         logger.debug(f"Performing automatic backup of bootstrap, platform and user settings in path {backup_path}")
-        self.workspace.settings.backup_platform_settings(backup_path)
 
     def configure_settings(self):
         """
         Perform settings configuration, must be called after a backup of settings has been created with
         backup_settings(). Preferred ways to modify settings are:
 
-        self.workspace.settings.modify_platform_setting()
+        --regset="<key>=<value> arguments via the command line
 
         :return: None
         """
@@ -143,7 +142,6 @@ def restore_settings(self):
         """
         backup_path = self.workspace.settings.get_temp_path()
         logger.debug(f"Restoring backup of bootstrap, platform and user settings in path {backup_path}")
-        self.workspace.settings.restore_platform_settings(backup_path)
 
     def teardown(self):
         """

diff --git a/Tools/LyTestTools/ly_test_tools/launchers/platforms/linux/launcher.py b/Tools/LyTestTools/ly_test_tools/launchers/platforms/linux/launcher.py
@@ -171,9 +171,7 @@ def configure_settings(self):
         self.args.append(f'--regset="/Amazon/AzCore/Bootstrap/project_path={self.workspace.paths.project()}"')
         self.args.append(f'--regset="/Amazon/AzCore/Bootstrap/remote_ip={host_ip}"')
         self.args.append(f'--regset="/Amazon/AzCore/Bootstrap/allowed_list={host_ip}"')
-
-        self.workspace.settings.modify_platform_setting("r_ShaderCompilerServer", host_ip)
-        self.workspace.settings.modify_platform_setting("log_RemoteConsoleAllowedAddresses", host_ip)
+        self.args.append(f'--log_RemoteConsoleAllowedAddresses={host_ip}')
 
 
 class DedicatedLinuxLauncher(LinuxLauncher):

diff --git a/Tools/LyTestTools/ly_test_tools/launchers/platforms/win/launcher.py b/Tools/LyTestTools/ly_test_tools/launchers/platforms/win/launcher.py
@@ -171,8 +171,7 @@ def configure_settings(self):
         self.args.append(f'--regset="/Amazon/AzCore/Bootstrap/project_path={self.workspace.paths.project()}"')
         self.args.append(f'--regset="/Amazon/AzCore/Bootstrap/remote_ip={host_ip}"')
         self.args.append(f'--regset="/Amazon/AzCore/Bootstrap/allowed_list={host_ip}"')
-
-        self.workspace.settings.modify_platform_setting("log_RemoteConsoleAllowedAddresses", host_ip)
+        self.args.append(f'--log_RemoteConsoleAllowedAddresses={host_ip}')
 
 
 class DedicatedWinLauncher(WinLauncher):

diff --git a/Tools/LyTestTools/ly_test_tools/o3de/settings.py b/Tools/LyTestTools/ly_test_tools/o3de/settings.py
@@ -31,36 +31,13 @@ def __init__(self, temp_path, resource_locator):
     def get_temp_path(self):
         return self._temp_path
 
-    def modify_asset_processor_setting(self, setting, value):
-        logger.info(f'Updating setting {setting} to {value}')
-        _edit_text_settings_file(self._resource_locator.asset_processor_config_file(), setting, value)
-
-    def modify_platform_setting(self, setting, value):
-        logger.debug(f'Updating setting {setting} to {value}')
-        _edit_text_settings_file(self._resource_locator.platform_config_file(), setting, value)
-
     def backup_asset_processor_settings(self, backup_path=None):
         self._backup_settings(self._resource_locator.asset_processor_config_file(), backup_path)
 
-    def backup_platform_settings(self, backup_path=None):
-        """
-        Creates a backup of the platform settings file (~/dev/system_[platform].cfg) in the backup_path. If no path is
-        provided, it will store in the workspace temp path (the contents of the workspace temp directory are removed
-        during workspace teardown)
-        """
-        self._backup_settings(self._resource_locator.platform_config_file(), backup_path)
 
     def restore_asset_processor_settings(self, backup_path):
         self._restore_settings(self._resource_locator.asset_processor_config_file(), backup_path)
 
-    def restore_platform_settings(self, backup_path=None):
-        """
-        Restores the platform settings file (~/dev/system_[platform].cfg) from its backup.
-        The backup is stored in the backup_path.
-        If no backup_path is provided, it will attempt to retrieve the backup from the workspace temp path.
-        """
-        self._restore_settings(self._resource_locator.platform_config_file(), backup_path)
-
     def backup_json_settings(self, json_settings_file, backup_path=None):
         """
         Creates a backup of a Json/Registry Settings file in the backup_path. If no path is provided, it will store in

diff --git a/Tools/LyTestTools/tests/unit/test_abstract_resource_locator.py b/Tools/LyTestTools/tests/unit/test_abstract_resource_locator.py
@@ -181,12 +181,6 @@ def test_DevicesFile_IsCalled_ReturnsDevicesFilePath(self, mock_expanduser):
 
         assert abstract_resource_locator.AbstractResourceLocator.devices_file() == expected_path
 
-    def test_PlatformConfigFile_NotImplemented_RaisesNotImplementedError(self):
-        mock_abstract_resource_locator = abstract_resource_locator.AbstractResourceLocator(
-            mock_build_directory, mock_project)
-        with pytest.raises(NotImplementedError):
-            mock_abstract_resource_locator.platform_config_file()
-
     def test_PlatformCache_NotImplemented_RaisesNotImplementedError(self):
         mock_abstract_resource_locator = abstract_resource_locator.AbstractResourceLocator(
             mock_build_directory, mock_project)

diff --git a/Tools/LyTestTools/tests/unit/test_launcher_android.py b/Tools/LyTestTools/tests/unit/test_launcher_android.py
@@ -236,16 +236,6 @@ def test_Teardown_ValidTeardown_TeardownSucceeds(self, mock_restore, mock_config
         mock_restore.assert_called_once()
         mock_workspace.shader_compiler.stop.assert_called_once()
 
-    @mock.patch('ly_test_tools.launchers.platforms.base.Launcher._config_ini_to_dict')
-    @mock.patch('ly_test_tools.o3de.settings.LySettings.modify_platform_setting', mock.MagicMock)
-    def test_ConfigureSettings_DefaultValues_SetsValues(self, mock_config):
-        mock_config.return_value = VALID_ANDROID_CONFIG
-        mock_workspace = MockedWorkspace()
-
-        launcher = ly_test_tools.launchers.AndroidLauncher(mock_workspace, ["dummy"])
-        launcher.configure_settings()
-
-        assert mock_workspace.settings.modify_platform_setting.call_count == 1
 
     @mock.patch('ly_test_tools.launchers.platforms.base.Launcher._config_ini_to_dict')
     @mock.patch('ly_test_tools.environment.process_utils.check_output')

diff --git a/Tools/LyTestTools/tests/unit/test_manager_platforms_mac.py b/Tools/LyTestTools/tests/unit/test_manager_platforms_mac.py
@@ -33,15 +33,6 @@
             mock.MagicMock(return_value=mock_project))
 class TestMacResourceLocator(object):
 
-    def test_PlatformConfigFile_HasPath_ReturnsPath(self):
-        mac_resource_locator = ly_test_tools._internal.managers.platforms.mac._MacResourceLocator(mock_build_directory,
-                                                                                                  mock_project)
-        expected = os.path.join(
-            mac_resource_locator.engine_root(),
-            CONFIG_FILE)
-
-        assert mac_resource_locator.platform_config_file() == expected
-
     def test_PlatformCache_HasPath_ReturnsPath(self):
         mac_resource_locator = ly_test_tools._internal.managers.platforms.mac._MacResourceLocator(mock_build_directory,
                                                                                                   mock_project)

diff --git a/Tools/LyTestTools/tests/unit/test_manager_platforms_windows.py b/Tools/LyTestTools/tests/unit/test_manager_platforms_windows.py
@@ -38,15 +38,6 @@
     return_value=mock_project))
 class TestWindowsResourceLocator(object):
 
-    def test_PlatformConfigFile_HasPath_ReturnsPath(self):
-        windows_resource_locator = ly_test_tools._internal.managers.platforms.windows._WindowsResourceLocator(
-            mock_build_directory, mock_project)
-        expected = os.path.join(
-            windows_resource_locator.engine_root(),
-            CONFIG_FILE)
-
-        assert windows_resource_locator.platform_config_file() == expected
-
     def test_PlatformCache_HasPath_ReturnsPath(self):
         windows_resource_locator = ly_test_tools._internal.managers.platforms.windows._WindowsResourceLocator(
             mock_build_directory, mock_project)

diff --git a/cmake/Tools/layout_tool.py b/cmake/Tools/layout_tool.py
@@ -114,14 +114,6 @@ def _validate_remote_ap(input_remote_ip, input_remote_connect, remote_on_check):
                                                                          f'remote_ip'
                                                                          ])
 
-    # Validate the system_{platform}_{asset type}.cfg exists
-    platform_system_cfg_file = layout_path / f'system_{platform_name_lower}_{asset_type}.cfg'
-    if not platform_system_cfg_file.is_file():
-        warning_count += _warn(f"'system_{platform_name_lower}_{asset_type}.cfg' is missing from {str(layout_path)}")
-        system_config_values = None
-    else:
-        system_config_values = common.get_config_file_values(str(platform_system_cfg_file), [])
-
     if bootstrap_values:
 
         remote_ip = bootstrap_values.get(f'{platform_name_lower}_remote_ip') or bootstrap_values.get('remote_ip') or LOCAL_HOST
@@ -140,54 +132,20 @@ def _validate_remote_ap(input_remote_ip, input_remote_connect, remote_on_check):
         project_asset_path = layout_path / project_name_lower
         if not project_asset_path.is_dir():
             warning_count += _warn(f"Asset folder for project {project_name} is missing from the deployment layout.")
-
-        elif system_config_values is not None:
-
-            shaders_remote_compiler = '0'
-            asset_processor_shader_compiler = system_config_values.get('r_AssetProcessorShaderCompiler') or '0'
-            shader_compiler_server = LOCAL_HOST
-            shaders_allow_compilation = system_config_values.get('r_ShadersAllowCompilation')
-
-            def _validate_remote_shader_settings():
-
-                if asset_processor_shader_compiler != '1':
-                    return _warn(f"Connection to the remote shader compiler (r_ShaderCompilerServer) is not properly "
-                                 f"set in system_{platform_name_lower}_{asset_type}.cfg. If it is set to {LOCAL_HOST}, then "
-                                 f"r_AssetProcessorShaderCompiler must be set to 1.")
-
-
-                else:
-                    if _validate_remote_ap(remote_ip, remote_connect, False) > 0:
-                        return _warn(f"The system_{platform_name_lower}_{asset_type}.cfg file is configured to connect to the"
-                                     f" shader compiler server through the remote connection to the Asset Processor.")
-                return 0
-
+        else:
             # Validation steps based on the asset mode
             if asset_mode == ASSET_MODE_PAK:
                 # Validate that we have pak files
-                pak_count = 0
-                has_shader_pak = False
                 project_paks = project_asset_path.glob("*.pak")
-                for project_pak in project_paks:
-                    if project_pak.name == 'shadercachestartup.pak':
-                        has_shader_pak = True
-                    pak_count += 1
+                pak_count = len(project_paks)
 
                 if pak_count == 0:
                     warning_count += _warn("No pak files found for PAK mode deployment")
                     # Check if the shader paks are set
-                if has_shader_pak:
-                    # Since we are not connecting to the shader compiler, also make sure bootstrap is not configured to
-                    # connect to Asset Processor remotely
-                    warning_count += _validate_remote_ap(remote_ip, remote_connect, False)
 
-                    if shaders_allow_compilation is not None and shaders_allow_compilation == '1':
-                        warning_count += _warn(f"Shader paks are set for project {project_name} but shader compiling "
-                                               f"(r_ShadersAllowCompilation) is still enabled "
-                                               f"for it in system_{platform_name_lower}_{asset_type}.cfg.")
-
-                else:
-                    warning_count += _validate_remote_shader_settings()
+                # Since we are using pak files, make sure the settings are not configured to
+                # connect to Asset Processor remotely
+                warning_count += _validate_remote_ap(remote_ip, remote_connect, False)
 
             elif asset_mode == ASSET_MODE_VFS:
                 remote_file_system = bootstrap_values.get(f'{platform_name_lower}_remote_filesystem') or '0'
@@ -196,10 +154,6 @@ def _validate_remote_shader_settings():
                 else:
                     warning_count += _validate_remote_ap(remote_ip, remote_connect, True)
 
-            else:
-                # If there are no shader paks, make sure that a connection to the shader compiler is set
-                warning_count += _validate_remote_shader_settings()
-
     return warning_count