Additional simplification, fixes and test cases. Added test assembly to support serialization within editor test. Added test utilities.

Author: ekcoh

Assets/Tests/InputSystem.Editor/EditorPrefsTestUtils.cs

@@ -0,0 +1,52 @@
+using UnityEditor;
+
+namespace Tests.InputSystem.Editor
+{
+    /// <summary>
+    /// Utility to simplify editor tests with respect to editor preferences.
+    /// </summary>
+    internal static class EditorPrefsTestUtils
+    {
+        private const string EnterPlayModeOptionsEnabledKey = "EnterPlayModeOptionsEnabled";
+        private const string EnterPlayModeOptionsKey = "EnterPlayModeOptions";
+
+        private static bool _savedEnterPlayModeOptionsEnabled;
+        private static int _savedEnterPlayModeOptions;
+
+        /// <summary>
+        /// Call this from a tests SetUp routine to save editor preferences so they can be restored after the test.
+        /// </summary>
+        public static void SaveEditorPrefs()
+        {
+            _savedEnterPlayModeOptionsEnabled = EditorPrefs.GetBool(EnterPlayModeOptionsEnabledKey, false);
+            _savedEnterPlayModeOptions = EditorPrefs.GetInt(EnterPlayModeOptionsKey, (int)EnterPlayModeOptions.None);
+        }
+
+        /// <summary>
+        /// Call this from a tests TearDown routine to restore editor preferences to the state it had before the test.
+        /// </summary>
+        public static void RestoreEditorPrefs()
+        {
+            EditorPrefs.SetBool(EnterPlayModeOptionsEnabledKey, _savedEnterPlayModeOptionsEnabled);
+            EditorPrefs.SetInt(EnterPlayModeOptionsKey, _savedEnterPlayModeOptions);
+        }
+
+        /// <summary>
+        /// Call this from within a test to temporarily enable domain reload.
+        /// </summary>
+        public static void EnableDomainReload()
+        {
+            EditorPrefs.SetBool(EnterPlayModeOptionsEnabledKey, false);
+        }
+
+        /// <summary>
+        /// Call this from within a test to temporarily disable domain reload (and scene reloads).
+        /// </summary>
+        public static void DisableDomainReload()
+        {
+            EditorPrefs.SetBool(EnterPlayModeOptionsEnabledKey, true);
+            EditorPrefs.SetInt(EnterPlayModeOptionsKey, (int)(EnterPlayModeOptions.DisableDomainReload |
+                EnterPlayModeOptions.DisableSceneReload));
+        }
+    }
+}

Assets/Tests/InputSystem.Editor/EditorPrefsTestUtils.cs.meta

