Skip to content

Conversation

@liveans
Copy link
Member

@liveans liveans commented Jul 12, 2024

Disables BidirectionStreamingTests for WS2022

main PR #104722 which fixes the behavior, but in release branches we're disabling tests instead.

Description

Recently, WS2022 got an update which backports AUTOMATIC_CHUNKING in WinHttp, and it affects our test in CI. Because, we were relying on it, now we're disabling tests because #104891 started to pop up with the backport in WS2022.

Customer Impact

No Customer Impact, test-only change.

Regression

No

Testing

CI

Risk

Test-only change, no risk.

Package authoring signed off?

IMPORTANT: If this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

@liveans liveans requested a review from a team July 12, 2024 21:57
@ghost ghost added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jul 12, 2024
@liveans
Copy link
Member Author

liveans commented Jul 12, 2024

/backport to release/6.0-staging

@github-actions
Copy link
Contributor

Started backporting to release/6.0-staging: https://github.com/dotnet/runtime/actions/runs/9914603625

@liveans liveans added area-System.Net.Http and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Jul 12, 2024
@dotnet-policy-service
Copy link
Contributor

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

@github-actions
Copy link
Contributor

@liveans backporting to release/6.0-staging failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: Fix WinHttp StreamingTest backward compat version
Using index info to reconstruct a base tree...
M	src/libraries/Common/tests/TestUtilities/System/PlatformDetection.Windows.cs
M	src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/BidirectionStreamingTest.cs
Falling back to patching base and 3-way merge...
Auto-merging src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/BidirectionStreamingTest.cs
Auto-merging src/libraries/Common/tests/TestUtilities/System/PlatformDetection.Windows.cs
CONFLICT (content): Merge conflict in src/libraries/Common/tests/TestUtilities/System/PlatformDetection.Windows.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 Fix WinHttp StreamingTest backward compat version
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

@github-actions
Copy link
Contributor

@liveans an error occurred while backporting to release/6.0-staging, please check the run log for details!

Error: git am failed, most likely due to a merge conflict.

@liveans liveans added this to the 8.0.x milestone Jul 12, 2024
@liveans
Copy link
Member Author

liveans commented Jul 13, 2024

/azp run runtime-libraries-coreclr outerloop-windows

@liveans liveans added the Servicing-approved Approved for servicing release label Jul 13, 2024
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liveans
Copy link
Member Author

liveans commented Jul 13, 2024

This change is failing on windows.amd64.server2022.open.svc images but passing on windows.amd64.server2022.open.rt for both release/8.0-staging and release/6.0-staging, it's highly likely that open.svc is using older version on server2022.

rzikm
rzikm previously approved these changes Jul 13, 2024
Copy link
Member

@rzikm rzikm left a comment

Choose a reason for hiding this comment

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

LGTM, but please use the full servicing template for consistency.

@liveans
Copy link
Member Author

liveans commented Jul 13, 2024

LGTM, but please use the full servicing template for consistency.

I will keep this on hold, until we figure out what's going on with failures in release branches.

@rzikm rzikm dismissed their stale review July 13, 2024 07:29

Failing tests, will need more work

@ManickaP ManickaP added Servicing-consider Issue for next servicing release review and removed Servicing-approved Approved for servicing release labels Jul 15, 2024
@ManickaP ManickaP marked this pull request as draft July 15, 2024 07:47
@liveans liveans force-pushed the fix_winhttp_streamingtest_backward_compat_version branch from 5ad8650 to 3ec784e Compare July 16, 2024 07:59
@liveans liveans marked this pull request as ready for review July 16, 2024 07:59
Copy link
Member

@rzikm rzikm left a comment

Choose a reason for hiding this comment

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

LGTM if Ci is green

…ests/BidirectionStreamingTest.cs

Co-authored-by: Radek Zikmund <[email protected]>
@liveans liveans added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Jul 16, 2024
@liveans
Copy link
Member Author

liveans commented Jul 16, 2024

Test only change, CI seems happy (windows passes), tell mode, adding servicing-approved

@liveans liveans merged commit 57a18ff into dotnet:release/8.0-staging Jul 16, 2024
@liveans liveans deleted the fix_winhttp_streamingtest_backward_compat_version branch July 16, 2024 11:07
@github-actions github-actions bot locked and limited conversation to collaborators Aug 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Net.Http Servicing-approved Approved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants