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

Added runtime dependency to fix the intermittent test failures #54587

Merged
merged 5 commits into from
Jun 23, 2021
Merged

Added runtime dependency to fix the intermittent test failures #54587

merged 5 commits into from
Jun 23, 2021

Conversation

Daniel-Genkin
Copy link
Contributor

Fixed the intermittent test failures caused by mono-config.json being loaded out of order

@ghost
Copy link

ghost commented Jun 23, 2021

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

Issue Details

Fixed the intermittent test failures caused by mono-config.json being loaded out of order

Author: Daniel-Genkin
Assignees: -
Labels:

area-VM-meta-mono
Milestone: -

@Daniel-Genkin
Copy link
Contributor Author

Daniel-Genkin commented Jun 23, 2021 •
edited
Loading

@kg Is this what you meant? Also, can you please add the necessary reviewers. I still don't have access to do it.

kg
kg reviewed Jun 23, 2021
View reviewed changes
src/mono/sample/mbr/browser/runtime.js Outdated
Module.config = await MONO.mono_wasm_load_config("./mono-config.json");
Module.removeDependency('mono-config.json');
Copy link
Contributor

Choose a reason for hiding this comment

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

Indentation got mucked up here

@kg
Copy link
Contributor

kg commented Jun 23, 2021

It might make sense to hoist all this complexity into mono_wasm_load_config, and introduce a 'mono_wasm_load_config_async' which adds the dependency, kicks off the async load, then removes the dependency. That way you don't have to touch all these files any time something changes, and it's easy for people to properly imitate the pattern.

@Daniel-Genkin
Copy link
Contributor Author

Daniel-Genkin commented Jun 23, 2021 •
edited
Loading

Sorry, I was rushing. Haha this is straight from my nightmares. Also, I can't test it locally since my local machine's build is currently messed up

lewing
lewing requested changes Jun 23, 2021
View reviewed changes
Copy link
Member

@lewing lewing left a comment

Choose a reason for hiding this comment

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

pass configFilePath as the

src/mono/wasm/runtime/library_mono.js Outdated Show resolved Hide resolved
src/mono/wasm/runtime/library_mono.js Outdated Show resolved Hide resolved
@Daniel-Genkin
Copy link
Contributor Author

Looks like the tests that were failing for Ankit and Katelyn now pass
https://helixre8s23ayyeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-54587-merge-ee87aa6564394220aa/System.Net.Http.Enterprise.Tests/console.e71ce652.log?sv=2019-07-07&se=2021-07-13T02%3A56%3A40Z&sr=c&sp=rl&sig=cywykSQQsk2Ik7XAuj37PYFu5zTeNRzOpHBKwAlcHDQ%3D

@Daniel-Genkin Daniel-Genkin changed the title Added runtime dependency to hopefully fix the intermittent test failures Added runtime dependency to fix the intermittent test failures Jun 23, 2021
@lewing lewing added the arch-wasm WebAssembly architecture label Jun 23, 2021
@ghost
Copy link

ghost commented Jun 23, 2021

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

Issue Details

Fixed the intermittent test failures caused by mono-config.json being loaded out of order

Author: Daniel-Genkin
Assignees: -
Labels:

arch-wasm, area-VM-meta-mono
Milestone: -

@lewing lewing merged commit d87b783 into dotnet:main Jun 23, 2021
@lewing
Copy link
Member

lewing commented Jun 23, 2021 •
edited
Loading

This is a JavaScript only change that cannot impact the other failing lanes.

@lewing lewing mentioned this pull request Jun 23, 2021
Merged
thaystg added a commit to thaystg/runtime that referenced this pull request Jun 23, 2021
@thaystg
Merge remote-tracking branch 'origin/main' into thays_fix_exception_i…
ae2a17c 
…nitial_config

* origin/main: (362 commits)
  [wasm][debugger] Reuse debugger-agent on wasm debugger (dotnet#52300)
  Put Crossgen2 in sync with dotnet#54235 (dotnet#54438)
  Move System.Object serialization to ObjectConverter (dotnet#54436)
  Move setting fHasVirtualStaticMethods out of sanity check section (dotnet#54574)
  [wasm] Compile .bc->.o in parallel, before passing to the linker (dotnet#54053)
  Change PathInternal.IsCaseSensitive to a constant (dotnet#54340)
  Make mono_polling_required a public symbol (dotnet#54592)
  Respect EventSource::IsSupported setting in more codepaths (dotnet#51977)
  Root ComActivator for hosting (dotnet#54524)
  Add ILLink annotations to S.D.Common related to DbConnectionStringBuilder (dotnet#54280)
  Fix finalizer issue with regions (dotnet#54550)
  Add support for multi-arch install locations (dotnet#53763)
  Update library testing docs page to reduce confusion (dotnet#54324)
  [FileStream] handle UNC and device paths (dotnet#54483)
  Update NetAnalyzers version (dotnet#54511)
  Added runtime dependency to fix the intermittent test failures (dotnet#54587)
  Disable failing System.Reflection.Tests.ModuleTests.GetMethods (dotnet#54564)
  [wasm] Move AOT builds from `runtime-staging` to `runtime` (dotnet#54577)
  Keep obj node for ArrayIndex. (dotnet#54584)
  Disable another failing MemoryCache test (dotnet#54578)
  ...
@ghost ghost locked as resolved and limited conversation to collaborators Jul 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kg kg kg left review comments

@lewing lewing lewing approved these changes

@marek-safar marek-safar Awaiting requested review from marek-safar

Assignees
No one assigned
Labels
arch-wasm WebAssembly architecture area-VM-meta-mono
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Daniel-Genkin @kg @lewing