@@ -0,0 +1,176 @@
+using System;
+using System.Collections;
+using System.Linq;
+using NUnit.Framework;
+using UnityEditor;
+using UnityEditor.SceneManagement;
+using UnityEngine;
+using UnityEngine.InputSystem;
+using UnityEngine.InputSystem.Editor;
+using UnityEngine.SceneManagement;
+using UnityEngine.TestTools;
+using Object = UnityEngine.Object;
+
+namespace Tests.InputSystem.Editor
+{
+    /// <summary>
+    /// Editor tests for <see cref="UnityEngine.InputSystem.InputActionReference"/>.
+    /// </summary>
+    /// <remarks>
+    /// This test need fixed asset paths since mid-test domain reloads would otherwise discard data.
+    ///
+    /// Be aware that if you get failures in editor tests that switch between play mode and edit mode via coroutines
+    /// you might get misleading stack traces that indicate errors in different places than they actually happen.
+    /// At least this have been observered for exception stack traces.
+    /// </remarks>
+    internal class InputActionReferenceEditorTests
+    {
+        private Scene m_Scene;
+
+        private const string assetPath = "Assets/__InputActionReferenceEditorTestsActions.inputactions";
+        private const string dummyPath = "Assets/__InputActionReferenceEditorTestsDummy.asset";
+        private const string scenePath = "Assets/__InputActionReferenceEditorTestsScene.unity";
+
+        private void CreateAsset()
+        {
+            var asset = ScriptableObject.CreateInstance<InputActionAsset>();
+
+            var map1 = new InputActionMap("map1");
+            map1.AddAction("action1");
+            map1.AddAction("action2");
+            asset.AddActionMap(map1);
+
+            System.IO.File.WriteAllText(assetPath, asset.ToJson());
+            Object.DestroyImmediate(asset);
+            AssetDatabase.ImportAsset(assetPath);
+        }
+
+        [SetUp]
+        public void SetUp()
+        {
+            // This looks odd, but when we yield into play mode from our test coroutine we may get a domain reload
+            // (depending on editor preferences) which will trigger another SetUp() mid-test.
+            if (Application.isPlaying)
+                return;
+
+            m_Scene = EditorSceneManager.NewScene(NewSceneSetup.EmptyScene);
+            CreateAsset();
+
+            var go = new GameObject("Root");
+            var behaviour = go.AddComponent<InputActionBehaviour>();
+            var reference = InputActionImporter.LoadInputActionReferencesFromAsset(assetPath).First(
+                r => "action1".Equals(r.action.name));
+            behaviour.referenceAsField = reference;
+            behaviour.referenceAsReference = reference;
+
+            TestUtils.SaveScene(m_Scene, scenePath);
+        }
+
+        [TearDown]
+        public void TearDown()
+        {
+            // This looks odd, but when we yield into play mode from our test coroutine we may get a domain reload
+            // (depending on editor preferences) which will trigger another TearDown() mid-test.
+            if (Application.isPlaying)
+                return;
+
+            // Close scene
+            EditorSceneManager.CloseScene(m_Scene, true);
+
+            // Clean-up
+            AssetDatabase.DeleteAsset(dummyPath);
+            AssetDatabase.DeleteAsset(assetPath);
+            AssetDatabase.DeleteAsset(scenePath);
+        }
+
+        private static InputActionBehaviour GetBehaviour() => Object.FindObjectOfType<InputActionBehaviour>();
+        private static InputActionAsset GetAsset() => AssetDatabase.LoadAssetAtPath<InputActionAsset>(assetPath);
+
+        // For unclear reason, NUnit fails to assert throwing exceptions after transition into play-mode.
+        // So until that can be sorted out, we do it manually (in the same way) ourselves.
+        private static void AssertThrows<T>(Action action) where T : Exception
+        {
+            var exceptionThrown = false;
+            try
+            {
+                action();
+            }
+            catch (InvalidOperationException)
+            {
+                exceptionThrown = true;
+            }
+            Assert.IsTrue(exceptionThrown, $"Expected exception of type {typeof(T)} to be thrown but it was not.");
+        }
+
+        [UnityTest]
+        [Description("https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1584")]
+        public IEnumerator ReferenceSetInPlaymodeShouldBeRestored_WhenExitingPlaymode()
+        {
+            // Edit-mode section
+            {
+                // Sanity check our initial edit-mode state
+                var obj = GetBehaviour();
+                Assert.That(obj.referenceAsField.action, Is.SameAs(GetAsset().FindAction("map1/action1")));
+                Assert.That(obj.referenceAsReference.action, Is.SameAs(GetAsset().FindAction("map1/action1")));
+
+                // Enter play-mode (This will lead to domain reload by default).
+                yield return new EnterPlayMode();
+            }
+
+            // Play-mode section
+            {
+                var obj = GetBehaviour();
+                var editModeAction = GetAsset().FindAction("map1/action1");
+                var playModeAction = GetAsset().FindAction("map1/action2");
+
+                // Make sure our action reference is consistent in play-mode
+                Assert.That(obj.referenceAsField.action, Is.SameAs(editModeAction));
+
+                // ISXB-1584: Attempting assignment of persisted input action reference in play-mode in editor.
+                // Rationale: We cannot allow this since it would corrupt the source asset since changes applied to SO
+                // mapped to an asset isn't reverted when exiting play-mode.
+                //
+                // Here we would like to do:
+                //      Assert.Throws<InvalidOperationException>(() => obj.reference.Set(null));
+                //
+                // But we can't since it would fail with NullReferenceException.
+                // It turns out that because of the domain reload / Unity’s internal serialization quirks, the obj is
+                // sometimes null inside the lambda when NUnit captures it for execution. So to work around this we
+                // instead do the same kind of check manually for now which doesn't seem to have this problem.
+                //
+                // It is odd since NUnit does basically does the same thing (apart from wrapping the lambda as a
+                // TestDelegate). So the WHY for this problem remains unclear for now.
+                AssertThrows<InvalidOperationException>(() => obj.referenceAsField.Set(playModeAction));
+                AssertThrows<InvalidOperationException>(() => obj.referenceAsReference.Set(editModeAction));
+
+                // Make sure there were no side-effects.
+                Assert.That(obj.referenceAsField.action, Is.SameAs(editModeAction));
+                Assert.That(obj.referenceAsReference.action, Is.SameAs(editModeAction));
+
+                // Correct usage is to use a run-time assigned input action reference instead. It is up to the user
+                // to decide whether this reference should additionally be persisted (which is possible by saving it to
+                // and asset, or by using SerializeReference).
+                obj.referenceAsField = InputActionReference.Create(playModeAction);
+                obj.referenceAsReference = InputActionReference.Create(playModeAction);
+
+                // Makes sure we have the expected reference.
+                Assert.That(obj.referenceAsField.action, Is.SameAs(playModeAction));
+                Assert.That(obj.referenceAsReference.action, Is.SameAs(playModeAction));
+
+                // Exit play-mode (This will lead to domain reload by default).
+                yield return new ExitPlayMode();
+            }
+
+            // Edit-mode section
+            {
+                // Make sure our reference is back to its initial edit mode state
+                var obj = GetBehaviour();
+                var editModeAction = GetAsset().FindAction("map1/action1");
+                Assert.That(obj.referenceAsField.action, Is.SameAs(editModeAction));
+                Assert.That(obj.referenceAsReference.action, Is.SameAs(editModeAction));
+            }
+
+            yield return null;
+        }
+    }
+}

