[wasm][debugger] Fix hang in ArrayTests #63528
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Tagging subscribers to this area: @thaystg Issue Detailsnull
|
radical
force-pushed
the
debug-array-tests
branch
4 times, most recently
from
d4240ca to
00d96f3
Compare
radical
changed the title
There were some instances of async calls being blocking by the use of `.Wait()` on the task, in various `Check*` methods. And that would cause completions to get blocked, and not get run. In some cases, we didn't await the task at all, and so failures in that would only fail randomly. This removes all uses of `.Wait()`, and makes the methods async, and correctly awaits on them.
.. to make it clearer that it's safe to use `.Result` on it, without blocking.
Fails with:
```
Failed DebuggerTests.SteppingTests.CheckResetFrameNumberForEachStep [299 ms]
Error Message:
Assert.Equal() Failure
↓ (pos 46)
Expected: ···/debugger-test.cs#806#8
Actual: ···/debugger-test.cs#802#8
↑ (pos 46)
```
The test code depends on
`private Func<DateTime> todayFunc = () => DateTime.Now;`
.. and checking `f (today.DayOfWeek == DayOfWeek.Sunday)`,
causing it to fail on Sundays.
This didn't always fail because the async lambda with the checks wasn't
awaited.
```
[xUnit.net 00:00:03.78] DebuggerTests.ArrayTests.InvalidAccessors [FAIL]
Failed DebuggerTests.ArrayTests.InvalidAccessors [451 ms]
Error Message:
[] Could not find variable 'c'
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.GetAndAssertObjectWithName(JToken obj, String name, String label) in /Users/radical/dev/runtime/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 393
at DebuggerTests.ArrayTests.<InvalidAccessors>b__14_0(JToken locals) in /Users/radical/dev/runtime/src/mono/wasm/debugger/DebuggerTestSuite/ArrayTests.cs:line 632
at DebuggerTests.DebuggerTestBase.CheckInspectLocalsAtBreakpointSite(String type, String method, Int32 line_offset, String bp_function_name, String eval_expression, Func`2 locals_fn, Func`2 wait_for_event_fn, Boolean use_cfo, String assembly, Int32 col) in /Users/radical/dev/runtime/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 239
at DebuggerTests.ArrayTests.InvalidAccessors() in /Users/radical/dev/runtime/src/mono/wasm/debugger/DebuggerTestSuite/ArrayTests.cs:line 626
```
radical
force-pushed
the
debug-array-tests
branch
from
e4ed9e4 to
23837e5
Compare
Member
Author
|
Once the current run passes, I'll add back rest of the builds. |
This reverts commit 23837e5.
Member
Author
|
/azp run runtime-manual |
|
Azure Pipelines successfully started running 1 pipeline(s). |
thaystg
approved these changes
Jan 10, 2022
Member
Author
|
The browser AOT test didn't really fail though helix seems to be marking it as failed. https://helixre8s23ayyeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-63528-merge-332ab7a0928e4fec9d/normal-Microsoft.Extensions.Logging.Generators.Roslyn3.11.Tests.Attempt.2/1/console.aceb3530.log?sv=2019-07-07&se=2022-01-30T04%3A43%3A58Z&sr=c&sp=rl&sig=iISX%2FhYYAf1Nw7W%2FJBjhkps%2BqVIExAnJR%2BWevpVSV2M%3D The failing debugger test is a known failure. |
ghost
locked as resolved and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There were some instances of async calls being blocking by the use of
.Wait()on the task, in variousCheck*methods. And that would causecompletions to get blocked, and not get run.
In some cases, we didn't await the task at all, and so failures in that
would only fail randomly.
This removes all uses of
.Wait(), and makes the methods async, andcorrectly awaits on them.
Fixes #62661 .