Make sure to set the scan code for KEYBDINPUT #33434
Conversation
This call only works when the tests are run as an administrator. For local testing, the call will always fail because the tests run as a normal user. For CI testing, the call is simply unnecessary because the machines are not connected to physical input devices (keyboard/mouse) that would be otherwise able to interfere with the test sequence.
| throw new ExternalException("Sending input failed because input was blocked by another thread.", hresult); | ||
| } | ||
| } | ||
| ResetKeyState(VirtualKeyCode.LCONTROL); |
There was a problem hiding this comment.
Before doing the reset, is it worth checking if the key is still active, so we can know if we're leaking modifier keys somewhere?
There was a problem hiding this comment.
I was not able to find a leak.
| default: | ||
| Debug.WriteLine($"ERROR: Encountered illegal input type: {input.Type}"); | ||
| break; | ||
| // Slight delay on Esc to ensure the Ctrl key isn't registered as down |
There was a problem hiding this comment.
Better to spin until it's not? That'll doubly help if it turns out there's a deeper issue, because then we'll see hangs in a place where we'll understand what's up.
There was a problem hiding this comment.
(Also maybe link to a tracking bug? Since it's not obvious to me why Escape is more special than any other modifier here. Yes, it has the added affect of launching explorer, but Ctrl+any other letter may invoke random VS commands which will confuse us just as much.)
There was a problem hiding this comment.
Ctrl+Esc is handled by the DWM, not Visual Studio. I couldn't find any evidence that the Ctrl key state is latching inside Visual Studio. The same cannot be said when pairing keyboard and mouse handling inside the IDE.
There was a problem hiding this comment.
I was not able to reproduce this locally even with high-speed uninterrupted toggling of Ctrl and Esc in thousands of iterations, so I haven't found any reason to believe a specific spin mechanism would be effective in the scenario.
| using Microsoft.CodeAnalysis.Serialization; | ||
| using Microsoft.CodeAnalysis.Text; | ||
| using Roslyn.Utilities; | ||
|
|
||
| #if VALIDATE_CHECKSUM |
There was a problem hiding this comment.
Why all the fancy conditional compilation? I assume we'd be fixing this ASAP?
There was a problem hiding this comment.
Why all the fancy conditional compilation?
It excludes the implementation helper from release builds.
I assume we'd be fixing this ASAP?
Definitely hoping we do. It seems like a serious issue.
|
|
||
| if (inputBlocked) | ||
| { | ||
| IntegrationHelper.UnblockInput(); |
There was a problem hiding this comment.
Any guess if this was causing problems for us?
There was a problem hiding this comment.
Nothing conclusive. I eliminated it simply so the integration test runs are a closer match to the state when devs run the tests. This one has a commit message with rationale for the change.
jasonmalinowski
dismissed
their stale review
PR was changed significantly, and a few small questions/suggestions now.
Integration tests require active focus, so we set it there before SendInput calls and allow it to stay there.
|
Superseded by #33524 |
See michaelnoonan/inputsimulator#13