Assets/Tests/InputSystem.Editor/InputActionReferenceEditorTests.cs

@@ -1,7 +1,12 @@
+using System;
+using System.Linq;
+using UnityEditor;
+using UnityEditor.SceneManagement;
 using UnityEngine.InputSystem;
 using UnityEngine.InputSystem.Editor;
+using UnityEngine.SceneManagement;
 
-// Replicated in this test assembly to avoid building public API picked up by PackageValidator
+// Replicated in this test assembly to avoid building public API picked up by PackageValidator.
 internal class TestUtils
 {
 #if UNITY_EDITOR
@@ -24,5 +29,85 @@ public static void RestoreDialogs()
         Dialog.ControlScheme.SetDeleteControlScheme(null);
     }
 
+    /// <summary>
+    /// Returns the (default) name of a scene asset created at the given path.
+    /// </summary>
+    /// <param name="scenePath">The path to the asset.</param>
+    /// <returns>Scene name.</returns>
+    public static string GetSceneName(string scenePath)
+    {
+        return System.IO.Path.GetFileNameWithoutExtension(scenePath);
+    }
+
+    /// <summary>
+    /// Saves a scene to the given path.
+    /// </summary>
+    /// <param name="scene">The scene to be saved.</param>
+    /// <param name="scenePath">The target scene path.</param>
+    /// <param name="makeDirty">If true, force saves the scene by making it explicitly dirty.</param>
+    /// <exception cref="Exception">Throws exception if failed to save scene
+    /// (if dirty or <paramref name="makeDirty"/> was true)</exception>
+    public static void SaveScene(Scene scene, string scenePath, bool makeDirty = true)
+    {
+        var wasDirty = scene.isDirty;
+        if (makeDirty)
+            EditorSceneManager.MarkSceneDirty(scene);
+        var wasSaved = EditorSceneManager.SaveScene(scene, scenePath);
+        if (!wasSaved && (wasDirty || makeDirty))
+            throw new Exception($"Failed to save scene to {scenePath}");
+    }
+
+    /// <summary>
+    /// Adds a scene asset to build settings unless it is already present.
+    /// </summary>
+    /// <param name="scenePath">The scene path to the scene asset.</param>
+    /// <returns>Value tuple where first argument (int) specifies the build index of the scene and the second
+    /// argument (bool) specifies whether the scene was added (true) or already present (false).</returns>
+    public static ValueTuple<int, bool> AddSceneToEditorBuildSettings(string scenePath)
+    {
+        // Determine if scene is already present or not.
+        var existingScenes = EditorBuildSettings.scenes;
+        var index = FindSceneIndexByPath(existingScenes, scenePath);
+        if (index >= 0)
+            return new ValueTuple<int, bool>(index, false);
+
+        // Add the new scene.
+        var newScenes = new EditorBuildSettingsScene[existingScenes.Length + 1];
+        Array.Copy(existingScenes, newScenes, existingScenes.Length);
+        newScenes[existingScenes.Length] = new EditorBuildSettingsScene(scenePath, true);
+        EditorBuildSettings.scenes = newScenes;
+
+        return new ValueTuple<int, bool>(existingScenes.Length, true);
+    }
+
+    /// <summary>
+    /// Removes a scene asset from editor build settings given its path.
+    /// </summary>
+    /// <param name="scenePath">The asset path of the scene to be removed.</param>
+    /// <returns>true if the scene was removed, false if it do not exist within editor build settings scenes.</returns>
+    public static bool RemoveSceneFromEditorBuildSettings(string scenePath)
+    {
+        var existingScenes = EditorBuildSettings.scenes;
+        var index = FindSceneIndexByPath(existingScenes, scenePath);
+        if (index < 0)
+            return false;
+
+        var newScenes = new EditorBuildSettingsScene[existingScenes.Length - 1];
+        Array.Copy(existingScenes, newScenes, index);
+        Array.Copy(existingScenes, index + 1, newScenes, index, existingScenes.Length - index - 1);
+        EditorBuildSettings.scenes = newScenes;
+        return true;
+    }
+
+    private static int FindSceneIndexByPath(EditorBuildSettingsScene[] scenes, string scenePath)
+    {
+        for (var i = 0; i < scenes.Length; ++i)
+        {
+            if (scenes[i].path == scenePath)
+                return i;
+        }
+        return -1;
+    }
+
 #endif // UNITY_EDITOR
 }

Assets/Tests/InputSystem.Editor/InputActionReferenceEditorTests.cs.meta

@@ -5,7 +5,8 @@
         "UnityEngine.TestRunner",
         "UnityEditor.TestRunner",
         "Unity.InputSystem",
-        "Unity.InputSystem.TestFramework"
+        "Unity.InputSystem.TestFramework",
+        "Unity.InputSystem.TestSupport"
     ],
     "includePlatforms": [
         "Editor"

Assets/Tests/InputSystem.Editor/InputActionsEditorTests.cs.meta

@@ -0,0 +1,18 @@
+using UnityEngine;
+using UnityEngine.InputSystem;
+
+// This class and this assembly should NOT be considered API, it is only public and its own assembly for the following
+// reasons:
+// 1) Unity only supports serialization of public types stored in a file with the same name.
+//    If this wasn't the case, this type would be internal to the test assembly.
+// 2) Editor test assemblies cannot contain MonoBehaviour that should be added to scene game objects.
+//    Hence, this assembly needs to be a regular assembly and referenced by editor test assembly to use this
+//    MonoBehaviour in a scene.
+//
+// Note that we serialize both as field and reference. Note that this should not make any difference for Unity.Object
+// types, but is included for completeness.
+public sealed class InputActionBehaviour : MonoBehaviour
+{
+    [SerializeField] public InputActionReference referenceAsField;
+    [SerializeReference] public InputActionReference referenceAsReference;
+}