From 739c4a18b80851663fee59b75cd3ffa014e3bf6c Mon Sep 17 00:00:00 2001 From: Koji Hasegawa Date: Fri, 3 May 2024 17:26:48 +0900 Subject: [PATCH] Fix to not throw exception in Attributes --- RuntimeInternals/SceneManagerHelper.cs | 32 +++++++++++++------ .../UnityEditor/GameViewSizeWrapper.cs | 4 ++- .../SceneManagerHelperTest.cs | 32 ++++++++++++------- 3 files changed, 46 insertions(+), 22 deletions(-) diff --git a/RuntimeInternals/SceneManagerHelper.cs b/RuntimeInternals/SceneManagerHelper.cs index 6b07d45..0d207cd 100644 --- a/RuntimeInternals/SceneManagerHelper.cs +++ b/RuntimeInternals/SceneManagerHelper.cs @@ -86,7 +86,10 @@ internal static string GetExistScenePath(string path, string callerFilePath) path = GetAbsolutePath(path, callerFilePath); } - ValidatePath(path); + if (!ValidatePath(path)) + { + return null; + } #if UNITY_EDITOR return GetExistScenePathInEditor(path); #else @@ -112,34 +115,43 @@ internal static string GetAbsolutePath(string relativePath, string callerFilePat return absolutePath.Substring(packageIndexOf); } - Debug.LogError( - $"Can not resolve absolute path. relativePath: {relativePath}, callerFilePath: {callerFilePath}"); + Debug.LogError($"Can not resolve absolute path. relative: {relativePath}, caller: {callerFilePath}"); return null; // Note: Do not use Exception (and Assert). Because freezes async tests on UTF v1.3.4, See UUM-25085. } - private static void ValidatePath(string path) + private static bool ValidatePath(string path) { if (!path.StartsWith("Assets/") && !path.StartsWith("Packages/")) { - throw new ArgumentException($"Scene path must start with `Assets/` or `Packages/`. path: {path}"); + Debug.LogError($"Scene path must start with `Assets/` or `Packages/`. path: {path}"); + return false; + // Note: Do not use Exception (and Assert). Because freezes async tests on UTF v1.3.4, See UUM-25085. } var split = path.Split('/'); if (split[0].Equals("Packages") && split[1].IndexOfAny(new[] { '*', '?' }) >= 0) { - throw new ArgumentException($"Wildcards cannot be used in the package name of path: {path}"); + Debug.LogError($"Wildcards cannot be used in the package name of path: {path}"); + return false; + // Note: Do not use Exception (and Assert). Because freezes async tests on UTF v1.3.4, See UUM-25085. } if (!path.EndsWith(".unity")) { - throw new ArgumentException($"Scene path must ends with `.unity`. path: {path}"); + Debug.LogError($"Scene path must ends with `.unity`. path: {path}"); + return false; + // Note: Do not use Exception (and Assert). Because freezes async tests on UTF v1.3.4, See UUM-25085. } if (split.Last().IndexOfAny(new[] { '*', '?' }) >= 0) { - throw new ArgumentException($"Wildcards cannot be used in the most right section of path: {path}"); + Debug.LogError($"Wildcards cannot be used in the scene name of path: {path}"); + return false; + // Note: Do not use Exception (and Assert). Because freezes async tests on UTF v1.3.4, See UUM-25085. } + + return true; } private static string SearchFolder(string path) @@ -208,7 +220,9 @@ private static string GetExistScenePathInEditor(string path) } } - throw new FileNotFoundException($"Scene `{path}` is not found in AssetDatabase"); + Debug.LogError($"Scene `{path}` is not found in AssetDatabase"); + return null; + // Note: Do not use Exception (and Assert). Because freezes async tests on UTF v1.3.4, See UUM-25085. } #endif diff --git a/RuntimeInternals/Wrappers/UnityEditor/GameViewSizeWrapper.cs b/RuntimeInternals/Wrappers/UnityEditor/GameViewSizeWrapper.cs index eb2a652..a9d6c08 100644 --- a/RuntimeInternals/Wrappers/UnityEditor/GameViewSizeWrapper.cs +++ b/RuntimeInternals/Wrappers/UnityEditor/GameViewSizeWrapper.cs @@ -110,7 +110,9 @@ public string DisplayText() var displayTextProperty = s_gameViewSize.GetProperty("displayText"); if (displayTextProperty == null) { - throw new NullReferenceException("GameViewSize.displayText property not found."); + Debug.LogError("GameViewSize.displayText property not found."); + return null; + // Note: Do not use Exception (and Assert). Because freezes async tests on UTF v1.3.4, See UUM-25085. } var displayText = displayTextProperty.GetValue(_instance); diff --git a/Tests/RuntimeInternals/SceneManagerHelperTest.cs b/Tests/RuntimeInternals/SceneManagerHelperTest.cs index d1620a3..ffe3e6a 100644 --- a/Tests/RuntimeInternals/SceneManagerHelperTest.cs +++ b/Tests/RuntimeInternals/SceneManagerHelperTest.cs @@ -1,8 +1,7 @@ // Copyright (c) 2023-2024 Koji Hasegawa. // This software is released under the MIT License. -using System; -using System.IO; +using System.Text.RegularExpressions; using System.Threading.Tasks; using Cysharp.Threading.Tasks; using NUnit.Framework; @@ -54,23 +53,32 @@ public void GetExistScenePath_ExistPath_GotExistScenePath(string path) Assert.That(actual, Is.EqualTo(ExistScenePath)); } - [TestCase("Packages/**/NotInScenesInBuild.unity")] - [TestCase("**/NotInScenesInBuild.unity")] - [TestCase("Packages/com.nowsprinting.test-helper/Tests/Scenes/NotInScenesInBuild")] - [TestCase("Packages/com.nowsprinting.test-helper/Tests/Scenes/Not??ScenesInBuild.unity")] - [TestCase("Packages/com.nowsprinting.test-helper/Tests/Scenes/*InScenesInBuild.unity")] - public void GetExistScenePath_InvalidGlobPattern_ThrowsArgumentException(string path) + [TestCase("**/NotInScenesInBuild.unity", "Scene path must start with `Assets/` or `Packages/`")] + [TestCase("Packages/com.nowsprinting.test-helper/Tests/Scenes/NotInScenesInBuild", + "Scene path must ends with `.unity`")] + [TestCase("Packages/**/NotInScenesInBuild.unity", "Wildcards cannot be used in the package name of path")] + [TestCase("Packages/com.nowsprinting.test-helper/Tests/Scenes/Not??ScenesInBuild.unity", + "Wildcards cannot be used in the scene name of path")] + [TestCase("Packages/com.nowsprinting.test-helper/Tests/Scenes/*InScenesInBuild.unity", + "Wildcards cannot be used in the scene name of path")] + public void GetExistScenePath_InvalidGlobPattern_OutputLogError(string path, string expected) { - Assert.That(() => SceneManagerHelper.GetExistScenePath(path, null), Throws.TypeOf()); + var actual = SceneManagerHelper.GetExistScenePath(path, null); + + Assert.That(actual, Is.Null); + LogAssert.Expect(LogType.Error, new Regex(expected)); } [TestCase("Packages/com.nowsprinting.test-helper/Tests/Scenes/NotExistScene.unity")] // Not exist path [TestCase("Packages/com.nowsprinting.test-helper/*/NotInScenesInBuild.unity")] // Not match path pattern [UnityPlatform(RuntimePlatform.OSXEditor, RuntimePlatform.WindowsEditor, RuntimePlatform.LinuxEditor)] - public void GetExistScenePath_NotExistPath_InEditor_ThrowsFileNotFoundException(string path) + // Note: Returns scene name when running on player. + public void GetExistScenePath_NotExistPath_InEditor_OutputLogError(string path) { - Assert.That(() => SceneManagerHelper.GetExistScenePath(path, null), Throws.TypeOf()); - // Note: Returns scene name when running on player. + var actual = SceneManagerHelper.GetExistScenePath(path, null); + + Assert.That(actual, Is.Null); + LogAssert.Expect(LogType.Error, $"Scene `{path}` is not found in AssetDatabase"); } [TestCase("Packages/com.nowsprinting.test-helper/Tests/Scenes/NotInScenesInBuild.unity")]