Fix WinUI TestContext.Current being null#7749
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses a WinUI execution scenario where the ambient TestContext.Current can be null during test execution by ensuring the ITestContext is available directly on TestMethodInfo when invoking the executor.
Changes:
- Assigns the runner’s
_testContextonto theTestMethodInfoinstance immediately before executing the test method. - Reworks
TestMethodInfo.TestContextfrom a static accessor into an instance property that can use an explicitly assigned context, with a fallback toTestContext.Current.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodRunner.cs | Sets testMethodInfo.TestContext before invoking the TestMethodAttribute executor. |
| src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodInfo.cs | Converts TestContext to an instance property backed by a stored context, falling back to TestContext.Current. |
Evangelink
left a comment
There was a problem hiding this comment.
Overall the fix looks correct and well-targeted. The approach of using an instance field with AsyncLocal fallback is clean and maintains backward compatibility for non-WinUI scenarios.
A few observations:
-
Missing tests: There are no unit tests covering the new behavior. A test in
TestMethodRunnerTeststhat verifiesTestContextis properly set on theTestMethodInfobefore executor invocation would help prevent regression. Specifically, a test showing the fallback works whenTestContext.Currentis null butfieldis set would directly validate the WinUI fix. -
PR description: The body is currently empty (template placeholder). It would help reviewers to add root cause context — specifically that
AsyncLocaldoesn't flow through WinUI'sDispatcherQueue.TryEnqueue, causingTestContext.Currentto be null on the dispatcher thread. -
Minor: The
fieldis never cleared after execution, but sinceTestMethodInfois created fresh per test (viaTypeCache.ResolveTestMethodInfo), there's no stale state risk. No action needed.
Looks good modulo tests and documentation. 🎉 Temperature: warm/supportive. Sentiment: approving with minor suggestions.
|
@Evangelink I'll let you complete this by copying the tests that Copilot originally wrote in the other PR (please ensure the tests will fail on current main). |
|
@copilot fix these issues: @azure-pipelines test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Execution/TestMethodInfoTests.cs(195,1): error : [TestMethodInfoInvokeShouldWaitForAsyncTestMethodsToComplete] [net462] This program location is thought to be unreachable. File='/_/src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodInfo.cs' Line=58 @azure-pipelines test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Execution/TestMethodInfoTests.cs(214,1): error : [TestMethodInfoInvokeAsyncShouldHandleThrowAssertInconclusive] [net462] This program location is thought to be unreachable. File='/_/src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodInfo.cs' Line=58 @azure-pipelines test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Execution/TestMethodInfoTests.cs(234,1): error : [TestMethodInfoInvokeAsyncShouldHandleAssertInconclusive] [net462] This program location is thought to be unreachable. File='/_/src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodInfo.cs' Line=58 @azure-pipelines test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Execution/TestMethodInfoTests.cs(252,1): error : [TestMethodInfoInvokeShouldHandleThrowAssertInconclusive] [net462] This program location is thought to be unreachable. File='/_/src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodInfo.cs' Line=58 @azure-pipelines test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Execution/TestMethodInfoTests.cs(272,1): error : [TestMethodInfoInvokeShouldHandleAssertInconclusive] [net462] This program location is thought to be unreachable. File='/_/src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodInfo.cs' Line=58 |
Agent-Logs-Url: https://github.com/microsoft/testfx/sessions/0103e8e1-f2d9-4a30-9e5d-93ea53c3775f Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
Fixed in ff33e06. I restored the Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Agent-Logs-Url: https://github.com/microsoft/testfx/sessions/c216287c-de48-4a49-8c08-d7e369d2edea Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
Agent-Logs-Url: https://github.com/microsoft/testfx/sessions/c216287c-de48-4a49-8c08-d7e369d2edea Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot fix azure-pipelines test\UnitTests\MSTestAdapter.PlatformServices.UnitTests\Execution\TestMethodRunnerTests.cs(466,53): error CS1061: (NETCORE_ENGINEERING_TELEMETRY=Build) 'Task' does not contain a definition for 'WaitAsync' and no accessible extension method 'WaitAsync' accepting a first argument of type 'Task' could be found (are you missing a using directive or an assembly reference?) |
Agent-Logs-Url: https://github.com/microsoft/testfx/sessions/5400fae9-d407-494c-8bf1-c7b27ec6d7d0 Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
Fixed in Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
Thanks for fixing this issue! Is there a plan to release a new stable version with this fix soon? It would be great if a release is planned for this week. Otherwise, I will stay on |
|
/backport to rel/4.2 |
1 similar comment
|
/backport to rel/4.2 |
|
Started backporting to rel/4.2: https://github.com/microsoft/testfx/actions/runs/25057861590 |
Yes sorry I am quite burried with work, I'm queueing that now |
…ckport to rel/4.2) (#7905) Co-authored-by: Youssef1313 <youssefvictor00@gmail.com> Co-authored-by: Amaury Levé <amauryleve@microsoft.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
No problem, that would be great if possible. I will keep an eye on the release this week. |
|
There was an issue with the release pipeline, I'm looking at it now |
No description provided.