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

[infra] Improve test app UIs and usability, switch to fast pass rendering #4426

Merged

Conversation

marcelwgn
Copy link
Contributor

Description

Improved multiple test pages to be usable without UIA software, also switched to fast pass rendering where reasonable.

For math fun facts, check out the commit messages of my commits (non merge commits).

Motivation and Context

Make test app more usable and friendly.

How Has This Been Tested?

Tested manually, also ran selection of interaction tests.

Screenshots (if appropriate):

@ghost ghost added the needs-triage Issue needs to be triaged by the area owners label Mar 5, 2021
@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 Mar 6, 2021
@StephenLPeters
Copy link
Contributor

What is the goal of these changes?

@marcelwgn marcelwgn force-pushed the user/chingucoding/testapp-ui-improvements branch from f787f12 to a70820f Compare March 6, 2021 08:22
@marcelwgn
Copy link
Contributor Author

@StephenLPeters Pretty much the same as #1793. Improve UI, fix pages where not everything could be accessed without UIA on CI dimensions or the pages where hard to use due to difficult contrast ratios. As part of this, I also went through all the TextBlocks and updated them to use fast pass rendering. Not sure though why the commit history looks the way it does though. Also the latest commit was an accident, removed that now.

@marcelwgn marcelwgn force-pushed the user/chingucoding/testapp-ui-improvements branch from 97c43bb to a70820f Compare March 7, 2021 23:23
… way that you can assemble two spheres out of it with same volume (Banach Tarski Paradox)
…""not listable""), there exists a well-ordering of the real numbers (ZFC & axiom of choice)
…athematics went through a crisis since there was no clear axiomatic foundation for Math; this was eventually solved by using set theory (ZFC) as foundation
…of digits, it is neither poven nor disproven that pi contains any sequence of digits (i.e. it is a normal number). most normal numbers we know are constructed (e.g. Champerowne's constant)
…he only holomorphic (complex differentiable) functions with a global maximum are constants, all other holomorphic functions have no global maximum besides on the domain boundary
@marcelwgn marcelwgn force-pushed the user/chingucoding/testapp-ui-improvements branch from a70820f to a3aed91 Compare March 8, 2021 00:34
@marcelwgn
Copy link
Contributor Author

Fixed the commit history, sorry that so many folks were pinged by my lacking rebasing skills. This commit history makes a lot more sense.

@@ -142,7 +142,7 @@ public void FocusComingFromAnotherRepeaterTest()
VerifySelectedFocusedIndex(2, true);

// Move focus to the first RadioButtons control by pressing TAB again
KeyboardHelper.PressKey(Key.Tab);
KeyboardHelper.PressKey(Key.Tab, ModifierKey.Shift);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this intentional ?

Copy link
Contributor Author

@marcelwgn marcelwgn Mar 9, 2021

Choose a reason for hiding this comment

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

Yes, the old test assumed that moving forward in tab order loops back to the first RadioButtons since we hid the test app bar and thus move to the first control in the page. However since we no longer hide the testapp bar, we need to move backwards instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

Curious as to why you are removing the bit to hide test app bar?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IIRC, you couldn't really leave the app after that since the only way to get back was using some off screen elements that are triggered through UIA when executing tests.

@ranjeshj
Copy link
Contributor

ranjeshj commented Mar 9, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@StephenLPeters
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ranjeshj
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@marcelwgn
Copy link
Contributor Author

All tests should work now, could we merge this PR then or are there other things that need addressing?

@StephenLPeters
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@marcelwgn
Copy link
Contributor Author

@StephenLPeters A few verification tests failed, but I have no idea why since those aren't actually affected by the test app UI are they? Also if we have verification fails, shouldn't the drop contain the updated files? My guess is that it was just a bad run, maybe the merge with main fixes this.

@StephenLPeters
Copy link
Contributor

@StephenLPeters A few verification tests failed, but I have no idea why since those aren't actually affected by the test app UI are they? Also if we have verification fails, shouldn't the drop contain the updated files? My guess is that it was just a bad run, maybe the merge with main fixes this.

We actually changed how the pipelines are organized a bit which resulted in some changes to the VisualVerification test work flow. The updated documentation is in the developer guide: https://github.com/microsoft/microsoft-ui-xaml/blob/main/docs/developer_guide.md#visual-tree-verification-tests.

Can you post a link to the ADO pipeline and I'll take a look. (or I can dig it up if you don't have it handy.)

@StephenLPeters
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@marcelwgn
Copy link
Contributor Author

I think I found out why a lot of tests failed: I accidently checked in a change to the test app where it used the Version1 styles instead of the Version2 styles which of course broke a lot of verification tests (and some other tests too).

@StephenLPeters
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@karkarl
Copy link
Contributor

karkarl commented Nov 8, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@karkarl
Copy link
Contributor

karkarl commented Nov 10, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@@ -7,7 +7,7 @@
xmlns:local="using:MUXControlsTestApp"
xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
xmlns:media="using:Microsoft.UI.Xaml.Media"
xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" xmlns:contract5Present="http://schemas.microsoft.com/winfx/2006/xaml/presentation?IsApiContractPresent(Windows.Foundation.UniversalApiContract, 5)"
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: new line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, fixed now (albeit a nit).

@karkarl
Copy link
Contributor

karkarl commented Nov 10, 2021

I manually compared the before and after MUXControlsTestApps and they look great! Thanks for doing this.

@karkarl
Copy link
Contributor

karkarl commented Nov 10, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@marcelwgn
Copy link
Contributor Author

Thank you for reviewing this large PR @karenbtlai !!!

@karkarl karkarl merged commit feb7029 into microsoft:main Nov 10, 2021
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.

4 participants