Test TextBox with a TextBoxMask.Mask and x:Bind #3512

Rosuavio · 2020-09-30T19:48:21Z

Add UI tests for TextBox with a TextBoxMask.Mask and x:Bind
Uses WinUI Test Infrasttcure from #3482 to test the fix from #3338 for #3335.

PR Type

What kind of change does this PR introduce?

Feature

Build or CI related changes

What is the current behavior?

No UI Tests for TextBoxMask

What is the new behavior?

Given a TextBox with a Mask set and its Text bound to a C# property using x:Bind. It tests if the text in the TextBox updates when that property is updated.

PR Checklist

Please check if your PR fulfills the following requirements:

Tested code with current supported SDKs
~~[ ] Pull Request has been submitted to the documentation repository instructions. Link:~~
~~[ ] Sample in sample app has been added / updated (for bug fixes / features) ~~
- ~~[ ] Icon has been created (if new sample) following the Thumbnail Style Guide and templates ~~
Tests for the changes have been added (for bug fixes / features) (if applicable)
Header has been added to all new source files (run build/UpdateHeaders.bat)
Contains NO breaking changes

Other information

Does not test #3514

ghost · 2020-09-30T19:48:29Z

Thanks RosarioPulella for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

net-foundation-cla · 2020-09-30T19:48:39Z

All CLA requirements met.

net-foundation-cla · 2020-09-30T19:48:39Z

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

❌ RosarioPulella sign now
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

michael-hawker · 2020-10-15T17:10:14Z

Changed the base of this PR to the other branch for now so it's easier to review/comment on until that one gets merged, then we can switch back.

michael-hawker

Just want to make sure we use this PR to set ourselves up for adding more tests as we move forward from here! 🙂

UITests/UITests.App/MainPage.xaml

UITests/UITests.App/Pages/TextBoxMask.xaml

michael-hawker · 2020-10-15T17:14:53Z

UITests/UITests.App/UITests.App.csproj

    <Compile Include="MainPage.xaml.cs">
      <DependentUpon>MainPage.xaml</DependentUpon>
    </Compile>
+    <Compile Include="Pages\SimpleTest.xaml.cs">


@azchohfi can we use the SDK Extras to switch to the new-style project here? Don't want to get into merge problems in the future with multiple-tests being written.

Otherwise we can try using the glob syntax instead?

michael-hawker · 2020-10-15T17:16:15Z

UITests/UITests.App/MainPage.xaml

-            <TextBlock x:Name="textBlock"/>
+        <Frame x:Name="navigationFrame" />
+
+        <StackPanel Orientation="Horizontal" Grid.Row="1" >


However, I don't like the use of the buttons here to control the page navigation, we should build this into the infrastructure itself. So the MainPage has a function of understanding how to switch tasks and can evaluate a string and navigate to that page based on reflection or something, similar to how we navigate within the SampleApp currently:

https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/49760c325ecbddab254b61214e5a808e866e51d6/Microsoft.Toolkit.Uwp.SampleApp/Models/Sample.cs#L100

michael-hawker · 2020-10-15T17:18:07Z

UITests/UITests.Tests.Shared/Tests.cs

        }
+
+        [TestMethod]
+        public void TestTextBoxMaskBinding_Property()


I definitely want to figure out how we split these into multiple files as well, so we can have multiple test cases per test page. i.e. we can have a test page using the component with a setup or two, and then have multiple smaller test methods to run against it.

Though ideally, we figure out how to manage this without tearing down and re-opening the app each time?

@azchohfi as commented on the other PR, do you know how we'd structure this here or have suggestions?

UITests/UITests.App/Pages/TextBoxMask.xaml.cs

michael-hawker · 2020-10-15T17:32:09Z

UITests/UITests.Tests.Shared/Tests.cs

+            Verify.AreEqual(newValue, textBox.GetText());
+        }
+
+        private static void OpenTest(string name)


Yeah, so this method should be integrated into the test-harness itself and know how to communicate with the MainPage in order to switch the page in the frame. We don't want to have to rely on adding more custom buttons and things to the main test page in order to do this.

As mentioned in the other PR I think the use of a custom Attribute could be handy if we could make that work. i.e.

