From 5cc05c24caedd5e9c33715b64165832ff62e206e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Akan=20Sidenvall?= Date: Mon, 6 Oct 2025 20:27:17 +0200 Subject: [PATCH 1/3] Extended documentation of InputTestFixture. --- .../InputTestFixtureTests.cs | 112 ++++++++++++++++++ .../InputTestFixtureTests.cs.meta | 3 + .../Tests/TestFixture/InputTestFixture.cs | 6 + 3 files changed, 121 insertions(+) create mode 100644 Assets/Tests/InputSystem.Editor/InputTestFixtureTests.cs create mode 100644 Assets/Tests/InputSystem.Editor/InputTestFixtureTests.cs.meta diff --git a/Assets/Tests/InputSystem.Editor/InputTestFixtureTests.cs b/Assets/Tests/InputSystem.Editor/InputTestFixtureTests.cs new file mode 100644 index 0000000000..02399b94c4 --- /dev/null +++ b/Assets/Tests/InputSystem.Editor/InputTestFixtureTests.cs @@ -0,0 +1,112 @@ +using System.Collections; +using NUnit.Framework; +using UnityEngine.InputSystem; +using UnityEngine.TestTools; + +// Attempting to simplify code from ISXB-1637 in this file. + +// What is this? Its a play-mode test defined in an editor assembly. +// This is similar to tests of input system itself. It works as expected. +public class InputTestFixtureTestsV1 : InputTestFixture +{ + private Keyboard _keyboard; + + [SetUp] + public void Setup() + { + base.Setup(); + _keyboard = InputSystem.AddDevice(); + } + + [TearDown] + public void TearDown() + { + InputSystem.RemoveDevice(_keyboard); + base.TearDown(); + } + + [Test] + public void Test1() + { + Press(_keyboard.spaceKey); + } + + [Test] + public void Test2() + { + Press(_keyboard.spaceKey); + } +} + +// What is this? Its a play-mode test defined in an editor assembly. +// It uses OneTimeSetUp and OneTimeTearDown to create device which collides with SetUp and TearDown behavior +// of the inherited InputTestFixture. +// +// Results in (Similar to reported stack trace): +// System.ArgumentNullException : Value cannot be null. +// Parameter name: source +// --- +// at (wrapper managed-to-native) Unity.Collections.LowLevel.Unsafe.UnsafeUtility.MemCpy(void*,void*,long) +// at UnityEngine.InputSystem.LowLevel.DeltaStateEvent.From (UnityEngine.InputSystem.InputControl control, +// UnityEngine.InputSystem.LowLevel.InputEventPtr& eventPtr, Unity.Collections.Allocator allocator) [0x000e5] +// in Packages/com.unity.inputsystem/InputSystem/Events/DeltaStateEvent.cs:99 +public class InputTestFixtureTestsV2 : InputTestFixture +{ + private Keyboard _keyboard; + + [OneTimeSetUp] + public void UnitySetup() + { + _keyboard = InputSystem.AddDevice(); + } + + [OneTimeTearDown] + public void UnityTearDown() + { + InputSystem.RemoveDevice(_keyboard); + } + + [Test] + public void Test1() + { + Press(_keyboard.spaceKey); + } + + [Test] + public void Test2() + { + Press(_keyboard.spaceKey); + } +} + +// What is this? This is similar to the repro project for this bug. +public class InputTestFixtureTestsV3 : InputTestFixture +{ + private Keyboard _keyboard; + + [OneTimeSetUp] + public void UnitySetup() + { + _keyboard = InputSystem.AddDevice(); + } + + [OneTimeTearDown] + public void UnityTearDown() + { + InputSystem.RemoveDevice(_keyboard); + } + + [UnityTest] + public IEnumerator Test1() + { + Press(_keyboard.spaceKey); + yield break; + } + + [UnityTest] + public IEnumerator Test2() + { + Press(_keyboard.spaceKey); + yield break; + } +} diff --git a/Assets/Tests/InputSystem.Editor/InputTestFixtureTests.cs.meta b/Assets/Tests/InputSystem.Editor/InputTestFixtureTests.cs.meta new file mode 100644 index 0000000000..8cb308e72d --- /dev/null +++ b/Assets/Tests/InputSystem.Editor/InputTestFixtureTests.cs.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: c3455fa1a90b4c5683d9f36ec3ff075a +timeCreated: 1759764564 \ No newline at end of file diff --git a/Packages/com.unity.inputsystem/Tests/TestFixture/InputTestFixture.cs b/Packages/com.unity.inputsystem/Tests/TestFixture/InputTestFixture.cs index 2727be91be..316d7545d6 100644 --- a/Packages/com.unity.inputsystem/Tests/TestFixture/InputTestFixture.cs +++ b/Packages/com.unity.inputsystem/Tests/TestFixture/InputTestFixture.cs @@ -59,6 +59,12 @@ namespace UnityEngine.InputSystem /// input and device discovery or removal notifications from platform code. This ensures /// that while the test is running, input that may be generated on the machine running /// the test will not infer with it. + /// + /// Be cautious when using and + /// in combination with this test fixture. + /// Any devices or Input System related setup prior to will be invalid after this + /// test fixture is set up. You may of course use these NUnit features but should not affect the Input System + /// under test from those methods. /// public class InputTestFixture { From d133a2ddf4b1b1b8bfdb5ab4ce61851ffbb5248b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Akan=20Sidenvall?= Date: Mon, 6 Oct 2025 20:30:08 +0200 Subject: [PATCH 2/3] Updated xmldoc of InputTestFixture --- .../Tests/TestFixture/InputTestFixture.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Packages/com.unity.inputsystem/Tests/TestFixture/InputTestFixture.cs b/Packages/com.unity.inputsystem/Tests/TestFixture/InputTestFixture.cs index 316d7545d6..cac8d144bb 100644 --- a/Packages/com.unity.inputsystem/Tests/TestFixture/InputTestFixture.cs +++ b/Packages/com.unity.inputsystem/Tests/TestFixture/InputTestFixture.cs @@ -62,9 +62,10 @@ namespace UnityEngine.InputSystem /// /// Be cautious when using and /// in combination with this test fixture. - /// Any devices or Input System related setup prior to will be invalid after this - /// test fixture is set up. You may of course use these NUnit features but should not affect the Input System - /// under test from those methods. + /// For example, any devices created prior to execution of would be added to the actual + /// Input System instead of the test fixture system and after has executed such devices + /// will no longer be valid. You may of course use these NUnit features but it is advised to not attemp affecting + /// the Input System under test from those methods. /// public class InputTestFixture { From d355b21e4367e73d7c3ad905554bb8af0b342aba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Akan=20Sidenvall?= Date: Tue, 7 Oct 2025 10:46:26 +0200 Subject: [PATCH 3/3] FIX: Fixed an issue where accessing a device from an outer context within a test fixture context would lead to exception or crash. --- .../InputTestFixtureTests.cs | 158 +++++++++++------- Packages/com.unity.inputsystem/CHANGELOG.md | 4 +- .../InputSystem/Controls/InputControl.cs | 3 + .../InputSystem/Events/DeltaStateEvent.cs | 2 + .../Tests/TestFixture/InputTestFixture.cs | 12 +- 5 files changed, 114 insertions(+), 65 deletions(-) diff --git a/Assets/Tests/InputSystem.Editor/InputTestFixtureTests.cs b/Assets/Tests/InputSystem.Editor/InputTestFixtureTests.cs index 02399b94c4..5429c90f7b 100644 --- a/Assets/Tests/InputSystem.Editor/InputTestFixtureTests.cs +++ b/Assets/Tests/InputSystem.Editor/InputTestFixtureTests.cs @@ -1,112 +1,146 @@ +using System; using System.Collections; +using System.Linq; using NUnit.Framework; using UnityEngine.InputSystem; using UnityEngine.TestTools; -// Attempting to simplify code from ISXB-1637 in this file. - -// What is this? Its a play-mode test defined in an editor assembly. -// This is similar to tests of input system itself. It works as expected. -public class InputTestFixtureTestsV1 : InputTestFixture +/// +/// Test suite to verify test fixture API published in . +/// +/// +/// This test fixture captures confusion around usage reported in +/// https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1637. +/// +internal class InputTestFixtureTests : InputTestFixture { - private Keyboard _keyboard; + private Keyboard correctlyUsedDevice; + private Keyboard incorrectlyUsedDevice; + + [OneTimeSetUp] + public void UnitySetup() + { + // This is incorrect use since it will add a device to the actual input system since it executes before + // the InputTestFixture.Setup() method. Hence, after Setup() has executed the device is not part of + // the test input system instance. + incorrectlyUsedDevice = InputSystem.AddDevice(); + Assert.That(InputSystem.devices.Contains(incorrectlyUsedDevice), Is.True); + } + + [OneTimeTearDown] + public void UnityTearDown() + { + // Once InputTestFixture.TearDown() has executed, the state stack will have been popped and the keyboard + // we added before entering the test fixture should have been restored. + Assert.That(InputSystem.devices.Contains(incorrectlyUsedDevice), Is.True); + } [SetUp] - public void Setup() + public override void Setup() { + // At this point we are still using the actual system so our device created from UnitySetup() should still + // exist with the context. + Assert.That(InputSystem.devices.Contains(incorrectlyUsedDevice), Is.True); + + // This is expected usage pattern, first calling base.Setup() when overriding Setup like this, then + // creating a fake device via the test fixture instance that only lives with the test context. base.Setup(); - _keyboard = InputSystem.AddDevice(); + correctlyUsedDevice = InputSystem.AddDevice(); + + // Since we have now entered a temporary test state our device created in UnitySetup() will no longer exist + // with this context. + Assert.That(InputSystem.devices.Contains(incorrectlyUsedDevice), Is.False); } [TearDown] - public void TearDown() + public override void TearDown() { - InputSystem.RemoveDevice(_keyboard); + // This is expected usage pattern, we might want to do something with the device here, but it needs + // to happen before base.TearDown() since it would delete the fake device. + Assert.That(InputSystem.devices.Contains(correctlyUsedDevice), Is.True); + InputSystem.RemoveDevice(correctlyUsedDevice); + + // Restore state base.TearDown(); + + // Our test device should no longer exist with the system since we are back to real instance + Assert.That(InputSystem.devices.Contains(correctlyUsedDevice), Is.False); } + #region Editor playmode tests + [Test] - public void Test1() + public void Press_ShouldMutateDeviceState_WithinPlayModeTestFixtureContext() { - Press(_keyboard.spaceKey); + Press(correctlyUsedDevice.spaceKey); + Assert.That(correctlyUsedDevice.spaceKey.isPressed, Is.True); } [Test] - public void Test2() + public void Press_ShouldThrow_WithinPlayModeTestFixtureContextIfInvalidDevice() { - Press(_keyboard.spaceKey); + Assert.That(incorrectlyUsedDevice.spaceKey.isPressed, Is.False); + Assert.Throws(() => Press(incorrectlyUsedDevice.spaceKey)); } -} - -// What is this? Its a play-mode test defined in an editor assembly. -// It uses OneTimeSetUp and OneTimeTearDown to create device which collides with SetUp and TearDown behavior -// of the inherited InputTestFixture. -// -// Results in (Similar to reported stack trace): -// System.ArgumentNullException : Value cannot be null. -// Parameter name: source -// --- -// at (wrapper managed-to-native) Unity.Collections.LowLevel.Unsafe.UnsafeUtility.MemCpy(void*,void*,long) -// at UnityEngine.InputSystem.LowLevel.DeltaStateEvent.From (UnityEngine.InputSystem.InputControl control, -// UnityEngine.InputSystem.LowLevel.InputEventPtr& eventPtr, Unity.Collections.Allocator allocator) [0x000e5] -// in Packages/com.unity.inputsystem/InputSystem/Events/DeltaStateEvent.cs:99 -public class InputTestFixtureTestsV2 : InputTestFixture -{ - private Keyboard _keyboard; - [OneTimeSetUp] - public void UnitySetup() + [Test] + public void Release_ShouldMutateDeviceState_WithinPlayModeTestFixtureContext() { - _keyboard = InputSystem.AddDevice(); + Press(correctlyUsedDevice.spaceKey); + Release(correctlyUsedDevice.spaceKey); + Assert.That(correctlyUsedDevice.spaceKey.isPressed, Is.False); } - [OneTimeTearDown] - public void UnityTearDown() + [Test] + public void Release_ShouldThrow_WithinPlayModeTestFixtureContextIfInvalidDevice() { - InputSystem.RemoveDevice(_keyboard); + Assert.That(incorrectlyUsedDevice.spaceKey.isPressed, Is.False); + Assert.Throws(() => Release(incorrectlyUsedDevice.spaceKey)); } [Test] - public void Test1() + public void PressAndRelease_ShouldMutateDeviceState_WithinPlayModeTestFixtureContext() { - Press(_keyboard.spaceKey); + PressAndRelease(correctlyUsedDevice.spaceKey); + Assert.That(correctlyUsedDevice.spaceKey.isPressed, Is.False); } [Test] - public void Test2() + public void PressAndRelease_ShouldThrow_WithinPlayModeTestFixtureContextIfInvalidDevice() { - Press(_keyboard.spaceKey); + PressAndRelease(incorrectlyUsedDevice.spaceKey); + Assert.Throws(() => PressAndRelease(incorrectlyUsedDevice.spaceKey)); } -} -// What is this? This is similar to the repro project for this bug. -public class InputTestFixtureTestsV3 : InputTestFixture -{ - private Keyboard _keyboard; - - [OneTimeSetUp] - public void UnitySetup() + [Test] + public void Click_ShouldMutateDeviceState_WithinPlayModeTestFixtureContext() { - _keyboard = InputSystem.AddDevice(); + Click(correctlyUsedDevice.spaceKey); + Assert.That(correctlyUsedDevice.spaceKey.isPressed, Is.False); } - [OneTimeTearDown] - public void UnityTearDown() + [Test] + public void Click_ShouldThrow_WithinPlayModeTestFixtureContextIfInvalidDevice() { - InputSystem.RemoveDevice(_keyboard); + Click(correctlyUsedDevice.spaceKey); + Assert.Throws(() => Click(incorrectlyUsedDevice.spaceKey)); } - [UnityTest] - public IEnumerator Test1() - { - Press(_keyboard.spaceKey); - yield break; - } + // TODO Add remaining + + #endregion // Playmode tests + + #region // Edit-mode tests [UnityTest] - public IEnumerator Test2() + public IEnumerator Press_ShouldMutateDeviceState_WithinEditModeTestFixtureContext() { - Press(_keyboard.spaceKey); - yield break; + Press(correctlyUsedDevice.spaceKey); + yield return null; // Need to yield control to see change in next frame when running in edit-mode + Assert.That(correctlyUsedDevice.spaceKey.isPressed, Is.True); } + + // TODO Add remaining tests + + #endregion // Edit-mode tests } diff --git a/Packages/com.unity.inputsystem/CHANGELOG.md b/Packages/com.unity.inputsystem/CHANGELOG.md index a691fdd3c9..7d8c0a43db 100644 --- a/Packages/com.unity.inputsystem/CHANGELOG.md +++ b/Packages/com.unity.inputsystem/CHANGELOG.md @@ -10,7 +10,9 @@ however, it has to be formatted properly to pass verification tests. ## [Unreleased] - yyyy-mm-dd - +### Fixed +- Fixed an issue in `DeltaStateEvent.From` where unsafe code would throw exception or crash if internal pointer `currentStatePtr` would be `null`. [ISXB-1637](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1637). +- Fixed an issue in `InputTestFixture.Set` where attempting to change state of a device not belonging to the test fixture context would result in null pointer exception or crash. [ISXB-1637](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1637). ## [1.15.0] - 2025-10-03 diff --git a/Packages/com.unity.inputsystem/InputSystem/Controls/InputControl.cs b/Packages/com.unity.inputsystem/InputSystem/Controls/InputControl.cs index 4634d67af7..ff29db8010 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Controls/InputControl.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Controls/InputControl.cs @@ -996,6 +996,9 @@ protected internal uint stateOffsetRelativeToDeviceRoot } } + // Allows determining if the device has an associated state pointer + internal unsafe bool hasState => currentStatePtr != null; + // This data is initialized by InputDeviceBuilder. internal InternedString m_Name; internal string m_Path; diff --git a/Packages/com.unity.inputsystem/InputSystem/Events/DeltaStateEvent.cs b/Packages/com.unity.inputsystem/InputSystem/Events/DeltaStateEvent.cs index 9002279341..32dc3d9983 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Events/DeltaStateEvent.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Events/DeltaStateEvent.cs @@ -76,6 +76,8 @@ public static NativeArray From(InputControl control, out InputEventPtr eve if (!device.added) throw new ArgumentException($"Device for control '{control}' has not been added to system", nameof(control)); + if (control.currentStatePtr == null) // Protects statePtr assignment below + throw new ArgumentNullException($"Control '{control}' do not have an associated state"); ref var deviceStateBlock = ref device.m_StateBlock; ref var controlStateBlock = ref control.m_StateBlock; diff --git a/Packages/com.unity.inputsystem/Tests/TestFixture/InputTestFixture.cs b/Packages/com.unity.inputsystem/Tests/TestFixture/InputTestFixture.cs index cac8d144bb..587c8790b3 100644 --- a/Packages/com.unity.inputsystem/Tests/TestFixture/InputTestFixture.cs +++ b/Packages/com.unity.inputsystem/Tests/TestFixture/InputTestFixture.cs @@ -64,8 +64,9 @@ namespace UnityEngine.InputSystem /// in combination with this test fixture. /// For example, any devices created prior to execution of would be added to the actual /// Input System instead of the test fixture system and after has executed such devices - /// will no longer be valid. You may of course use these NUnit features but it is advised to not attemp affecting - /// the Input System under test from those methods. + /// will no longer be valid. You may of course use these NUnit features, but it is advised to not attempt affecting + /// the Input System under test from those methods since it would affect the real system and not the system + /// under test. /// public class InputTestFixture { @@ -552,8 +553,15 @@ public void Set(InputControl control, TValue state, double time if (control == null) throw new ArgumentNullException(nameof(control)); if (!control.device.added) + { throw new ArgumentException( $"Device of control '{control}' has not been added to the system", nameof(control)); + } + if (!control.hasState) + { + throw new ArgumentException($"Control '{control}' does not have any associated state. " + + "Make sure the control or device was added after executing Setup().", nameof(control)); + } if (IsUnityTest()) queueEventOnly = true;