From 009e96d601dec262c2c9564bf01c00be9bdc3eea Mon Sep 17 00:00:00 2001 From: evanchia Date: Wed, 16 Jun 2021 10:02:36 -0700 Subject: [PATCH 1/3] fixing ly_test_tools function and unit tests --- .../managers/abstract_resource_locator.py | 39 ++++++++++++++++--- .../unit/test_abstract_resource_locator.py | 23 ++++++++--- .../tests/unit/test_builtin_helpers.py | 2 + .../tests/unit/test_manager_platforms_mac.py | 28 +++++++------ .../unit/test_manager_platforms_windows.py | 31 +++++++++------ 5 files changed, 90 insertions(+), 33 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 index ec4b43b023..c1a22e5641 100755 --- a/Tools/LyTestTools/ly_test_tools/_internal/managers/abstract_resource_locator.py +++ b/Tools/LyTestTools/ly_test_tools/_internal/managers/abstract_resource_locator.py @@ -14,7 +14,9 @@ Utility class to resolve Lumberyard directory paths & file mappings. import os import pathlib import warnings +import json from abc import ABCMeta, abstractmethod +from weakref import KeyedRef import ly_test_tools._internal.pytest_plugin from ly_test_tools.environment.file_system import find_ancestor_file @@ -50,11 +52,38 @@ def _find_project_json(engine_root, project): Find the project.json file for this project. :return: Full path to the project.json file """ - # First check relative to defined build directory, for external projects which configure through SDK settings - project_json = find_ancestor_file(target_file_name='project.json', - start_path=ly_test_tools._internal.pytest_plugin.build_directory) - if not project_json: # check internally for a project bundled with the engine - project_json = os.path.join(engine_root, project, 'project.json') + project_json = None + + # Check the o3de_manifest.json and for the "projects" key + manifest_json = os.path.join(os.path.expanduser('~'), '.o3de', 'o3de_manifest.json') + if os.path.isfile(manifest_json): + # Read the o3de_manifest.json + with open(manifest_json, "r") as manifest_file: + json_data = json.load(manifest_file) + # Look at the "projects" key for registered project paths + try: + for projects_path in json_data["projects"]: + # Only look at project directories that match our project + if project == os.path.basename(projects_path): + check_project_json = os.path.join(projects_path, 'project.json') + # Check for the project.json file inside of the project directory + if os.path.isfile(check_project_json): + project_json = check_project_json + except KeyError: + pass # No projects found in the manifest json + + # Check relative to defined build directory, for external projects which configure through SDK settings + if not project_json: + project_json = find_ancestor_file(target_file_name='project.json', + start_path=ly_test_tools._internal.pytest_plugin.build_directory) + # Check internally for a project bundled with the engine + if not project_json: + check_project_json = os.path.join(engine_root, project, 'project.json') + if os.path.isfile(check_project_json): + project_json = check_project_json + + if not project_json: + raise OSError(f"Unable to find the project directory for project: ${project}") return project_json diff --git a/Tools/LyTestTools/tests/unit/test_abstract_resource_locator.py b/Tools/LyTestTools/tests/unit/test_abstract_resource_locator.py index 12286b3dd7..a96f2a0699 100755 --- a/Tools/LyTestTools/tests/unit/test_abstract_resource_locator.py +++ b/Tools/LyTestTools/tests/unit/test_abstract_resource_locator.py @@ -24,6 +24,8 @@ mock_engine_root = "mock_engine_root" mock_dev_path = "mock_dev_path" mock_build_directory = 'mock_build_directory' mock_project = 'mock_project' +mock_manifest_json = {'projects': [mock_project]} +mock_project_json = os.path.join(mock_project, 'project.json') class TestFindEngineRoot(object): @@ -47,11 +49,24 @@ class TestFindEngineRoot(object): with pytest.raises(OSError): abstract_resource_locator._find_engine_root(mock_initial_path) +@mock.patch('builtins.open', mock.MagicMock()) +class TestFindProjectJson(object): + + @mock.patch('os.path.isfile', mock.MagicMock(return_value=True)) + @mock.patch('os.path.basename', mock.MagicMock(return_value=mock_project)) + @mock.patch('json.load', mock.MagicMock(return_value=mock_manifest_json)) + def test_FindProjectJson_ManifestJson_ReturnsProjectJson(self): + project = abstract_resource_locator._find_project_json(mock_engine_root, mock_project) + + assert project == mock_project_json + @mock.patch('ly_test_tools._internal.managers.abstract_resource_locator.os.path.abspath', mock.MagicMock(return_value=mock_initial_path)) @mock.patch('ly_test_tools._internal.managers.abstract_resource_locator._find_engine_root', mock.MagicMock(return_value=mock_engine_root)) +@mock.patch('ly_test_tools._internal.managers.abstract_resource_locator._find_project_json', + mock.MagicMock(return_value=os.path.join(mock_project, 'project.json'))) class TestAbstractResourceLocator(object): def test_Init_HasEngineRoot_SetsAttrs(self): @@ -93,12 +108,11 @@ class TestAbstractResourceLocator(object): assert mock_abstract_resource_locator.build_directory() == mock_build_directory - def test_Project_IsCalled_ReturnsProjectPath(self): + def test_Project_IsCalled_ReturnsProjectDir(self): mock_abstract_resource_locator = abstract_resource_locator.AbstractResourceLocator( mock_build_directory, mock_project) - expected_path = os.path.join(mock_abstract_resource_locator.engine_root(), mock_project) - assert mock_abstract_resource_locator.project() == expected_path + assert mock_abstract_resource_locator.project() == os.path.dirname(mock_project_json) def test_AssetProcessor_IsCalled_ReturnsAssetProcessorPath(self): mock_abstract_resource_locator = abstract_resource_locator.AbstractResourceLocator( @@ -168,8 +182,7 @@ class TestAbstractResourceLocator(object): def test_AutoexecFile_IsCalled_ReturnsAutoexecFilePath(self): mock_abstract_resource_locator = abstract_resource_locator.AbstractResourceLocator( mock_build_directory, mock_project) - expected_path = os.path.join(mock_abstract_resource_locator.engine_root(), - mock_abstract_resource_locator._project, + expected_path = os.path.join(mock_abstract_resource_locator._project, 'autoexec.cfg') assert mock_abstract_resource_locator.autoexec_file() == expected_path diff --git a/Tools/LyTestTools/tests/unit/test_builtin_helpers.py b/Tools/LyTestTools/tests/unit/test_builtin_helpers.py index a4ebf5acec..23a5a10bf3 100755 --- a/Tools/LyTestTools/tests/unit/test_builtin_helpers.py +++ b/Tools/LyTestTools/tests/unit/test_builtin_helpers.py @@ -68,6 +68,8 @@ class TestBuiltinHelpers(object): assert type(under_test) == expected_workspace + @mock.patch('ly_test_tools._internal.managers.abstract_resource_locator._find_project_json', + mock.MagicMock(return_value='mock_project')) @mock.patch('ly_test_tools._internal.managers.workspace.AbstractWorkspaceManager', mock.MagicMock(return_value=MockedWorkspaceManager)) @mock.patch('ly_test_tools.builtin.helpers.MAC', True) diff --git a/Tools/LyTestTools/tests/unit/test_manager_platforms_mac.py b/Tools/LyTestTools/tests/unit/test_manager_platforms_mac.py index bbd8fbf9ae..0acf6457d9 100755 --- a/Tools/LyTestTools/tests/unit/test_manager_platforms_mac.py +++ b/Tools/LyTestTools/tests/unit/test_manager_platforms_mac.py @@ -14,6 +14,7 @@ Unit tests for ly_test_tools._internal.managers.platforms.mac import unittest.mock as mock import os import pytest +import ly_test_tools from ly_test_tools._internal.managers.platforms.mac import ( _MacResourceLocator, MacWorkspaceManager, @@ -22,11 +23,6 @@ from ly_test_tools import MAC pytestmark = pytest.mark.SUITE_smoke -if not MAC: - pytestmark = pytest.mark.skipif( - not MAC, - reason="test_manager_platforms_mac.py only runs on Mac") - mock_engine_root = 'mock_engine_root' mock_dev_path = 'mock_dev_path' mock_build_directory = 'mock_build_directory' @@ -34,16 +30,16 @@ mock_project = 'mock_project' mock_tmp_path = 'mock_tmp_path' mock_output_path = 'mock_output_path' -mac_resource_locator = _MacResourceLocator( - build_directory=mock_build_directory, - project=mock_project) - @mock.patch('ly_test_tools._internal.managers.abstract_resource_locator._find_engine_root', - mock.MagicMock(return_value=(mock_engine_root, mock_dev_path))) + mock.MagicMock(return_value=mock_engine_root)) +@mock.patch('ly_test_tools._internal.managers.abstract_resource_locator._find_project_json', + 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) @@ -51,6 +47,8 @@ class TestMacResourceLocator(object): 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) expected = os.path.join( mac_resource_locator.project_cache(), CACHE_DIR) @@ -58,6 +56,8 @@ class TestMacResourceLocator(object): assert mac_resource_locator.platform_cache() == expected def test_ProjectLog_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.project(), 'user', @@ -66,6 +66,8 @@ class TestMacResourceLocator(object): assert mac_resource_locator.project_log() == expected def test_ProjectScreenshots_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.project(), 'user', @@ -74,6 +76,8 @@ class TestMacResourceLocator(object): assert mac_resource_locator.project_screenshots() == expected def test_EditorLog_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.project_log(), 'editor.log') @@ -82,7 +86,9 @@ class TestMacResourceLocator(object): @mock.patch('ly_test_tools._internal.managers.abstract_resource_locator._find_engine_root', - mock.MagicMock(return_value=(mock_engine_root, mock_dev_path))) + mock.MagicMock(return_value=mock_engine_root)) +@mock.patch('ly_test_tools._internal.managers.abstract_resource_locator._find_project_json', + mock.MagicMock(return_value=mock_project)) class TestMacWorkspaceManager(object): def test_Init_SetDummyParams_ReturnsMacWorkspaceManager(self): diff --git a/Tools/LyTestTools/tests/unit/test_manager_platforms_windows.py b/Tools/LyTestTools/tests/unit/test_manager_platforms_windows.py index aaf34367e7..93ca9e3c07 100755 --- a/Tools/LyTestTools/tests/unit/test_manager_platforms_windows.py +++ b/Tools/LyTestTools/tests/unit/test_manager_platforms_windows.py @@ -14,6 +14,7 @@ Unit tests for ly_test_tools._internal.managers.platforms.windows import unittest.mock as mock import os import pytest +import ly_test_tools from ly_test_tools._internal.managers.platforms.windows import ( _WindowsResourceLocator, WindowsWorkspaceManager, @@ -34,22 +35,16 @@ mock_project = 'mock_project' mock_tmp_path = 'mock_tmp_path' mock_output_path = 'mock_output_path' -windows_resource_locator = _WindowsResourceLocator( - build_directory=mock_build_directory, - project=mock_project) - -windows_workspace_manager = WindowsWorkspaceManager( - build_directory=mock_build_directory, - project=mock_project, - tmp_path=mock_tmp_path, - output_path=mock_output_path) - @mock.patch('ly_test_tools._internal.managers.abstract_resource_locator._find_engine_root', - mock.MagicMock(return_value=(mock_engine_root, mock_dev_path))) + mock.MagicMock(return_value=mock_engine_root)) +@mock.patch('ly_test_tools._internal.managers.abstract_resource_locator._find_project_json', mock.MagicMock( + 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) @@ -57,12 +52,16 @@ class TestWindowsResourceLocator(object): 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) expected = os.path.join( windows_resource_locator.project_cache(), CACHE_DIR) assert windows_resource_locator.platform_cache() == expected def test_ProjectLog_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.project(), 'user', @@ -71,6 +70,8 @@ class TestWindowsResourceLocator(object): assert windows_resource_locator.project_log() == expected def test_ProjectScreenshots_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.project(), 'user', @@ -79,6 +80,8 @@ class TestWindowsResourceLocator(object): assert windows_resource_locator.project_screenshots() == expected def test_EditorLog_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.project_log(), 'editor.log') @@ -87,17 +90,21 @@ class TestWindowsResourceLocator(object): @mock.patch('ly_test_tools._internal.managers.abstract_resource_locator._find_engine_root', - mock.MagicMock(return_value=(mock_engine_root, mock_dev_path))) + mock.MagicMock(return_value=mock_engine_root)) +@mock.patch('ly_test_tools._internal.managers.abstract_resource_locator._find_project_json', mock.MagicMock( + return_value=mock_project)) class TestWindowsWorkspaceManager(object): @mock.patch('ly_test_tools.environment.reg_cleaner.create_ly_keys') def test_SetRegistryKeys_NewWorkspaceManager_KeyCreateCalled(self, mock_create_keys): + windows_workspace_manager = ly_test_tools._internal.managers.platforms.windows.WindowsWorkspaceManager() windows_workspace_manager.set_registry_keys() mock_create_keys.assert_called_once() @mock.patch('ly_test_tools.environment.reg_cleaner.clean_ly_keys') def test_ClearSettings_NewWorkspaceManager_KeyClearCalled(self, mock_clear_keys): + windows_workspace_manager = ly_test_tools._internal.managers.platforms.windows.WindowsWorkspaceManager() windows_workspace_manager.clear_settings() mock_clear_keys.assert_called_with(exception_list=r"SOFTWARE\Amazon\Lumberyard\Identity") From 017845e285e7fb58996abac4164ddcaf9d007541 Mon Sep 17 00:00:00 2001 From: evanchia Date: Wed, 16 Jun 2021 11:29:14 -0700 Subject: [PATCH 2/3] fixing project() func to read project.json --- .../managers/abstract_resource_locator.py | 15 +++++++++------ .../tests/unit/test_abstract_resource_locator.py | 5 +++-- 2 files changed, 12 insertions(+), 8 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 index c1a22e5641..a7a1c867f6 100755 --- a/Tools/LyTestTools/ly_test_tools/_internal/managers/abstract_resource_locator.py +++ b/Tools/LyTestTools/ly_test_tools/_internal/managers/abstract_resource_locator.py @@ -63,12 +63,15 @@ def _find_project_json(engine_root, project): # Look at the "projects" key for registered project paths try: for projects_path in json_data["projects"]: - # Only look at project directories that match our project - if project == os.path.basename(projects_path): - check_project_json = os.path.join(projects_path, 'project.json') - # Check for the project.json file inside of the project directory - if os.path.isfile(check_project_json): - project_json = check_project_json + check_project_json = os.path.join(projects_path, 'project.json') + # Check for the project.json file inside of the project directory + if os.path.isfile(check_project_json): + # Check if the "project_name" key matches our project + with open(check_project_json, "r") as project_json_file: + project_json_data = json.load(project_json_file) + if project == project_json_data["project_name"]: + project_json = check_project_json + break except KeyError: pass # No projects found in the manifest json diff --git a/Tools/LyTestTools/tests/unit/test_abstract_resource_locator.py b/Tools/LyTestTools/tests/unit/test_abstract_resource_locator.py index a96f2a0699..d8e4e5a14a 100755 --- a/Tools/LyTestTools/tests/unit/test_abstract_resource_locator.py +++ b/Tools/LyTestTools/tests/unit/test_abstract_resource_locator.py @@ -24,7 +24,8 @@ mock_engine_root = "mock_engine_root" mock_dev_path = "mock_dev_path" mock_build_directory = 'mock_build_directory' mock_project = 'mock_project' -mock_manifest_json = {'projects': [mock_project]} +mock_manifest_json_file = {'projects': [mock_project]} +mock_project_json_file = {'project_name': mock_project} mock_project_json = os.path.join(mock_project, 'project.json') @@ -54,7 +55,7 @@ class TestFindProjectJson(object): @mock.patch('os.path.isfile', mock.MagicMock(return_value=True)) @mock.patch('os.path.basename', mock.MagicMock(return_value=mock_project)) - @mock.patch('json.load', mock.MagicMock(return_value=mock_manifest_json)) + @mock.patch('json.load', mock.MagicMock(side_effect=[mock_manifest_json_file, mock_project_json_file])) def test_FindProjectJson_ManifestJson_ReturnsProjectJson(self): project = abstract_resource_locator._find_project_json(mock_engine_root, mock_project) From cf7f1defebad748776b1f7d3e4cafd1eaf19bdec Mon Sep 17 00:00:00 2001 From: evanchia Date: Wed, 16 Jun 2021 14:45:11 -0700 Subject: [PATCH 3/3] added warning message --- .../_internal/managers/abstract_resource_locator.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 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 index a7a1c867f6..48afeb8219 100755 --- a/Tools/LyTestTools/ly_test_tools/_internal/managers/abstract_resource_locator.py +++ b/Tools/LyTestTools/ly_test_tools/_internal/managers/abstract_resource_locator.py @@ -15,12 +15,14 @@ import os import pathlib import warnings import json +import logging from abc import ABCMeta, abstractmethod from weakref import KeyedRef import ly_test_tools._internal.pytest_plugin from ly_test_tools.environment.file_system import find_ancestor_file +logger = logging.getLogger(__name__) def _find_engine_root(initial_path): # type: (str) -> str @@ -72,8 +74,8 @@ def _find_project_json(engine_root, project): if project == project_json_data["project_name"]: project_json = check_project_json break - except KeyError: - pass # No projects found in the manifest json + except KeyError as err: + logger.warning(f"Project key could not be found due to error: {err}") # Check relative to defined build directory, for external projects which configure through SDK settings if not project_json: