Skip to content

Commit 49234f5

Browse files
ekcohK-Tone
andauthored
FIX: ISXB-1637 Crash on memcpy when calling Press() in an InputTestFixture (#2254)
Co-authored-by: Anthony Yakovlev <anthony.yakovlev@gmail.com>
1 parent 14d3e18 commit 49234f5

File tree

6 files changed

+294
-3
lines changed

6 files changed

+294
-3
lines changed
Lines changed: 226 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,226 @@
1+
using System;
2+
using System.Collections;
3+
using System.Linq;
4+
using NUnit.Framework;
5+
using UnityEngine;
6+
using UnityEngine.InputSystem;
7+
using UnityEngine.TestTools;
8+
using TouchPhase = UnityEngine.InputSystem.TouchPhase;
9+
10+
/// <summary>
11+
/// Test suite to verify test fixture API published in <see cref="InputTestFixture"/>.
12+
/// </summary>
13+
/// <remarks>
14+
/// This test fixture captures confusion around usage reported in
15+
/// https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1637.
16+
/// </remarks>
17+
internal class InputTestFixtureTests : InputTestFixture
18+
{
19+
private Keyboard correctlyUsedKeyboard;
20+
21+
private Keyboard incorrectlyUsedDevice;
22+
private Gamepad incorrectlyUsedGamepad;
23+
private Touchscreen incorrectlyUsedTouchscreen;
24+
25+
[OneTimeSetUp]
26+
public void UnitySetup()
27+
{
28+
// This is incorrect use since it will add a device to the actual input system since it executes before
29+
// the InputTestFixture.Setup() method. Hence, after Setup() has executed the device is not part of
30+
// the test input system instance.
31+
incorrectlyUsedDevice = InputSystem.AddDevice<Keyboard>();
32+
Assert.That(InputSystem.devices.Contains(incorrectlyUsedDevice), Is.True);
33+
34+
incorrectlyUsedGamepad = InputSystem.AddDevice<Gamepad>();
35+
Assert.That(InputSystem.devices.Contains(incorrectlyUsedGamepad), Is.True);
36+
37+
incorrectlyUsedTouchscreen = InputSystem.AddDevice<Touchscreen>();
38+
Assert.That(InputSystem.devices.Contains(incorrectlyUsedTouchscreen), Is.True);
39+
}
40+
41+
[OneTimeTearDown]
42+
public void UnityTearDown()
43+
{
44+
// Once InputTestFixture.TearDown() has executed, the state stack will have been popped and the correctlyUsedKeyboard
45+
// we added before entering the test fixture should have been restored.
46+
Assert.That(InputSystem.devices.Contains(incorrectlyUsedDevice), Is.True);
47+
Assert.That(InputSystem.devices.Contains(incorrectlyUsedGamepad), Is.True);
48+
Assert.That(InputSystem.devices.Contains(incorrectlyUsedTouchscreen), Is.True);
49+
}
50+
51+
[SetUp]
52+
public override void Setup()
53+
{
54+
// At this point we are still using the actual system so our device created from UnitySetup() should still
55+
// exist with the context.
56+
Assert.That(InputSystem.devices.Contains(incorrectlyUsedDevice), Is.True);
57+
Assert.That(InputSystem.devices.Contains(incorrectlyUsedGamepad), Is.True);
58+
Assert.That(InputSystem.devices.Contains(incorrectlyUsedTouchscreen), Is.True);
59+
60+
// This is expected usage pattern, first calling base.Setup() when overriding Setup like this, then
61+
// creating a fake device via the test fixture instance that only lives with the test context.
62+
base.Setup();
63+
64+
// Since we have now entered a temporary test state our device created in UnitySetup() will no longer exist
65+
// with this context.
66+
Assert.That(InputSystem.devices.Contains(incorrectlyUsedDevice), Is.False);
67+
Assert.That(InputSystem.devices.Contains(incorrectlyUsedGamepad), Is.False);
68+
Assert.That(InputSystem.devices.Contains(incorrectlyUsedTouchscreen), Is.False);
69+
}
70+
71+
[TearDown]
72+
public override void TearDown()
73+
{
74+
// Restore state
75+
base.TearDown();
76+
77+
// Our test device should no longer exist with the system since we are back to real instance
78+
Assert.That(InputSystem.devices.Contains(correctlyUsedKeyboard), Is.False);
79+
80+
Assert.That(InputSystem.devices.Contains(incorrectlyUsedDevice), Is.True);
81+
Assert.That(InputSystem.devices.Contains(incorrectlyUsedGamepad), Is.True);
82+
Assert.That(InputSystem.devices.Contains(incorrectlyUsedTouchscreen), Is.True);
83+
}
84+
85+
#region Editor playmode tests
86+
87+
[Test]
88+
public void Press_ShouldMutateDeviceState_WithinPlayModeTestFixtureContext()
89+
{
90+
correctlyUsedKeyboard = InputSystem.AddDevice<Keyboard>();
91+
Press(correctlyUsedKeyboard.spaceKey);
92+
Assert.That(correctlyUsedKeyboard.spaceKey.isPressed, Is.True);
93+
}
94+
95+
[Test]
96+
public void Press_ShouldThrow_WithinPlayModeTestFixtureContextIfInvalidDevice()
97+
{
98+
Assert.Throws<ArgumentException>(() => Press(incorrectlyUsedDevice.spaceKey));
99+
}
100+
101+
[Test]
102+
public void Release_ShouldMutateDeviceState_WithinPlayModeTestFixtureContext()
103+
{
104+
correctlyUsedKeyboard = InputSystem.AddDevice<Keyboard>();
105+
Press(correctlyUsedKeyboard.spaceKey);
106+
Release(correctlyUsedKeyboard.spaceKey);
107+
Assert.That(correctlyUsedKeyboard.spaceKey.isPressed, Is.False);
108+
}
109+
110+
[Test]
111+
public void Release_ShouldThrow_WithinPlayModeTestFixtureContextIfInvalidDevice()
112+
{
113+
Assert.Throws<ArgumentException>(() => Release(incorrectlyUsedDevice.spaceKey));
114+
}
115+
116+
[Test]
117+
public void PressAndRelease_ShouldMutateDeviceState_WithinPlayModeTestFixtureContext()
118+
{
119+
correctlyUsedKeyboard = InputSystem.AddDevice<Keyboard>();
120+
PressAndRelease(correctlyUsedKeyboard.spaceKey);
121+
Assert.That(correctlyUsedKeyboard.spaceKey.isPressed, Is.False);
122+
}
123+
124+
[Test]
125+
public void PressAndRelease_ShouldThrow_WithinPlayModeTestFixtureContextIfInvalidDevice()
126+
{
127+
Assert.Throws<ArgumentException>(() => PressAndRelease(incorrectlyUsedDevice.spaceKey));
128+
}
129+
130+
[Test]
131+
public void Click_ShouldMutateDeviceState_WithinPlayModeTestFixtureContext()
132+
{
133+
correctlyUsedKeyboard = InputSystem.AddDevice<Keyboard>();
134+
Click(correctlyUsedKeyboard.spaceKey);
135+
Assert.That(correctlyUsedKeyboard.spaceKey.isPressed, Is.False);
136+
}
137+
138+
[Test]
139+
public void Click_ShouldThrow_WithinPlayModeTestFixtureContextIfInvalidDevice()
140+
{
141+
Assert.Throws<ArgumentException>(() => Click(incorrectlyUsedDevice.spaceKey));
142+
}
143+
144+
[Test]
145+
public void Move_ShouldMutateDeviceState_WithinPlayModeTestFixtureContext()
146+
{
147+
var gamepad = InputSystem.AddDevice<Gamepad>();
148+
Move(gamepad.leftStick, new Vector2(1.0f, 0.0f));
149+
Assert.That(gamepad.leftStick.value, Is.EqualTo(new Vector2(1.0f, 0.0f)));
150+
}
151+
152+
[Test]
153+
public void Move_ShouldThrow_WithinPlayModeTestFixtureContextIfInvalidDevice()
154+
{
155+
Assert.Throws<ArgumentException>(() => Move(incorrectlyUsedGamepad.leftStick, new Vector2(1.0f, 0.0f)));
156+
}
157+
158+
[Test]
159+
public void Touch_ShouldMutateDeviceState_WithinPlayModeTestFixtureContext()
160+
{
161+
var touchscreen = InputSystem.AddDevice<Touchscreen>();
162+
BeginTouch(1, Vector2.zero, screen: touchscreen);
163+
Assert.That(touchscreen.touches.Count, Is.EqualTo(10));
164+
Assert.That(touchscreen.touches[0].touchId.value, Is.EqualTo(1));
165+
Assert.That(touchscreen.touches[0].position.value, Is.EqualTo(Vector2.zero));
166+
Assert.That(touchscreen.touches[0].phase.value, Is.EqualTo(TouchPhase.Began));
167+
168+
MoveTouch(1, Vector2.one, screen: touchscreen);
169+
Assert.That(touchscreen.touches.Count, Is.EqualTo(10));
170+
Assert.That(touchscreen.touches[0].touchId.value, Is.EqualTo(1));
171+
Assert.That(touchscreen.touches[0].position.value, Is.EqualTo(Vector2.one));
172+
Assert.That(touchscreen.touches[0].phase.value, Is.EqualTo(TouchPhase.Moved));
173+
174+
EndTouch(1, Vector2.zero, screen: touchscreen);
175+
Assert.That(touchscreen.touches.Count, Is.EqualTo(10));
176+
Assert.That(touchscreen.touches[0].touchId.value, Is.EqualTo(1));
177+
Assert.That(touchscreen.touches[0].position.value, Is.EqualTo(Vector2.zero));
178+
Assert.That(touchscreen.touches[0].phase.value, Is.EqualTo(TouchPhase.Ended));
179+
}
180+
181+
[Test]
182+
public void Touch_ShouldThrow_WithinPlayModeTestFixtureContextIfInvalidDevice()
183+
{
184+
Assert.Throws<ArgumentException>(() => BeginTouch(1, Vector2.zero, screen: incorrectlyUsedTouchscreen));
185+
Assert.Throws<ArgumentException>(() => MoveTouch(1, Vector2.zero, screen: incorrectlyUsedTouchscreen));
186+
Assert.Throws<ArgumentException>(() => EndTouch(1, Vector2.zero, screen: incorrectlyUsedTouchscreen));
187+
}
188+
189+
#endregion // Playmode tests
190+
191+
#region // Edit-mode tests
192+
193+
[UnityTest]
194+
public IEnumerator Press_ShouldThrow_WithinEditModeTestFixtureContext()
195+
{
196+
correctlyUsedKeyboard = InputSystem.AddDevice<Keyboard>();
197+
Assert.Throws<NotSupportedException>(() => Press(correctlyUsedKeyboard.spaceKey));
198+
yield break;
199+
}
200+
201+
[UnityTest]
202+
public IEnumerator Release_ShouldThrow_WithinEditModeTestFixtureContext()
203+
{
204+
correctlyUsedKeyboard = InputSystem.AddDevice<Keyboard>();
205+
Assert.Throws<NotSupportedException>(() => Release(correctlyUsedKeyboard.spaceKey));
206+
yield break;
207+
}
208+
209+
[UnityTest]
210+
public IEnumerator PressAndRelease_ShouldThrow_WithinEditModeTestFixtureContext()
211+
{
212+
correctlyUsedKeyboard = InputSystem.AddDevice<Keyboard>();
213+
Assert.Throws<NotSupportedException>(() => PressAndRelease(correctlyUsedKeyboard.spaceKey));
214+
yield break;
215+
}
216+
217+
[UnityTest]
218+
public IEnumerator Click_ShouldThrow_WithinEditModeTestFixtureContext()
219+
{
220+
correctlyUsedKeyboard = InputSystem.AddDevice<Keyboard>();
221+
Assert.Throws<NotSupportedException>(() => Click(correctlyUsedKeyboard.spaceKey));
222+
yield break;
223+
}
224+
225+
#endregion // Edit-mode tests
226+
}

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

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Packages/com.unity.inputsystem/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ however, it has to be formatted properly to pass verification tests.
1212

1313
### Fixed
1414
- Fixed warnings being generated on Unity 6.3 (beta). (ISXB-1718).
15+
- Fixed an issue in `DeltaStateEvent.From` where unsafe code would throw exception or crash if internal pointer `currentStatePtr` was `null`. [ISXB-1637](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1637).
16+
- 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).
1517

1618
## [1.15.0] - 2025-10-03
1719

Packages/com.unity.inputsystem/InputSystem/Events/DeltaStateEvent.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,8 @@ public static NativeArray<byte> From(InputControl control, out InputEventPtr eve
7676
if (!device.added)
7777
throw new ArgumentException($"Device for control '{control}' has not been added to system",
7878
nameof(control));
79+
if (control.currentStatePtr == null) // Protects statePtr assignment below
80+
throw new ArgumentNullException($"Control '{control}' does not have an associated state");
7981

8082
ref var deviceStateBlock = ref device.m_StateBlock;
8183
ref var controlStateBlock = ref control.m_StateBlock;

Packages/com.unity.inputsystem/InputSystem/InputManager.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1188,6 +1188,11 @@ private void NotifyUsageChanged(InputDevice device)
11881188

11891189
////TODO: make sure that no device or control with a '/' in the name can creep into the system
11901190

1191+
internal bool HasDevice(InputDevice device)
1192+
{
1193+
return device.m_DeviceIndex < m_DevicesCount && ReferenceEquals(m_Devices[device.m_DeviceIndex], device);
1194+
}
1195+
11911196
public InputDevice AddDevice(Type type, string name = null)
11921197
{
11931198
if (type == null)

Packages/com.unity.inputsystem/Tests/TestFixture/InputTestFixture.cs

Lines changed: 56 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,17 @@ namespace UnityEngine.InputSystem
5959
/// input and device discovery or removal notifications from platform code. This ensures
6060
/// that while the test is running, input that may be generated on the machine running
6161
/// the test will not infer with it.
62+
///
63+
/// Be cautious when using <c>NUnit.Framework.OneTimeSetUpAttribute</c> and
64+
/// <c>NUnit.Framework.OneTimeTearDownAttribute</c> in combination with this test fixture.
65+
/// For example, any devices created prior to execution of <see cref="Setup()"/> would be added to the actual
66+
/// Input System instead of the test fixture system and after <see cref="Setup()"/> has executed such devices
67+
/// will no longer be valid. You may of course use these NUnit features, but it is advised to not attempt affecting
68+
/// the Input System under test from those methods since it would affect the real system and not the system
69+
/// under test.
70+
///
71+
/// This test fixture is designed for play-mode tests and is generally not supported for edit-mode tests.
72+
/// Both <c>[Test]</c> and <c>[UnityTest]</c> are supported, but only in play-mode.
6273
/// </remarks>
6374
public class InputTestFixture
6475
{
@@ -533,6 +544,12 @@ public void Set<TValue>(InputDevice device, string path, TValue state, double ti
533544
/// Note that this parameter will be ignored if the test is a <c>[UnityTest]</c>. Multi-frame
534545
/// playmode tests will automatically process input as part of the Unity player loop.</param>
535546
/// <typeparam name="TValue">Value type of the given control.</typeparam>
547+
/// <exception cref="ArgumentNullException">If control is null.</exception>
548+
/// <exception cref="ArgumentException">If the device associated with <paramref name="control"/> has not
549+
/// been added to the system or if the control does not have any associated state. The latter may only
550+
/// happen if attempting to set a control of a device created outside the test context.</exception>
551+
/// <exception cref="NotSupportedException">If attempting to set a control of a test device in an
552+
/// editor assembly. [UnityTest] in editor assemblies is not supported by this test fixture.</exception>
536553
/// <example>
537554
/// <code>
538555
/// var gamepad = InputSystem.AddDevice&lt;Gamepad&gt;();
@@ -544,12 +561,14 @@ public void Set<TValue>(InputControl<TValue> control, TValue state, double time
544561
{
545562
if (control == null)
546563
throw new ArgumentNullException(nameof(control));
547-
if (!control.device.added)
548-
throw new ArgumentException(
549-
$"Device of control '{control}' has not been added to the system", nameof(control));
564+
CheckValidity(control.device, control);
550565

551566
if (IsUnityTest())
567+
{
568+
if (IsEditMode())
569+
throw new NotSupportedException("InputTestFixture.Set does not support edit mode (editor assembly) [UnityTest].");
552570
queueEventOnly = true;
571+
}
553572

554573
void SetUpAndQueueEvent(InputEventPtr eventPtr)
555574
{
@@ -663,6 +682,7 @@ public void SetTouch(int touchId, TouchPhase phase, Vector2 position, float pres
663682
if (screen == null)
664683
screen = InputSystem.AddDevice<Touchscreen>();
665684
}
685+
CheckValidity(screen);
666686

667687
InputSystem.QueueStateEvent(screen, new TouchState
668688
{
@@ -913,6 +933,39 @@ internal void SimulateDomainReload()
913933

914934
#endif
915935

936+
private static void CheckValidity(InputDevice device, InputControl control)
937+
{
938+
if (!device.added)
939+
{
940+
throw new ArgumentException(
941+
$"Device '{device}' has not been added to the system", nameof(device));
942+
}
943+
944+
// Guards against a device from another scope being used. This is a direct way to evaluate whether
945+
// the device is associated with the current manager state or not since device state isn't consistently
946+
// pushed/popped in the current design.
947+
var manager = InputSystem.s_Manager;
948+
if (manager == null || !manager.HasDevice(device))
949+
{
950+
throw new ArgumentException($"Control '{control}' does not have any associated state. " +
951+
"Make sure the control or device was added after executing Setup().", nameof(control));
952+
}
953+
}
954+
955+
private static void CheckValidity(InputControl control)
956+
{
957+
CheckValidity(control.device, control);
958+
}
959+
960+
/// <summary>
961+
/// Returns true if running inside an Edit Mode test (Editor assembly).
962+
/// Returns false if running in Play Mode.
963+
/// </summary>
964+
private static bool IsEditMode()
965+
{
966+
return Application.isEditor && !Application.isPlaying;
967+
}
968+
916969
#if UNITY_EDITOR
917970
/// <summary>
918971
/// Represents an analytics registration event captured by test harness.

0 commit comments

Comments
 (0)