Skip to content

[wasm][testing] hosting echo server in xharness process#52923

Merged
pavelsavara merged 6 commits intodotnet:mainfrom
pavelsavara:pr_wasm_network_test_server
Jun 9, 2021
Merged

[wasm][testing] hosting echo server in xharness process#52923
pavelsavara merged 6 commits intodotnet:mainfrom
pavelsavara:pr_wasm_network_test_server

Conversation

@pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented May 18, 2021 •
edited
Loading

This makes WebSocket test on WASM easier to work with, by moving the tests to inner loop. In order to do that we are hosting the echo test server which the tests use, in the xharness process. Because wasm is always tested with xharness.

@ghost ghost added the area-System.Net label May 18, 2021
@ghost
Copy link

ghost commented May 18, 2021

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

Issue Details

Companion change to dotnet/xharness#593

  • build echo test server and pass it to xharness
  • WebSocket need to run in browser/chromeDriver, not in V8 console
  • move WebSocket tests to inner loop for browser
  • doc fix
Author: pavelsavara
Assignees: -
Labels:

area-System.Net
Milestone: -

This was referenced May 18, 2021
Closed
Closed
Merged
@pavelsavara pavelsavara force-pushed the pr_wasm_network_test_server branch from 1468635 to 5eb357d Compare June 3, 2021 12:57
@pavelsavara pavelsavara requested a review from radical June 3, 2021 14:33
@pavelsavara
Copy link
Member Author

pavelsavara commented Jun 3, 2021

fail: Failed to find the middleware assembly at /__w/1/s/artifacts/bin/NetCoreServer/net6.0-Release/NetCoreServer.dll

@pavelsavara pavelsavara marked this pull request as ready for review June 4, 2021 13:53
@pavelsavara pavelsavara requested a review from mdh1418 June 4, 2021 13:53
@pavelsavara pavelsavara added the arch-wasm WebAssembly architecture label Jun 4, 2021
@ghost
Copy link

ghost commented Jun 4, 2021

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

This makes WebSocket test on WASM easier to work with, by moving the tests to inner loop. In order to do that we are hosting the echo test server which the tests use, in the xharness process. Because wasm is always tested with xharness.

Author: pavelsavara
Assignees: -
Labels:

arch-wasm, area-System.Net
Milestone: -

mdh1418
mdh1418 reviewed Jun 4, 2021
View reviewed changes
mdh1418
mdh1418 reviewed Jun 4, 2021
View reviewed changes
mdh1418
mdh1418 reviewed Jun 4, 2021
View reviewed changes
@pavelsavara pavelsavara requested a review from mdh1418 June 4, 2021 15:52
mdh1418
mdh1418 reviewed Jun 4, 2021
View reviewed changes
mdh1418
mdh1418 approved these changes Jun 4, 2021
View reviewed changes
Copy link
Member

@mdh1418 mdh1418 left a comment

Choose a reason for hiding this comment

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

Overall looks good to me, thanks!

The remaining nits I have are the naming in Configuration.Http.cs and using NetCoreAppCurrent instead in src/libraries/Common/tests/System/Net/Prerequisites/NetCoreServer/NetCoreServer.csproj

@pavelsavara pavelsavara requested a review from wfurt June 4, 2021 16:13
@radical radical requested a review from lewing June 4, 2021 16:18
radical
radical reviewed Jun 4, 2021
View reviewed changes
@pavelsavara
Copy link
Member Author

pavelsavara commented Jun 7, 2021

#52923 (comment)
[SkipOnPlatform(TestPlatforms.Browser, "Host is ignored on Browser")] do you believe we could fix this @radical ? Then ActiveIssue would be more appropriate.

@ManickaP ManickaP self-requested a review June 7, 2021 12:06
@radical
Copy link
Member

radical commented Jun 7, 2021

#52923 (comment)
[SkipOnPlatform(TestPlatforms.Browser, "Host is ignored on Browser")] do you believe we could fix this @radical ? Then ActiveIssue would be more appropriate.

As @mdh1418 asked, What does this mean? Is m.Headers.Host problematic?, could you just explain the reasoning? What fails, and how does it fail?

@pavelsavara
- improve doc
63e863b 
- move tests to inner loop
- more granular ActiveIssue dotnet#53592 for lack of TRACE
- more granular ActiveIssue dotnet#53591 for content on GET/HEAD
- more granular ActiveIssue dotnet#53874 for HttpRequestMessage.Headers.Host
- more granular ActiveIssue dotnet#53872 for NPE on System.Net.Http.BrowserHttpHandler
- fix HTTP vs HTTPS test configuration `Http2SecureRemoteEchoServer`
- include echo middleware in xharness server
- include middleware in Helix correlation payload
@pavelsavara
document dotnet#53876
3fdc1d9
@pavelsavara pavelsavara force-pushed the pr_wasm_network_test_server branch from c2746b9 to 3fdc1d9 Compare June 8, 2021 15:31
@pavelsavara
Copy link
Member Author

pavelsavara commented Jun 8, 2021

As @mdh1418 asked, What does this mean? Is m.Headers.Host problematic?, could you just explain the reasoning? What fails, and how does it fail?

Created active issue instead #53874

radical
radical reviewed Jun 8, 2021
View reviewed changes
ManickaP
ManickaP reviewed Jun 8, 2021
View reviewed changes
Copy link
Member

@ManickaP ManickaP left a comment

Choose a reason for hiding this comment

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

Looks good, just a question about the split of secure server for H/2. I'll approve once it's cleared.

ManickaP
ManickaP approved these changes Jun 9, 2021
View reviewed changes
Copy link
Member

@ManickaP ManickaP left a comment

Choose a reason for hiding this comment

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

Thanks!

@pavelsavara pavelsavara changed the title [wasm][testing] hosting webSocket echo server in xharness process [wasm][testing] hosting echo server in xharness process Jun 9, 2021
radical
radical approved these changes Jun 9, 2021
View reviewed changes
Copy link
Member

@radical radical left a comment

Choose a reason for hiding this comment

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

Thanks for your patience, LGTM! 👍
You might want to run the outerloop tests to make sure that this didn't break anything.

@pavelsavara
Copy link
Member Author

pavelsavara commented Jun 9, 2021

/azp run runtime-libraries-mono outerloop

@azure-pipelines
Copy link

azure-pipelines bot commented Jun 9, 2021

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara pavelsavara merged commit fb61be4 into dotnet:main Jun 9, 2021
@karelz karelz mentioned this pull request Jun 17, 2021
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Jul 9, 2021
@karelz karelz added this to the 6.0.0 milestone Jul 15, 2021
@pavelsavara pavelsavara deleted the pr_wasm_network_test_server branch July 29, 2021 08:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@radical radical radical approved these changes

@ManickaP ManickaP ManickaP approved these changes

@mdh1418 mdh1418 mdh1418 approved these changes

@wfurt wfurt Awaiting requested review from wfurt

@lewing lewing Awaiting requested review from lewing

Assignees

No one assigned

Labels

arch-wasm WebAssembly architecture area-System.Net

Projects

None yet

Milestone

6.0.0

Development

Successfully merging this pull request may close these issues.

[wasm] HttpResponseMessage.Headers is empty [wasm][HTTP] TypeError: Failed to fetch

5 participants

@pavelsavara @radical @ManickaP @mdh1418 @karelz