[TestMethod] [TestXamlFile("TextBoxMaskTestPage")] public void TestTextBoxMaskBinding_Property() {

The test harness before each test would read the attribute on it to grab the type name of the page to load, then we'd have to send the test app a message to load that page, and then execute the test.

Not sure if we would need an intermediary UI piece, use the clipboard, or go as heavy as an AppService to do this. Looks like in WinUI they just manipulate the UI similar to how you did it here, just more generally with a command: https://github.com/microsoft/microsoft-ui-xaml/blob/master/test/TestAppUtils/NavigateToTestCommand.cs#L12

And they use their TestSetupHelper to manually control the page at the start of each test: https://github.com/microsoft/microsoft-ui-xaml/blob/master/dev/TabView/InteractionTests/TabViewTests.cs#L49

@azchohfi thoughts? Should we just copy the setup WinUI has or should we try and improve upon this somehow with an attribute on the test method itself to control this?

I'm guessing it is easier if the idea is to better introp to WinUI that we more closely follow their patterns? @ranjeshj any preferences on if we experiment or try to more closely align? Has the current WinUI system been working well or have you been looking to changing it in the future?

I guess I left out that part of why I want to try and simplify these connections is to make it as straight-forward and easy for developers to add integration tests so that we can ensure adding them isn't a big burden to our community.

I think if we could introduce a new attribute based system here, it'd give developers the flexibility to re-use a page easily or setup multiple pages for different scenarios, eh?

It also makes each test easier to maintain and understand as all the test harness initialization is abstracted to the attribute.

Basically we'd set something up like this to grab the attribute for the base of our test system:

public TestContext TestContext { get; set; } [TestInitialize] public void TestInitialize() { Debug.WriteLine("Running Test: " + TestContext.TestName); var currentlyRunningMethod = GetType().GetMethod(TestContext.TestName); var descriptionAttributes = currentlyRunningMethod.GetCustomAttributes(typeof(DescriptionAttribute), true) as System.Collections.Generic.IEnumerable<DescriptionAttribute>; var firstOne = descriptionAttributes.FirstOrDefault(); if (firstOne != null) { Debug.WriteLine("Description: " + firstOne.Description); } }

This is just grabbing a description attribute, but would work the same for our custom attribute and use that info to load the appropriate page in the Test App via test control commands as done currently or some other channel (AppService?).

In either case, I think we should have the test app load the requested page on demand vs. trying to determine all the pages ahead of time and load them at the start of the app. I think this would help with the test app starting time? Which would help if you're picking specific tests to run from VS, right?

Thoughts?

michael-hawker · 2020-10-16T22:57:03Z

Sorry @RosarioPulella looks like my change I did forced this to close unexpectedly.

Can you rebase your branch on top of master and then try force-pushing this branch to see if we can re-open the PR after that?

Rosuavio · 2020-10-19T14:50:31Z

@michael-hawker Just rebasing did not fix the PR. I would need to change what branch the PR is targeting, and GitHub does not seem to let me do that. Should I open a new PR? I'll make sure to reference all the requested changes.

ghost requested review from Kyaa-dost, azchohfi and michael-hawker September 30, 2020 19:48

Rosuavio changed the title ~~Test TextBoxMask~~ Test TextBox with a TextBoxMask.Mask and x:Bind Oct 1, 2020

Rosuavio marked this pull request as ready for review October 1, 2020 19:34

Kyaa-dost added the extensions ⚡ label Oct 1, 2020

michael-hawker added this to the 7.0 milestone Oct 6, 2020

Rosuavio added 2 commits October 12, 2020 16:03

Adpat UITests to support multiple tests

d7ca569

Added TextBoxMask binding to property test.

1850ad5

michael-hawker mentioned this pull request Oct 15, 2020

WinUI - Test Infra #3482

Merged

7 tasks

michael-hawker changed the base branch from master to muxtestinfra October 15, 2020 17:09

michael-hawker requested changes Oct 15, 2020

View reviewed changes

ghost added the needs author feedback 📝 label Oct 15, 2020

ghost closed this Oct 16, 2020

ghost deleted the branch CommunityToolkit:muxtestinfra October 16, 2020 22:28

Rosuavio mentioned this pull request Oct 19, 2020

Test TextBox with a TextBoxMask.Mask and x:Bind #3533

Closed

4 tasks

Rosuavio mentioned this pull request Nov 3, 2020

Test TextBox with a TextBoxMask.Mask and x:Bind #3552

Closed

4 tasks

This pull request was closed.

Test TextBox with a TextBoxMask.Mask and x:Bind #3512

Test TextBox with a TextBoxMask.Mask and x:Bind #3512

Uh oh!

Conversation

Rosuavio commented Sep 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Type

What is the current behavior?

What is the new behavior?

PR Checklist

Other information

Uh oh!

ghost commented Sep 30, 2020

Uh oh!

net-foundation-cla bot commented Sep 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

net-foundation-cla bot commented Sep 30, 2020

Uh oh!

michael-hawker commented Oct 15, 2020

Uh oh!

michael-hawker left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

michael-hawker Oct 15, 2020

Choose a reason for hiding this comment

Uh oh!

michael-hawker Oct 15, 2020

Choose a reason for hiding this comment

Uh oh!

michael-hawker Oct 15, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

michael-hawker Oct 15, 2020

Choose a reason for hiding this comment

Uh oh!

michael-hawker Oct 15, 2020

Choose a reason for hiding this comment

Uh oh!

michael-hawker Oct 15, 2020

Choose a reason for hiding this comment

Uh oh!

michael-hawker commented Oct 16, 2020

Uh oh!

Rosuavio commented Oct 19, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Rosuavio commented Sep 30, 2020 •

edited

Loading

net-foundation-cla bot commented Sep 30, 2020 •

edited

Loading