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

[wasm] Fix WebSocket tests on NodeJS #64330

Merged
merged 14 commits into from
Feb 9, 2022
Merged

[wasm] Fix WebSocket tests on NodeJS #64330

merged 14 commits into from
Feb 9, 2022

Conversation

maraf
Copy link
Member

@maraf maraf commented Jan 26, 2022 •
edited
Loading

  • Fixes NodeJS packed as correlation payload doesn't preserve symbolic links #64473 - restore two symbolic links.
  • Add WasmXHarnessArgsCli as an alternative variable that can be used from cli, as WasmXHarnessMonoArgs is used in some csprojs.
  • Include TestEchoMiddleware and RemoteLoopMiddleware as correlation payload even for NodeJS.
  • Move SetScriptCommands to a targets so they are in correct order in the generated run script.

@maraf maraf added arch-wasm WebAssembly architecture area-Infrastructure-mono labels Jan 26, 2022
@maraf maraf self-assigned this Jan 26, 2022
@ghost
Copy link

ghost commented Jan 26, 2022

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

Issue Details
  • Move all SetScriptCommands for NPM restore to a target, so they are placed after scenario in the generated script.
  • Fix passing WasmXHarnessMonoArgs. The original where passed to XHarness, but not to the javascript.
Author: maraf
Assignees: maraf
Labels:

arch-wasm, area-Infrastructure-mono
Milestone: -

@maraf
Copy link
Member Author

maraf commented Jan 26, 2022

/azp run runtime-extra-platforms

@maraf maraf added this to the 7.0.0 milestone Jan 26, 2022
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@maraf
Copy link
Member Author

maraf commented Jan 27, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@maraf
Copy link
Member Author

maraf commented Jan 27, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member

Perhaps unrelated, but why we have WasmXHarnessArgs into _XHarnessArgs

and then both _XHarnessArgs and WasmXHarnessArgs into RunScriptCommand

Does that duplicate WasmXHarnessArgs ?

Also, could we improve it so that I could pass something like /p:WasmXHarnessArgs="--no-headless" from commandline without ruining --web-server-use-cors by forcing WasmXHarnessArgs to constant.

@maraf
Copy link
Member Author

maraf commented Jan 28, 2022

Perhaps unrelated, but why we have WasmXHarnessArgs into _XHarnessArgs

and then both _XHarnessArgs and WasmXHarnessArgs into RunScriptCommand

There are two flavors of those - MSBuild property vs environment variable. I have added a comment about it.

Also, could we improve it so that I could pass something like /p:WasmXHarnessArgs="--no-headless" from commandline without ruining --web-server-use-cors by forcing WasmXHarnessArgs to constant.

I have add a new MSBuild property WasmXHarnessArgsCli that behaves as WasmXHarnessArgs, but it's used in csprojs.

@maraf maraf mentioned this pull request Jan 28, 2022
Closed
@maraf maraf force-pushed the WasmFixWebSocketTestsOnNodeJs branch from 5766f80 to 1680cbe Compare February 3, 2022 13:42
@maraf
Copy link
Member Author

maraf commented Feb 3, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@maraf
Copy link
Member Author

maraf commented Feb 3, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Feb 3, 2022
Closed
Closed
@maraf
Copy link
Member Author

maraf commented Feb 4, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@maraf maraf mentioned this pull request Feb 4, 2022
Merged
@maraf
Copy link
Member Author

maraf commented Feb 4, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@maraf maraf force-pushed the WasmFixWebSocketTestsOnNodeJs branch from 6c75fce to 5fe7c0f Compare February 8, 2022 11:22
@maraf
Copy link
Member Author

maraf commented Feb 8, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@maraf
Copy link
Member Author

maraf commented Feb 8, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@maraf
Copy link
Member Author

maraf commented Feb 8, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@maraf maraf requested a review from radical February 8, 2022 16:23
@maraf maraf marked this pull request as ready for review February 8, 2022 16:23
@maraf
Copy link
Member Author

maraf commented Feb 8, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@maraf
Copy link
Member Author

maraf commented Feb 8, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@runfoapp runfoapp bot mentioned this pull request Feb 8, 2022
Closed
This was referenced Feb 8, 2022
Closed
Closed
Closed
Closed
Closed
@maraf maraf merged commit 1ac45f5 into dotnet:main Feb 9, 2022
@maraf maraf deleted the WasmFixWebSocketTestsOnNodeJs branch February 9, 2022 09:27
@ghost ghost locked as resolved and limited conversation to collaborators Mar 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@pavelsavara pavelsavara pavelsavara approved these changes

@radical radical Awaiting requested review from radical

Assignees

@maraf maraf

Labels
arch-wasm WebAssembly architecture area-Infrastructure-mono
Projects
None yet
Milestone
7.0.0
Development

Successfully merging this pull request may close these issues.

NodeJS packed as correlation payload doesn't preserve symbolic links
3 participants
@maraf @pavelsavara @radical