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

Disable test #112780

Closed
wants to merge 2 commits into from
Closed

Disable test #112780

wants to merge 2 commits into from

Conversation

ManickaP
Copy link
Member

Disabling test failing on assert in #112700.
This is already fixed in .NET 9.0+ by #93984.

cc @wfurt @karelz

Customer Impact

  • Customer reported
  • Found internally

Regression

  • Yes
  • No

Testing

CI

Risk

Very low - test only change.

@Copilot Copilot bot review requested due to automatic review settings February 21, 2025 10:37

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.

Comments suppressed due to low confidence (1)

src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/BidirectionStreamingTest.cs:99

  • [nitpick] Consider adding a comment specifying when to re-enable this test (e.g. for .NET 9.0+), so that test coverage is restored once the underlying issue is fixed.
[ActiveIssue("https://github.com/dotnet/runtime/issues/112700")]
Copy link
Contributor

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

@ManickaP ManickaP added the Servicing-approved Approved for servicing release label Feb 21, 2025
@ManickaP
Copy link
Member Author

Test only change, tell mode, adding servicing approved.

Copy link
Member

@wfurt wfurt left a comment

Choose a reason for hiding this comment

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

LGTM.

@ManickaP
Copy link
Member Author

/azp list

Copy link

CI/CD Pipelines for this repository:

@ManickaP
Copy link
Member Author

/azp run runtime

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liveans
Copy link
Member

liveans commented Feb 21, 2025

Disabling this test will have no effect about the issue, it will keep popping from some other test, because issue is on the hot path.
I'd prefer to backport the fix honestly, fix itself looks simple and safe to backport.

@rzikm
Copy link
Member

rzikm commented Feb 24, 2025

I second Ahmets concern, are we sure this happens only on the test that is being disabled?

@ManickaP ManickaP closed this Feb 24, 2025
@ManickaP ManickaP added NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) and removed Servicing-approved Approved for servicing release labels Feb 24, 2025
@ManickaP ManickaP reopened this Feb 24, 2025
@ManickaP
Copy link
Member Author

/azp run runtime

1 similar comment
@ManickaP
Copy link
Member Author

/azp run runtime

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ManickaP ManickaP closed this Mar 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Net.Http NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants