Quarantine ProcessBufferedRenderBatches_WritesRenders #63643
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR quarantines a flaky test (ProcessBufferedRenderBatches_WritesRenders) that occasionally fails due to rendering tasks not completing as expected, and adds temporary diagnostic logging to help debug the underlying issue.
- Quarantines the failing test with appropriate tracking issue reference
- Adds temporary logging to capture task state and exception details when the assertion fails
| var message = $"Render task should succeed synchronously.\nStatus: '{status}'\nInner exception: '{innerException}'"; | ||
| Assert.True(task.IsCompletedSuccessfully, message); | ||
| } | ||
|
|
||
| Assert.True(task.IsCompletedSuccessfully); |
There was a problem hiding this comment.
The assertion on line 783 is redundant since line 780 already performs the same check. Remove the duplicate assertion to avoid confusion and reduce code duplication.
| Assert.True(task.IsCompletedSuccessfully); |
Copilot uses AI. Check for mistakes.
| @@ -767,6 +769,17 @@ public Task SetParametersAsync(ParameterView parameters) | |||
| public void TriggerRender() | |||
| { | |||
| var task = _renderHandle.Dispatcher.InvokeAsync(() => _renderHandle.Render(_renderFragment)); | |||
|
|
|||
| // TODO: Remove the following block once ProcessBufferedRenderBatches_WritesRenders is fixed. | |||
There was a problem hiding this comment.
Why? Can't we keep this message for all the tests in case of failure?
There was a problem hiding this comment.
I guess we could. I didn't want to add anything into the "standard code path". But since its just test code, why not.
| var task = _renderHandle.Dispatcher.InvokeAsync(() => _renderHandle.Render(_renderFragment)); | ||
|
|
||
| // TODO: Remove the following block once ProcessBufferedRenderBatches_WritesRenders is fixed. | ||
| if (!task.IsCompletedSuccessfully) | ||
| { | ||
| // Log the task state for debugging purposes. | ||
| var status = task.Status; | ||
| var innerException = task.Exception?.InnerException; | ||
| var message = $"Render task should succeed synchronously.\nStatus: '{status}'\nInner exception: '{innerException}'"; | ||
| Assert.True(task.IsCompletedSuccessfully, message); | ||
| } | ||
|
|
There was a problem hiding this comment.
This is not really the "fix" for this. We need to find where it is going async.
There was a problem hiding this comment.
Oh, it is definitely not a fix. It is meant to help diagnosing the problem (as it is difficult to reproduce on demand).
|
/backport to release/10.0 |
|
Started backporting to release/10.0: https://github.com/dotnet/aspnetcore/actions/runs/17671816698 |
The
RemoteRendererTest.ProcessBufferedRenderBatches_WritesRenderstest occasionaly fails because one of the rendering tasks does not complete as expected. That means that the task either failed with an unhandled exception, or has not completed synchronously.The PR quarantines the test and adds temporary logging of the task state to help us eventually get more insight into the underlying issue.