Log more to test logger in HTTP/2 Kestrel tests by BrennanConroy · Pull Request #27238 · dotnet/aspnetcore (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation6 Commits3 Checks0 Files changed

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

[ Show hidden characters]({{ revealButtonHref }})

BrennanConroy

#26291 is about up for unquarantine, I looked into the test and couldn't repro and there are no test failure logs to look at.

I added a new log and fixed a couple logs that weren't logging to the test logger. We can probably close the issue and unquarantine the test while waiting for a new failure.

My best guess is that the stream draining code is taking too long sometimes on the CI when there are 200 streams

while (_serverActiveStreamCount > 0)
{
await _streamCompletionAwaitable;
UpdateCompletedStreams();
}

@BrennanConroy

halter73

@@ -186,7 +186,7 @@ public Http2TestBase()
// Emulate transport abort so the _connectionTask completes.
Task.Run(() =>
{
TestApplicationErrorLogger.LogInformation(0, ex, "ConnectionContext.Abort() was called. Completing _pair.Application.Output.");

Choose a reason for hiding this comment

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

Can we fix it so that everything that logs directly to the TestApplicationErrorLogger in a TestApplicationErrorLoggerLoggedTest-derived test class logs to Logger instead?

Choose a reason for hiding this comment

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

Not easily. Calling Logger.Log will call TestApplicationErrorLogger.Log which would then call Logger.Log which would...

Ideally, TestApplicationErrorLogger would not be accessible to TestApplicationErrorLoggerLoggedTest tests and tests would access the messages through a field. I'll push a commit to try that out.

@BrennanConroy

jkotalik

@BrennanConroy

ServerComparison.FunctionalTests.ResponseCompressionTests.ResponseCompression_AppCompression(variant: Server: IISExpress, TFM: net6.0, Type: Portable, Arch: x64, Host: InProcess) failed, any logs I should gather before creating an issue? @jkotalik

halter73

halter73

halter73

halter73

@BrennanConroy

Labels

area-networking

Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions