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

🧪 Add UI Tests for Gridsplitter #4090

Conversation

michael-hawker
Copy link
Member

Supports #4083

Decided to split out the test improvements and initial GridSplitter tests from my #4083 PR. This PR is ready to go and should be merged before it.

This adds a new ability to our communication pipe in order to execute custom function within the test host from the test harness tests. For instance in this case, we can try and get info from the VisualTree and pass back the result via reflection and serialization between the environment running the test and the place that's executing the test.

This was needed as Grid is not inspect-able by the automation methods used regularly in the test. We needed to look at the Grid object as that's what GridSplitter interacts with; however, we needed to use a UI test as it is controlled by user input (mouse).

This PR adds:

  • non-functional changes to GridSplitter XAML (XAML Styler applied)
  • new system for adding custom method calls in the communication pipe
  • Including handling serialization/deserialization using System.Text.Json
  • Method for retrieving a specific property off an object found in the VisualTree (searching down from the app root to a specific named element)
  • basic GridSplitter UI tests for basic manipulations/bounds checking with default settings

PR Type

What kind of change does this PR introduce?

  • Other... UI Tests 🧪

What is the current behavior?

Unable to introspect a Grid element within a UI test.

What is the new behavior?

Now able to get more detailed information about UI objects within a UI test.

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)
  • New major technical changes in the toolkit have or will be added to the Wiki e.g. build changes, source generators, testing infrastructure, sample creation changes, etc...
  • 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

@ghost
Copy link

ghost commented Jun 28, 2021

Thanks michael-hawker 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 🙌

@michael-hawker
Copy link
Member Author

FYI @azchohfi @RosarioPulella

I did also discover that GridSplitter has no way to manipulate the last column of a Grid... but this same limitation exists in WPF as well... Our logic code also seems much more complex than WPF's as well. Since we're all open-source and part of the .NET Foundation now, I may look at adopting the WPF logic more directly while refactoring in #4083.

Copy link
Contributor

@Rosuavio Rosuavio left a comment

Choose a reason for hiding this comment

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

I know re-running the UITests in CI did not show any issues, but I am worried that if we merge this we will inconstantly see a lot more CI failures on other branches. I ran the tests threw MSTest and TAEF and got errors when the test harness tried to open some samples.

MSTest log

 TestTextBoxMaskBinding_Property
   Source: TextBoxMaskTest.cs line 33
   Duration: 126 ms

  Message: 
    Initialization method UITests.Tests.TextBoxMaskTest.TestInitialize threw exception. System.InvalidOperationException: Test host didn't confirm test ready to execute page: TextBoxMaskTestPage.

  Stack Trace: 
    UITestBase.TestInitialize() line 93

  Standard Output: 
    Base Package Search Directory = "C:\Users\Rosario\source\repos\WindowsCommunityToolkit\UITests\UITests.App"
    Looking for CoreWindow...
    Found CoreWindow.
    Found test class "UITests.Tests.TextBoxMaskTest".
    Found test method "TestTextBoxMaskBinding_Property" in test class "UITests.Tests.TextBoxMaskTest".
    Found "UITests.Tests.TestPageAttribute" on test method "TestTextBoxMaskBinding_Property" in test class "UITests.Tests.TextBoxMaskTest". XamlFile: TextBoxMaskTestPage.
    [Harness] Sending Host Page Request: TextBoxMaskTestPage
    Status : Failure

TAEF log

Found CoreWindow.
Found test class "UITests.Tests.SimpleTest".
Found test method "SimpleTestMethod" in test class "UITests.Tests.SimpleTest".
Found "UITests.Tests.TestPageAttribute" on test method "SimpleTestMethod" in test class "UITests.Tests.SimpleTest". XamlFile: SimpleTestPage.
[Harness] Sending Host Page Request: SimpleTestPage
Status : Failure
Error: System.InvalidOperationException: Test host didn't confirm test ready to execute page: SimpleTestPage
   at UITests.Tests.UITestBase.TestInitialize() in C:\Users\Rosario\source\repos\WindowsCommunityToolkit\UITests\UITests.Tests.Shared\UITestBase.cs:line 93
   at WEX.TestExecution.TestClass.InvokeMethodImpl(Type testClassType, Object testClassInstance, String testMethodName, Boolean breakOnInvoke, TETestContext testContext, Boolean& testThrewException)
Saved screen capture to "C:\Users\Rosario\source\repos\WindowsCommunityToolkit\build\WexLogFileOutput\000001_Screenshot.jpg".
Error: TAEF: Setup fixture 'TestInitialize' for the scope 'UITests.Tests.SimpleTest.SimpleTestMethod' failed.
Saved screen capture to "C:\Users\Rosario\source\repos\WindowsCommunityToolkit\build\WexLogFileOutput\000002_Screenshot.jpg".

StartGroup: UITests.Tests.SimpleTest.SimpleTestMethod
Saved screen capture to "C:\Users\Rosario\source\repos\WindowsCommunityToolkit\build\WexLogFileOutput\000003_Screenshot.jpg".
EndGroup: UITests.Tests.SimpleTest.SimpleTestMethod [Failed]
Base Package Search Directory = "C:\Users\Rosario\source\repos\WindowsCommunityToolkit\UITests\UITests.App"
Closing application: {UITests.App, ApplicationFrameWindow, }
Invoking CloseAppInvoker {CloseApp, Button, __CloseAppInvoker}
Killing processes we might have inadvertently started...

Summary of Errors Outside of Tests:
    Error: System.InvalidOperationException: Test host didn't confirm test ready to execute page: SimpleTestPage
   at UITests.Tests.UITestBase.TestInitialize() in C:\Users\Rosario\source\repos\WindowsCommunityToolkit\UITests\UITests.Tests.Shared\UITestBase.cs:line 93
   at WEX.TestExecution.TestClass.InvokeMethodImpl(Type testClassType, Object testClassInstance, String testMethodName, Boolean breakOnInvoke, TETestContext testContext, Boolean& testThrewException)
    Error: TAEF: Setup fixture 'TestInitialize' for the scope 'UITests.Tests.SimpleTest.SimpleTestMethod' failed.

Summary of Non-passing Tests:
    UITests.Tests.SimpleTest.SimpleTestMethod [Failed]

Summary: Total=9, Passed=8, Failed=1, Blocked=0, Not Run=0, Skipped=0

@michael-hawker
Copy link
Member Author

@RosarioPulella I haven't modified the UITestBase or how the tests are initialized or executed at all here; so we shouldn't be seeing new failures. So this may have been an underlying issue exposed here, but having more tests again? Can you try running your main again and see if you see similar issues?

@Rosuavio
Copy link
Contributor

So this may have been an underlying issue exposed here, but having more tests again?

It could be, I am investigating. But what is clear is that at some point we request for a page, the harness has the right page an everything, and for some unknow reason when the request is sent threw the CommunicationService the request responds with a failure and the host never seems to have ever received the request (as there is no logs about the request). It could be a concurrency issue or we might be miss using the CommunicationService and it ends up in an undesired state.

Can you try running your main again and see if you see similar issues?

Not seeing these issues on main.

@michael-hawker
Copy link
Member Author

This is really weird, as I don't think we saw the same issue on the larger PR version of this, but that didn't have any additional changes to the tests. @azchohfi any thoughts on what may be going on here too?

@michael-hawker michael-hawker force-pushed the user/mhawker/gridsplitter-tests branch from f6b4cb9 to d397ecf Compare July 7, 2021 21:58
@michael-hawker
Copy link
Member Author

Passed the first time, re-running again to see if it'll pass again. Have a lot more output now about the service connection, so we can see if the app is getting the message or what type of response it may be getting back.

@michael-hawker
Copy link
Member Author

@RosarioPulella want to try again locally? Wondering if this is just a timing/threading thing, as the last two times with the additional log messages seem to be working for me in the CI...

@Rosuavio
Copy link
Contributor

Rosuavio commented Jul 8, 2021

I am still having the same issues on MSTest and TAEF locally. Perhaps we should subscribe to the CommunicationService.ServiceClosed event to see if the connection is closed early?

@michael-hawker
Copy link
Member Author

Good idea, I'll try adding that, seems like it's just a generic Failure we're getting back from the App Service which isn't too helpful.

Allows for access to VisualTree within UI Test
Uses System.Test.Json serialization to return UI object info between host/harness
Added intial basic GridSplitter tests to validate (though unsure why there's such a large variance in delta from drag... test is reporting moved the right number of requested pixels, so not sure if issue with delta vs. completed in GridSplitter itself)
@michael-hawker michael-hawker force-pushed the user/mhawker/gridsplitter-tests branch from 7591f61 to caf0d7f Compare July 9, 2021 09:16
@michael-hawker
Copy link
Member Author

Thanks @RosarioPulella, late night oversight.

Looks like we found our culprit:

Error: [Host] [Error] Background Task Instance Canceled. Reason: ExecutionTimeExceeded

I'll try adding this capability next to see if that resolves the issue. Otherwise we'll have to figure something else out... FYI @azchohfi, I don't think your WinUI 3 solution will have this same problem. Too bad gRPC doesn't really work with UWP, otherwise we could just share it here too, eh?

@michael-hawker
Copy link
Member Author

@RosarioPulella thinking you're probably done for the day already, but let me know if you have a chance to check locally again. Seems to have run this last time in the CI. 🤞 I'll try and kick this off again later to see if it runs a second time.

Copy link
Contributor

@Rosuavio Rosuavio left a comment

Choose a reason for hiding this comment

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

Looks good. It passed many repeated tests!

@michael-hawker
Copy link
Member Author

Thanks @azchohfi for the review. Addressed your comments, think we should be good now!

@Rosuavio Rosuavio closed this Jul 19, 2021
@Rosuavio Rosuavio reopened this Jul 19, 2021
@michael-hawker michael-hawker merged commit f62a60b into CommunityToolkit:main Jul 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants