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.

Sign up for GitHub

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

Jump to bottom

Skip WebSocketOutOfProcessTests on Windows.Amd64.Server2022.Open #59112

Merged
merged 3 commits into from
Nov 23, 2024
Merged

Skip WebSocketOutOfProcessTests on Windows.Amd64.Server2022.Open #59112

merged 3 commits into from
Nov 23, 2024

Conversation

captainsafia
Copy link
Member

Addresses #59111.

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions label Nov 22, 2024
@BrennanConroy
Copy link
Member

Huh, maybe skip attributes on abstract classes aren't discovered

aspnetcore/src/Servers/IIS/IIS/test/Common.FunctionalTests/WebSocketTests.cs

Lines 32 to 37 in 3ba179a

#if IISEXPRESS_FUNCTIONALS
[SkipOnHelix("Unsupported queue", Queues = "Windows.Amd64.VS2022.Pre.Open")]
#else
// These queues do not have websockets enabled currently for full IIS
[SkipOnHelix("Unsupported queue", Queues = "Windows.Amd64.VS2022.Pre.Open;Windows.Amd64.Server2022.Open")]
#endif

This is what we should do in both WebSocketsOutOfProcess and WebSocketsInProcess then.
{4CDAF7EB-1C97-44FC-B5E6-D6E9FDC56A0B}

@BrennanConroy
Copy link
Member

WebSocketsInProcess too

@captainsafia
Copy link
Member Author

WebSocketsInProcess too

I didn't spot any failures in the in-process tests. Do we want to update the annotation as a proactive measure since they use the same base class?

@BrennanConroy
Copy link
Member

The screenshot I shared in #59112 (comment) shows in process failing as well. It was from the latest Helix run.

@captainsafia captainsafia enabled auto-merge (squash) November 22, 2024 21:39
@captainsafia captainsafia merged commit 1770dcf into main Nov 23, 2024
27 checks passed
@captainsafia captainsafia deleted the qt-websockets-test branch November 23, 2024 03:50
@dotnet-policy-service dotnet-policy-service bot added this to the 10.0-preview1 milestone Nov 23, 2024
wtgodbe pushed a commit that referenced this pull request Nov 27, 2024
@captainsafia @wtgodbe
Skip WebSocketOutOfProcessTests on Windows.Amd64.Server2022.Open (#59112
d086eec 
)

* Skip WebSocketOutOfProcessTests on Windows.Amd64.Server2022.Open

* Use if-def from base class

* Fix in-proc and remove attributes from abstract class
wtgodbe pushed a commit that referenced this pull request Nov 27, 2024
@github-actions @BrennanConroy @captainsafia
[release/9.0] Fix IIS outofprocess to remove WebSocket compression ha…
401ae7c 
…ndshake (#58931)

* Fix IIS outofprocess to remove WebSocket compression handshake

* test name

* Don't run IIS websocket tests on unsupported queue

* comment

* using

* using

* not

* Skip WebSocketOutOfProcessTests on Windows.Amd64.Server2022.Open (#59112)

* Skip WebSocketOutOfProcessTests on Windows.Amd64.Server2022.Open

* Use if-def from base class

* Fix in-proc and remove attributes from abstract class

---------

Co-authored-by: Brennan <brecon@microsoft.com>
Co-authored-by: Safia Abdalla <safia@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BrennanConroy BrennanConroy BrennanConroy approved these changes

@halter73 halter73 Awaiting requested review from halter73 halter73 is a code owner

@JamesNK JamesNK Awaiting requested review from JamesNK JamesNK is a code owner

@mgravell mgravell Awaiting requested review from mgravell mgravell is a code owner

Assignees
No one assigned
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Milestone
10.0-preview1
Development

Successfully merging this pull request may close these issues.
2 participants
@captainsafia @BrennanConroy