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][libs] Fix WasmAppHost, and AppBundle for library tests #77719

Merged
merged 21 commits into from
Jan 31, 2023

Conversation

ilonatommy
Copy link
Member

@ilonatommy ilonatommy commented Nov 1, 2022

Partial fix for #76721.

This PR fixes running lib tests e.g.

dotnet run -r browser-wasm -c Debug --project src/libraries/System.Linq/tests/System.Linq.Tests.csproj --debug --host browser -p:DebuggerSupport=true 

results in:

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:07.29
WasmAppHost --runtime-config runtime-fork\artifacts\bin\System.Linq.Tests\Debug\net7.0\browser-wasm\AppBundle\WasmTestRunner.runtimeconfig.json --debug --host browser
Debug proxy for chrome now listening on http://127.0.0.1:65095/. And expecting chrome at http://localhost:9222/
Hosting environment: Production
Content root path: runtime-fork\artifacts\bin\System.Linq.Tests\Debug\net7.0\browser-wasm\AppBundle
Now listening on: http://127.0.0.1:65095
Debug proxy for firefox now listening on tcp://127.0.0.1:65096. And expecting firefox at port 6000 .

App url: http://127.0.0.1:9000/index.html
App url: https://127.0.0.1:50493/index.html

Scenarios from #75384 description got tested and work.

How it works:
image

@ilonatommy ilonatommy self-assigned this Nov 1, 2022
@ghost
Copy link

ghost commented Nov 1, 2022

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

Issue Details

Partial fix for #76721.
Sets the paths right, so that running e.g.

dotnet run -r browser-wasm -c Debug --project src/libraries/System.Linq/tests/System.Linq.Tests.csproj --debug --host browser -p:DebuggerSupport=true 

results in:

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:07.29
WasmAppHost --runtime-config C:\Users\itomkowicz\source\repos\runtime-fork\artifacts\bin\System.Linq.Tests\Debug\net7.0\browser-wasm\AppBundle\WasmTestRunner.runtimeconfig.json --debug --host browser
Debug proxy for chrome now listening on http://127.0.0.1:65095/. And expecting chrome at http://localhost:9222/
Hosting environment: Production
Content root path: C:\Users\itomkowicz\source\repos\runtime-fork\artifacts\bin\System.Linq.Tests\Debug\net7.0\browser-wasm\AppBundle
Now listening on: http://127.0.0.1:65095
Debug proxy for firefox now listening on tcp://127.0.0.1:65096. And expecting firefox at port 6000 .

App url: http://127.0.0.1:9000/index.html
App url: https://127.0.0.1:65097/index.html
Author: ilonatommy
Assignees: ilonatommy
Labels:

arch-wasm, area-Infrastructure-libraries

Milestone: -

@ghost
Copy link

ghost commented Nov 1, 2022

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

Issue Details

Partial fix for #76721.
Sets the paths right, so that running e.g.

dotnet run -r browser-wasm -c Debug --project src/libraries/System.Linq/tests/System.Linq.Tests.csproj --debug --host browser -p:DebuggerSupport=true 

results in:

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:07.29
WasmAppHost --runtime-config C:\Users\itomkowicz\source\repos\runtime-fork\artifacts\bin\System.Linq.Tests\Debug\net7.0\browser-wasm\AppBundle\WasmTestRunner.runtimeconfig.json --debug --host browser
Debug proxy for chrome now listening on http://127.0.0.1:65095/. And expecting chrome at http://localhost:9222/
Hosting environment: Production
Content root path: C:\Users\itomkowicz\source\repos\runtime-fork\artifacts\bin\System.Linq.Tests\Debug\net7.0\browser-wasm\AppBundle
Now listening on: http://127.0.0.1:65095
Debug proxy for firefox now listening on tcp://127.0.0.1:65096. And expecting firefox at port 6000 .

App url: http://127.0.0.1:9000/index.html
App url: https://127.0.0.1:65097/index.html
Author: ilonatommy
Assignees: ilonatommy
Labels:

arch-wasm, area-Infrastructure-libraries

Milestone: -

@ilonatommy
Copy link
Member Author

It needs more investigation, some tests WBT are failing with

An error occurred trying to start process '/datadisks/disk1/work/B23E09A6/w/B0E30929/e/dotnet-net7+latest/dotnet' with working directory '/datadisks/disk1/work/B23E09A6/w/B0E30929/e/wbt/Release_natwm0cn.rf0/bin/Release/net7.0/AppBundle'. No such file or directory.

@ilonatommy ilonatommy marked this pull request as draft November 1, 2022 17:11
@radical
Copy link
Member

radical commented Nov 1, 2022

The app bundle path is incorrect - /datadisks/disk1/work/B23E09A6/w/B0E30929/e/wbt/Debug_g1z0cnbw.fta/bin/Debug/net7.0/browser-wasm/AppBundle/ vs /datadisks/disk1/work/B23E09A6/w/B0E30929/e/wbt/Debug_g1z0cnbw.fta/bin/Debug/net7.0/AppBundle.

@ghost
Copy link

ghost commented Nov 1, 2022

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

Issue Details

Partial fix for #76721.
Sets the paths right, so that running e.g.

dotnet run -r browser-wasm -c Debug --project src/libraries/System.Linq/tests/System.Linq.Tests.csproj --debug --host browser -p:DebuggerSupport=true 

results in:

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:07.29
WasmAppHost --runtime-config C:\Users\itomkowicz\source\repos\runtime-fork\artifacts\bin\System.Linq.Tests\Debug\net7.0\browser-wasm\AppBundle\WasmTestRunner.runtimeconfig.json --debug --host browser
Debug proxy for chrome now listening on http://127.0.0.1:65095/. And expecting chrome at http://localhost:9222/
Hosting environment: Production
Content root path: C:\Users\itomkowicz\source\repos\runtime-fork\artifacts\bin\System.Linq.Tests\Debug\net7.0\browser-wasm\AppBundle
Now listening on: http://127.0.0.1:65095
Debug proxy for firefox now listening on tcp://127.0.0.1:65096. And expecting firefox at port 6000 .

App url: http://127.0.0.1:9000/index.html
App url: https://127.0.0.1:65097/index.html
Author: ilonatommy
Assignees: ilonatommy
Labels:

arch-wasm, area-Infrastructure-mono

Milestone: -

@ghost
Copy link

ghost commented Dec 23, 2022

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

@ilonatommy ilonatommy marked this pull request as ready for review January 23, 2023 07:26
@ghost ghost removed the needs-author-action An issue or pull request that requires more info or actions from the author. label Jan 25, 2023
@ilonatommy
Copy link
Member Author

Earlier failures, when we combined arguments in JS from the url's query and runArgs.json, were caused by nodeJs that does not have fetch function in the version==14.18.
Previously, we never actually used fetch("runArgs.json") because the condition: if (queryArguments.length > 0) then do not fetch runArgs.json was always true.

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.

Looks good. I would suggest adding instructions on how to use this with library tests in docs, but that can be in a follow up PR too.

Good job, and thanks for your patience, and all the testing needed for this one! 👍

src/mono/wasm/test-main.js Outdated Show resolved Hide resolved
if (!response.ok) {
console.debug(`could not load /args.json: ${response.status}. Ignoring`);
let runArgsJson;
// ToDo: runArgs should be read for all kinds of hosts, but
Copy link
Member

Choose a reason for hiding this comment

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

Can you please open an issue to track this?

Copy link
Member Author

Choose a reason for hiding this comment

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

@ilonatommy
Copy link
Member Author

Looks good. I would suggest adding instructions on how to use this with library tests in docs, but that can be in a follow up PR too.

Good job, and thanks for your patience, and all the testing needed for this one! 👍

Like this? #77368

Co-authored-by: Ankit Jain <radical@gmail.com>
@ilonatommy ilonatommy merged commit befeec4 into dotnet:main Jan 31, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Mar 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants