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

[wasm][testing] hosting webSocket echo server in xharness process #593

Merged
merged 15 commits into from
May 19, 2021

Conversation

pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented May 10, 2021

In case that we are running unit tests in xharness, we would like to be able to host webserver in the xharness process rather than to call external servers in azure.

That would allow us to move some unit tests from outer loop to inner loop. See dotnet/runtime#52923

Part of overall epic dotnet/runtime#42852

    <WasmXHarnessArgs>$(WasmXHarnessArgs) --set-web-server-http-env=DOTNET_TEST_WEBSOCKETHOST,DOTNET_TEST_HTTPHOST</WasmXHarnessArgs>
    <WasmXHarnessArgs>$(WasmXHarnessArgs) --set-web-server-https-env=DOTNET_TEST_SECUREWEBSOCKETHOST,DOTNET_TEST_SECUREHTTPHOST</WasmXHarnessArgs>
    <WasmXHarnessArgs>$(WasmXHarnessArgs) --web-server-middleware=$(ArtifactsDir)bin/MonoNetTestServer/$(NetCoreAppCurrent)-$(Configuration)/MonoNetTestServer.dll,GenericHandler</WasmXHarnessArgs>
    <WasmXHarnessArgs>$(WasmXHarnessArgs) --web-server-middleware=$(ArtifactsDir)bin/NetCoreServer/$(NetCoreAppCurrent)-$(Configuration)/NetCoreServer.dll,GenericHandler</WasmXHarnessArgs>


set XHARNESS_CLI_PATH=c:\Dev\xharness\artifacts\bin\Microsoft.DotNet.XHarness.CLI\Debug\net6.0\Microsoft.DotNet.XHarness.CLI.dll
set XHARNESS_COMMAND=test-browser
./dotnet build /t:Test src/libraries/System.Net.WebSockets.Client/tests /p:TargetOS=Browser /p:TargetArchitecture=wasm /p:Configuration=Release

@pavelsavara
Copy link
Member Author

@lewing

@pavelsavara
Copy link
Member Author

@akoeplinger could you please comment on topics in the description ? Thank you.

@pavelsavara
Copy link
Member Author

pavelsavara commented May 11, 2021

Right now, I'm thinking that the new echo endpoints should stay in runtime repository in form of assembly, which could be offered to xharness as a binary plugin.
That would be better than current PR state, because the test would be in same repo as tested code.
For example I would like to create unit test for dotnet/runtime#51041, which will need server side. Ideally on single PR.

It would still leave us with .NET 4.7 version of the CoreFxNetCloudService and another .net core version for xharness. Unless we convert whole CoreFxNetCloudService to .net core.

@lewing your thoughts ?

@radical
Copy link
Member

radical commented May 18, 2021

Could you please explain what this is doing, and why, in the PR description?

@pavelsavara
Copy link
Member Author

great feedback @radical, thanks!

@pavelsavara pavelsavara requested a review from mdh1418 May 19, 2021 08:16
@pavelsavara pavelsavara requested a review from radical May 19, 2021 08:30
@pavelsavara pavelsavara requested a review from radical May 19, 2021 14:11
@pavelsavara pavelsavara requested a review from radical May 19, 2021 15:24
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.

The environment var expansion stuff could maybe be done outside the wasm commands, but that can be done later when we can see how they get used for non-wasm cases.

LGTM! Thank you for the PR, and patience with the feedback 👍

@pavelsavara pavelsavara enabled auto-merge (squash) May 19, 2021 17:04
@pavelsavara pavelsavara merged commit 31d6c34 into dotnet:main May 19, 2021
Copy link
Member

@akoeplinger akoeplinger left a comment

Choose a reason for hiding this comment

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

Missed the ping on this one, looks good to me thank you!

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

Successfully merging this pull request may close these issues.

3 participants