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

[MT][browser] Disable new CI failures on library MT tests #93224

Merged
merged 4 commits into from
Oct 11, 2023

Conversation

ilonatommy
Copy link
Member

@ilonatommy ilonatommy commented Oct 9, 2023

  • Block Wasm.Browser.Threads.Minimal.Sample.csproj for MT

  • Move issues connected with MT exclusions to be blocked only in MT mode.

  • Add project exclusion for Microsoft.Extensions.DependencyInjection.Tests

  • Fixing fail: [out of order message from the browser]: http://127.0.0.1:33323/_framework/dotnet.js 2 WebSocket connection to 'ws://127.0.0.1:33323/_framework/dotnet.native.worker.js/console' failed: Error during WebSocket handshake: Unexpected response code: 404

@ghost
Copy link

ghost commented Oct 9, 2023

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

Issue Details
  • We were planning to switch off shouldContinueOnError after a few weeks from merging [browser][MT] enable all MT library tests on the runtime CI pipeline #91536 if it will not be failing after disabling.
  • Fixing fail: [out of order message from the browser]: http://127.0.0.1:33323/_framework/dotnet.js 2 WebSocket connection to 'ws://127.0.0.1:33323/_framework/dotnet.native.worker.js/console' failed: Error during WebSocket handshake: Unexpected response code: 404 log
Author: ilonatommy
Assignees: -
Labels:

arch-wasm, area-VM-threading-mono

Milestone: -

@pavelsavara
Copy link
Member

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@radical
Copy link
Member

radical commented Oct 9, 2023

How often has this run recently, and how often has it failed?

@pavelsavara
Copy link
Member

How often has this run recently, and how often has it failed?

It failed "silently" for me. I have not noticed that there is "organge" lane. I want it to really fail when I do /azp run runtime-wasm

@radical
Copy link
Member

radical commented Oct 9, 2023

'runtime-wasm' will be red then.

@pavelsavara
Copy link
Member

'runtime-wasm' will be red then.

Right, that's what we want to anything that was not already marked active issue by Ilona in #91536.

Am I missing something ?

@ilonatommy
Copy link
Member Author

This PR needs adding a blocking for Wasm.Browser.Threads.Minimal.Sample and Microsoft.Extensions.DependencyInjection.Tests, they might have been overlooked in the big blocking PR (I will add it). Beyond that, nothing else failed. If a new failure will arise, we will at least notice it thanks to this change.

@radical
Copy link
Member

radical commented Oct 9, 2023

'runtime-wasm' will be red then.

Right, that's what we want to anything that was not already marked active issue by Ilona in #91536.

Am I missing something ?

Has the job been green for few weeks when there were no PR introduced mt failures? That's what I'm talking about. It shouldn't randomly fail, else we would start getting merges on red and failures getting ignored because users aren't sure whether the failures are expected. A red pipeline on PRs that did not introduce mt failures would be counterproductive.

@radical
Copy link
Member

radical commented Oct 9, 2023

This PR needs adding a blocking for Wasm.Browser.Threads.Minimal.Sample and Microsoft.Extensions.DependencyInjection.Tests, they might have been overlooked in the big blocking PR (I will add it). Beyond that, nothing else failed. If a new failure will arise, we will at least notice it thanks to this change.

Then it hasn't been green/stable for few weeks.

@ilonatommy
Copy link
Member Author

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

src/libraries/tests.proj Outdated Show resolved Hide resolved
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.

Please don't merge this yet. We should have couple of weeks worth of builds where the job has been green, before we merge.

@ghost ghost added the needs-author-action An issue or pull request that requires more info or actions from the author. label Oct 9, 2023
@ghost ghost removed the needs-author-action An issue or pull request that requires more info or actions from the author. label Oct 9, 2023
@ilonatommy
Copy link
Member Author

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member

Please don't merge this yet. We should have couple of weeks worth of builds where the job has been green, before we merge.

In that case we need to make the test run much more often. What we have now is useless waiting without any observability. Do you have any proposal @radical ?

@radical
Copy link
Member

radical commented Oct 11, 2023

Please don't merge this yet. We should have couple of weeks worth of builds where the job has been green, before we merge.

In that case we need to make the test run much more often. What we have now is useless waiting without any observability. Do you have any proposal @radical ?

The test failures are observable, just that they won't show up as failures in github. Open the azdo build and you can see the failures, and all the details.

@ilonatommy
Copy link
Member Author

Let's merge at least these fixes for now. Failures are unrelated.

@radical
Copy link
Member

radical commented Oct 11, 2023

Let's merge at least these fixes for now.

👍 Please update the PR title, and description to match.

@ilonatommy ilonatommy changed the title [MT][browser] Allow CI failures on library MT tests [MT][browser] Disable new CI failures on library MT tests Oct 11, 2023
@radical
Copy link
Member

radical commented Oct 11, 2023

Also, please open an issue for the error that is unrelated, and isn't being flagged by build analysis.

@ilonatommy ilonatommy merged commit 7f63b6b into dotnet:main Oct 11, 2023
125 of 131 checks passed
@ghost ghost locked as resolved and limited conversation to collaborators Nov 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-VM-threading-mono
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants