-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[Windows] Review currently failing Windows tests #15530
Comments
This comment was marked as off-topic.
This comment was marked as off-topic.
@PureWeen are these running on CI now? It probably not very impactful to fix these tests unless we are running them? |
@jonathanpeppers we're getting CI spun up here soon so I'm just trying to gauge what to do about these failures for tracking purposes
Or some combination of those |
Let me know when Windows device tests are running on CI, as I probably won't fix the ones above until then. They are probably not real bugs, but something with the tests themselves. |
@jonathanpeppers sounds good! |
Fixes part of: dotnet#15530 These would fail randomly for me when I ran them. But after reviewing memory snapshots, I didn't find an actual issue the tests were just flaky. I found I could rework the tests to do: await AssertionExtensions.Wait(() => { GC.Collect(); GC.WaitForPendingFinalizers(); return !pageReference.IsAlive; }, timeout: 5000); And now they pass every time for me locally. So it waits up to 5 seconds, and quits early if the page is gone. If this still passes on all platforms -- we should just use this instead.
Fixes part of: #15530 These would fail randomly for me when I ran them. But after reviewing memory snapshots, I didn't find an actual issue the tests were just flaky. I found I could rework the tests to do: await AssertionExtensions.Wait(() => { GC.Collect(); GC.WaitForPendingFinalizers(); return !pageReference.IsAlive; }, timeout: 5000); And now they pass every time for me locally. So it waits up to 5 seconds, and quits early if the page is gone. If this still passes on all platforms -- we should just use this instead.
@jonathanpeppers not sure you're still interested and just caught this now... But I think all Windows tests are now running on CI. So if you have still something to do here, do it now! ✨ |
We've added this issue to our backlog, and we will work to address it as time and resources allow. If you have any additional information or questions about this issue, please leave a comment. For additional info about issue management, please read our Triage Process. |
Description
As part of #15527 I've skipped the tests listed below. All of these tests were failing on the original PR. We don't necessarily need to fix these issues for .NET8 but we should establish if we should fix these for .NET8 or just fix the tests themselves.
In addition after merging #15629 we're now running the DeviceTests for Windows, these have been disabled because they were failing and we should look into them.
Core
At the time of writing the whole Core project test runs have been disabled because it would crash while running, so that is something we need to fix, other than that, these tests are failing:
BlazorWebView
#16187
Essentials
Controls
Just like Core, the Controls projects also crashes somewhere along the test run and never completes. We need to investigate why and enable it for running on CI again
Unpackaged
Then we need to run this full suite also for unpackaged apps. I think we can make this a variation by leveraging the
device
parameter that is in place for iOS and Android and add that to the matrix for WindowsRandom
RunHeadless
propertyThe text was updated successfully, but these errors were encountered: