Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed the loading of the legacy system_<platform>_<asset_platform>.cfg files #13721

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
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>
  • Loading branch information
lemonade-dm committed Jan 9, 2023
commit 04ab7e6e35180c8f24a7cbdc7c763183e0d48757
Original file line number Diff line number Diff line change
Expand Up @@ -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):
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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):
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
"""
Expand All @@ -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):
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
23 changes: 0 additions & 23 deletions Tools/LyTestTools/ly_test_tools/o3de/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
evanchia-ly-sdets marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 0 additions & 10 deletions Tools/LyTestTools/tests/unit/test_launcher_android.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
11 changes: 1 addition & 10 deletions Tools/LyTestTools/tests/unit/test_manager_platforms_mac.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

from ly_test_tools._internal.managers.platforms.mac import (
_MacResourceLocator, MacWorkspaceManager,
CACHE_DIR, CONFIG_FILE)
CACHE_DIR)
from ly_test_tools import MAC

pytestmark = pytest.mark.SUITE_smoke
Expand All @@ -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)
Expand Down
11 changes: 1 addition & 10 deletions Tools/LyTestTools/tests/unit/test_manager_platforms_windows.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

from ly_test_tools._internal.managers.platforms.windows import (
_WindowsResourceLocator, WindowsWorkspaceManager,
CACHE_DIR, CONFIG_FILE)
CACHE_DIR)
from ly_test_tools import WINDOWS

pytestmark = pytest.mark.SUITE_smoke
Expand All @@ -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)
Expand Down
56 changes: 5 additions & 51 deletions cmake/Tools/layout_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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'
Expand All @@ -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


Expand Down