Skip to content
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

Fix HttpHandlerDiagnosticListenerTests.TestW3CHeadersTraceStateAndCorrelationContext #112753

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

antonfirsov
Copy link
Member

@antonfirsov antonfirsov commented Feb 20, 2025

It's unclear to me how did this test pass before, but given it's testing W3C tracestate format behavior on .NET Framework, ActivityIdFormat.W3C should be explicitly set as DefaultIdFormat in the test. This will ensure that the Request-Id header (that is specific to the Hierarchical format) will be absent, making the last Assert pass.

Fixes #112426

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

@antonfirsov
Copy link
Member Author

/azp run runtime-libraries-coreclr outerloop

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@tarekgh
Copy link
Member

tarekgh commented Feb 20, 2025

@antonfirsov the issue #112426 is opened against .NET 8.0. This PR will force closing the issue which will make the failure still show in .NET 8 builds. Will you port this fix to 8.0 and possibly 9.0?

CC @noahfalk

@tarekgh tarekgh added this to the 10.0.0 milestone Feb 20, 2025
@antonfirsov
Copy link
Member Author

Will you port this fix to 8.0 and possibly 9.0?

Sure, thanks for the heads-up!

@@ -145,6 +145,7 @@ public async Task TestBasicReceiveAndResponseEvents()
}
}

[ActiveIssue("https://github.com/dotnet/runtime/issues/112792")]
Copy link
Member Author

Choose a reason for hiding this comment

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

I almost missed that this test is also failing. It seems to be a product bug, disabling for now. cc @tarekgh in case you have concerns.

@antonfirsov
Copy link
Member Author

/azp run runtime-libraries-coreclr outerloop

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants