Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update gamepad helper #3152

Conversation

marcelwgn
Copy link
Collaborator

Description

This updates the GamePad helper with the hope to improve the reliability for tests that inject GamePad input.

General idea was: Press button, wait 50 milliseconds, lift button.

New idea: Press button, wait for testframe to report input was received, wait for idle and lift button again.
If the testframe doesn't have the checkbox to report it received the input, we fallback to the old behavior.

Motivation and Context

How Has This Been Tested?

Ran tests multiple times, though they were reliable on my machine before this change too.

How should we test this?

I think we should run CI with the changes, if everything goes fine, we can try reenabling tests and run the CI multiple times to determine if those tests are reliable enough.

The tests that are currently disabled due to gamepad issues are:

  • RatingControlTests.GamepadTest
  • TreeViewTests.TreeViewMultiSelectGamepadTest_ContentMode
  • RadioButtonsTests.GamepadCanEscapeAndDoesNotSelectWithFocus

Screenshots (if appropriate):

@msft-github-bot msft-github-bot added the needs-triage Issue needs to be triaged by the area owners label Aug 18, 2020
@StephenLPeters
Copy link
Contributor

@chingucoding can you pull master to pick up the CI build fix that just went in?

{
Wait.ForMilliseconds(10);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer using something like this

checkBox.GetToggledWaiter().TryWait(TimeSpan.FromMilliseconds(millisecondsTimeout));

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switched to that. Is there any documentation on this? I can't seem to find any helpful documents on the Microsoft.Windows.Apps.Test.Foundation namespace or TAEF 🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think unfortunately not, @kmahone might know something

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also just to be sure, TryWait in this case tries to either wait for the value to change or continue after 50 milliseconds right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be able to see the source by using F12 in VS. I think that after the timespan this will throw, and I think there is an optional parameter to make it continue without throwing instead

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hitting F12 only gives me this:
image

@StephenLPeters
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@StephenLPeters StephenLPeters added area-TestInfrastructure Issue in the test infrastructure (e.g. in Helix scripts) team-Controls Issue for the Controls team and removed needs-triage Issue needs to be triaged by the area owners labels Aug 18, 2020
Copy link
Contributor

@StephenLPeters StephenLPeters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-TestInfrastructure Issue in the test infrastructure (e.g. in Helix scripts) team-Controls Issue for the Controls team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants