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

[browser] stop testing with nodejs #108582

Merged

Conversation

pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented Oct 7, 2024

To reduce WASM test matrix

  • removed pipelines
  • WasmTestOnNodeJS scenario
  • Wasm.Build.Tests tests
  • Old Blazor_net50 project that was added in [wasm] Don't use workload for tfm < net6.0 #56606 to be used in WBT but we started using different logic to test Blazor since then and it can be safely removed.

I left in place

  • PlatformDetection.IsNotNodeJS
    • HTTP & WS tests
    • Hybrid globalization tests
  • detection in `src\mono\browser\test-main.js
  • any production code

@pavelsavara pavelsavara added arch-wasm WebAssembly architecture area-Build-mono os-browser Browser variant of arch-wasm labels Oct 7, 2024
@pavelsavara pavelsavara added this to the 10.0.0 milestone Oct 7, 2024
@pavelsavara pavelsavara self-assigned this Oct 7, 2024
@pavelsavara pavelsavara marked this pull request as ready for review October 7, 2024 12:34
Copy link
Member

@ilonatommy ilonatommy left a comment

Choose a reason for hiding this comment

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

We might be missing removal of this and places it's being called at:

Edit: it's not called from anywhere...

@pavelsavara
Copy link
Member Author

  • UpdateRuntimeconfigTemplateForNode

Is used for wasmconsole template testing, which is V8. Do we want to drop that template too and simplify the host ?

@ilonatommy
Copy link
Member

ilonatommy commented Oct 7, 2024

  • UpdateRuntimeconfigTemplateForNode

Is used for wasmconsole template testing, which is V8. Do we want to drop that template too and simplify the host ?

We're not testing libs with v8 anymore, so that would make it consistent.
Edit: actually we are, in outerloop and extra-platforms. Sorry, then this argument does not make sense.

@maraf
Copy link
Member

maraf commented Oct 8, 2024

Do we want to drop that template too and simplify the host ?

Yes, it can be done here or in a follow up.

Are we going to explicitly remove parts of typescript related to node?

@pavelsavara
Copy link
Member Author

Do we want to drop that template too and simplify the host ?

Yes, it can be done here or in a follow up.

Let's do it in follow up.

@lewing please confirm we have quorum on removing console from dotnet new and dotnet run

Are we going to explicitly remove parts of typescript related to node?

Let's do that in yet another separate PR and we will see how many things there are. I guess it's not that many and possibly we could leave it in place.

@ilonatommy
Copy link
Member

ilonatommy commented Oct 8, 2024

I understand that we are using v8 to save resources during running tests, however I don't see much difference in the time used in
/p:Scenario=WasmTestOnChrome and /p:Scenario=WasmTestOnV8, checking biggest lib test project: System.Runtime.Tests (test count differs, so let's look at the averaged value)

V8:     total="64976" time="122.362"
Chrome: total="64970" time="114.317" 
Averaged chome time if it had same number of tests as v8: 122,350

I am proposing:

lane current scenarios future scenarios changed?
LibraryTests Linux WasmTestOnV8,WasmTestOnChrome,WasmTestOnFirefox WasmTestOnChrome,WasmTestOnFirefox Y
LibraryTests Windows WasmTestOnChrome WasmTestOnChrome N
LibraryTests_Smoke_AOT Linux WasmTestOnV8 WasmTestOnChrome Y
LibraryTests_Smoke_AOT Windows WasmTestOnChrome WasmTestOnChrome N
LibraryTests_EAT Linux WasmTestOnV8 WasmTestOnChrome Y
LibraryTests_Threading Linux WasmTestOnChrome,WasmTestOnFirefox WasmTestOnChrome,WasmTestOnFirefox N
extra-platforms WasmTestOnV8, WasmTestOnChrome WasmTestOnChrome Y
outer-loop WasmTestOnV8 WasmTestOnChrome Y

@ilonatommy
Copy link
Member

/ba-g failures are DebuggerTests, not related

@ilonatommy ilonatommy merged commit 59a460a into dotnet:main Oct 8, 2024
155 of 161 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-wasm WebAssembly architecture area-Build-mono os-browser Browser variant of arch-wasm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants