Test webview2 by pinzart90 · Pull Request #14670 · DynamoDS/Dynamo

pinzart90 · 2023-12-01T16:36:08Z

Using the default SyncronizationContext during tests will cause async operations to be continued on the ThreadPool
ex.

 async DoStuff() {
   await http.Get("www.google.com");
   someUIControl.Visible = Visibility.Hidden;
}
If you call DoStuff() from Thread1, the code with "someUIControl.Visible = Visibility.Hidden;" will run on a different thread (from the ThreadPool).

To fix this scenario, I moved all our UI tests on the WPF sync context (DispatcherSyncronizationContext). This ensures that all async/await methods resume execution on the same thread.
A note worth mentioning is that Task.Run or Task.Factory.StartNew will, by default, run the task on a ThreadPool thread. We should make sure that we are delegating all UI calls to the UI dispatcher or ensuring the Task.Run calls are executed directly on the UI thread (via argument TaskScheduler.FromCurrentSynchronizationContext())

Some tests will create webview2 controls.

Some of the webview 2 controls are visible and initializeAsync is called, but they finish so fast (the thread dos not process the message in time) that Dispose is called before they get a chace to finish initialization (System.ObjectDisposedException might be thrown)
Some of the webview 2 controls are created hidden and initializeAsync will be blocked until the control becomes visible. The tests will finish and Dispose will be called. (ComExceptions or ObjectDisposedException).

Most of these crashes might happen outside of the tests that caused them. Depends on what test actually calls DoEvents (i.e tells the thread to process its message loop)

To fix these I moved most of the webview2 initialize calls on the Loaded event in this PR. Also I ensured that all tests that create visible webview2 controls are processed and finished before the test continues and eventually disposes of the control (using DispatcherUtil.DoEventsLoop)

…/Dynamo into test_webview2

pinzart90 · 2023-12-04T19:26:35Z

test/DynamoCoreWpfTests/PackageManager/PackageManagerUITests.cs

+                // We need to run the async ExecutePackageDownload function in a separate thread so that the Thread.Sleep does not mess up the other awaiting tasks.
+                //actually perform the download & install operations
+                pmVmMock.Object.ExecutePackageDownload(id, pkgVer, "");
+            }).Wait();


Task.Run is a viable option here because there is no UI interaction from ExecutePackageDownload

This could be resolved by running pmVmMock.Object.ExecutePackageDownload(id, pkgVer, ""); on the test thread but adding a DispatcherUtil.DoEvents() after the thread.Sleep bellow (would have been needed if there was UI interaction in ExecutePackageDownload)

could you also make this test async, await the download and get rid of the sleep?

It is not that easy to await the download. The ExecutePackageDownload would have to be refactored to return a task fo the download and install operations.

…/Dynamo into test_webview2

pinzart90 · 2023-12-04T21:21:25Z

master-15 tests passed here https://master-15.jenkins.autodesk.com/view/DYN/job/DYN-DevCI_Self_Service/1319/

pinzart90 · 2023-12-05T15:18:11Z

test/DynamoCoreTests/UnitTestBase.cs

            {
                AppDomain.CurrentDomain.AssemblyResolve -= assemblyHelper.ResolveAssembly;
            }
+            System.Console.WriteLine("Finished test: " + TestContext.CurrentContext.Test.Name);


These do add more logs to the jobs output but they are useful

mjkkirschner · 2023-12-05T15:27:14Z

test/DynamoCoreWpfTests/Utility/DispatcherUtil.cs

-        /// <returns></returns>
-        private static object ExitFrame(object frame)
+        /// <param name="check">When check returns true, the even loop is stopped.</param>
+        [SecurityPermission(SecurityAction.Demand, Flags = SecurityPermissionFlag.UnmanagedCode)]


I don't know. I copied it from the DoEvents method

I do know that the Dispatcher does call into unmanaged code when it processes its message queue

I feel you can probably get rid of it all, I think security permissions system is removed.

mjkkirschner · 2023-12-05T15:29:16Z

test/DynamoCoreWpfTests/ViewExtensions/DocumentationBrowserViewExtensionTests.cs

        }

-        [Test,Category("Failure")]
+        [Test]


mjkkirschner · 2023-12-05T15:31:04Z

test/DynamoCoreWpfTests/ViewExtensions/DocumentationBrowserViewExtensionTests.cs

+            }
+
+            // Wait for the DocumentationBrowserView webview2 control to finish initialization
+            DispatcherUtil.DoEventsLoop(() =>


so essentially all devs working on tests with webview2 controls need to use this API for tests?

Hopefully not all the time. Only if the webview2 control is expected to be visible during the test

I will try to add some sort of docs somewhere

It would be a really nice addition with a custom analyzer using roslyn. We could detect usage of an API that would show the WebView control in a method marked with the test attribute, and add a compiler error if we don't see this method used... anyway, for another day.

* upate * Update ViewExtension.cs * Update DocumentationBrowserView.xaml.cs * Update NotificationsViewExtension.cs * update * update * Update NotificationCenterController.cs * Test webview2 (#14670) * update * update * Update DocumentationBrowserView.xaml.cs * update * Update DocumentationBrowserView.xaml.cs * update * update --------- Co-authored-by: pinzart <[email protected]> * Update NotificationCenterController.cs * Update DispatcherUtil.cs * Update CS_SDK.props * Revert "Update CS_SDK.props" This reverts commit cfaceb2. --------- Co-authored-by: pinzart <[email protected]>

pinzart90 added 12 commits December 1, 2023 09:58

Merge branch 'master' into test_webview2

d7aa848

Merge branch 'master' into test_webview2

b0e00ad

update

2c46476

Merge branch 'master' into test_webview2

0576e03

Merge branch 'webview2_initialization' of https://github.com/DynamoDS…

e9e50f0

…/Dynamo into test_webview2

update

8c2994a

Update DocumentationBrowserView.xaml.cs

d7fbf84

Merge branch 'webview2_initialization' of https://github.com/DynamoDS…

0a5e088

…/Dynamo into test_webview2

update

ca82198

Update DocumentationBrowserView.xaml.cs

7c0c338

update from base

8587754

update

e31fcc9

pinzart90 marked this pull request as ready for review December 4, 2023 17:40

pinzart90 added 2 commits December 4, 2023 13:56

Merge branch 'webview2_initialization' of https://github.com/DynamoDS…

fc73055

…/Dynamo into test_webview2

update

2a7a066

pinzart90 commented Dec 4, 2023

View reviewed changes

pinzart90 requested review from aparajit-pratap, dnenov, mjkkirschner and sm6srw December 4, 2023 19:29

Merge branch 'webview2_initialization' of https://github.com/DynamoDS…

0e951e0

…/Dynamo into test_webview2

pinzart90 commented Dec 5, 2023

View reviewed changes

mjkkirschner reviewed Dec 5, 2023

View reviewed changes

test/DynamoCoreWpfTests/ViewExtensions/DocumentationBrowserViewExtensionTests.cs

}

[Test,Category("Failure")]

[Test]

Copy link

Member

mjkkirschner Dec 5, 2023

Choose a reason for hiding this comment

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

woohoo!

mjkkirschner reviewed Dec 5, 2023

View reviewed changes

mjkkirschner approved these changes Dec 5, 2023

View reviewed changes

pinzart90 merged commit ff8d3af into webview2_initialization Dec 5, 2023

pinzart90 deleted the test_webview2 branch December 5, 2023 15:34

Conversation

pinzart90 commented Dec 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pinzart90 Dec 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pinzart90 commented Dec 4, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mjkkirschner Dec 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mjkkirschner Dec 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pinzart90 commented Dec 1, 2023 •

edited

Loading

pinzart90 Dec 4, 2023 •

edited

Loading

mjkkirschner Dec 5, 2023 •

edited

Loading

mjkkirschner Dec 5, 2023 •

edited

Loading