-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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][MT] enable more tests #97391
[browser][MT] enable more tests #97391
Conversation
Tagging subscribers to 'arch-wasm': @lewing Issue Detailsnull
|
/azp run runtime-wasm |
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, I only have concerns about change in runtime-extra-platforms-wasm.yml
@@ -130,8 +130,6 @@ jobs: | |||
# Always run for runtime-wasm because tests are not run in runtime | |||
alwaysRun: ${{ parameters.isWasmOnlyBuild }} | |||
|
|||
# NOTE - Since threading is experimental, we don't want to block mainline work | |||
shouldContinueOnError: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't it be in a future PR, where we don't simultaneously enable any new tests, just change the warning to error? Are we 100% sure about the enabled tests to not fail?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to start moving quicker now. This is the way how it will get noticed, rather than ignored.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the last 7 days we had 7 PR that are not connected with threading that had failures on '_Threading' lanes (both timeouts and BadExists). There is 100% probability that this change will introduce a mess
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's a goal. To get people to start working the [ActiveIssue]
chore, instead of ignoring like for the last 6 months.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @lewing is it good time to start moving on this ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we need to start standing up the tests and marking the failing ones asap. I'll let people know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm working to make it as green as possible before we merge this.
/azp run runtime-wasm |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-wasm |
Azure Pipelines successfully started running 1 pipeline(s). |
d40aee7
to
767a4c3
Compare
/azp run runtime-wasm |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-wasm |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-wasm |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-wasm |
Azure Pipelines successfully started running 1 pipeline(s). |
dc9eb39
to
dc13b06
Compare
/azp run runtime-wasm |
Azure Pipelines successfully started running 1 pipeline(s). |
I filled #97518, the rest is green or known |
No description provided.