NEW: Test case for monotonic timestamps. #2262
Conversation
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr-agent |
PR Code Suggestions ✨No code suggestions found for the PR. |
Codecov ReportAll modified and coverable lines are covered by tests ✅ @@ Coverage Diff @@
## develop #2262 +/- ##
===========================================
+ Coverage 76.70% 76.81% +0.10%
===========================================
Files 465 476 +11
Lines 87919 88659 +740
===========================================
+ Hits 67442 68101 +659
- Misses 20477 20558 +81 Flags with carried forward coverage won't be shown. Click here to find out more.
... and 25 files with indirect coverage changes 🚀 New features to boost your workflow:
|
ekcoh
changed the title
|
This currently fails for Android due to package containing Android specific path which ignores timestamps. |
| // Accepted: equal | ||
| InputSystem.QueueStateEvent(device, new KeyboardState(Key.Space), 1.0); | ||
| InputSystem.Update(); | ||
| Assert.That(device.spaceKey.ReadValue(), NUnit.Framework.Is.EqualTo(1.0f).Within(1e-6)); |
There was a problem hiding this comment.
I'm wondering why we don't use the already defined using Is = UnityEngine.TestTools.Constraints.Is that we use a lot across all codebase tests?
The same applies to other NUnit.Framework.Is.Equal calls below.
There was a problem hiding this comment.
Will probably switch to UnityEngine.TestTools.Constraints.Is but it's just terribly annoying to have hundreds of code warnings inside the file leading to the quick navigation in scrollbar being unusable due to all warnings. The above Is extends the class I used and that generates a warning, e.g. "you should not access static functions via derived class", which makes sense. But maybe we should just add the required pragmas to the test classes to see actual warnings and fix them if this noise is removed.
There was a problem hiding this comment.
I create a separate PR to fix these messy using/aliasing.
Good question, I want to sync up with Android team how they decide to land and backport the pending PR first. Then we either need to add an exception to the test (until it can be removed from InputManager), or let this just sit here until it can be removed from Input Manager. It might be that the exception becomes version specific, but ideally not. Other ideas @jfreire-unity ? |
Description
As part of a pending Unity PR we ended up in a discussion whether Input System requires strictly increasing timestamps or not. This PR proves with an automated tests that timestamps do not have to be strictly incrementing, but should have timestamps greater or equal to the last received timestamp for the same source (device).
Skipping CHANGELOG update since test only.
Testing status & QA
Adds a single test.
Overall Product Risks
Comments to reviewers
Nothing special.
Checklist
Before review:
Changed,Fixed,Addedsections.Area_CanDoX,Area_CanDoX_EvenIfYIsTheCase,Area_WhenIDoX_AndYHappens_ThisIsTheResult.During merge:
NEW: ___.FIX: ___.DOCS: ___.CHANGE: ___.RELEASE: 1.1.0-preview.3.After merge: