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][debugger] Run debugger test in codespace #64746

Merged
merged 18 commits into from
Feb 25, 2022

Conversation

ilonatommy
Copy link
Member

@ilonatommy ilonatommy commented Feb 3, 2022

Enables running debugger tests with no additional package installations.
Running a sample debugger test:

dotnet test /p:RuntimeConfiguration=Release /p:Configuration=Debug src/mono/wasm/debugger/DebuggerTestSuite  --filter DebuggerTests.SteppingTests.InspectLocalsDuringSteppingIn

Changes in this PR:

  • Added chromium installation to the dockerfile.
  • Added --no-sandbox to arguments passed to harness when we detect that code is running in a container.

Recommendation:
use at least 8 core VM. For smaller sizes you can encounter an error: Less than 64MB of free space in temporary directory for shared memory files.

Test that always fail in codespace on main (different than typically):

DebuggerTests.ArrayTests.InvalidArrayId [FAIL]
DebuggerTests.EvaluateOnCallFrameTests.EvaluateSimpleMethodCallsError [FAIL]

Possible workaround if --no-sanbox was in some point inconvenient -> adding to devcontainer.json:

"runArgs": ["--security-opt","seccomp=.devcontainer/chrome.json"],

in this solution bigger VM is also needed because of threads' crashes:

Gathering state for process 13427 dotnet
Crashing thread 000052dd signal 00000006

unless we use the VM with 16 core or 8 core.

@ilonatommy ilonatommy added arch-wasm WebAssembly architecture area-Debugger-mono labels Feb 3, 2022
@ilonatommy ilonatommy self-assigned this Feb 3, 2022
@ghost
Copy link

ghost commented Feb 3, 2022

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

Issue Details
  • Added chromium installation to the dockerfile.
  • Added --no-sandbox option needed for running tests in codespaces (locally works with this option).
Author: ilonatommy
Assignees: ilonatommy
Labels:

arch-wasm, area-Debugger-mono

Milestone: -

@ilonatommy ilonatommy removed the request for review from marek-safar February 3, 2022 13:56
@thaystg thaystg requested a review from lambdageek February 3, 2022 13:57
@fanyang-mono
Copy link
Member

The change to the Dockerfile looks good to me. I wonder if you change to the test harness startup file triggered the failures on CI.

@radical
Copy link
Member

radical commented Feb 3, 2022

Some of these failures were due to missing helix queues, which is fixed on main.

@ilonatommy ilonatommy marked this pull request as draft February 10, 2022 18:24
@ilonatommy ilonatommy marked this pull request as ready for review February 18, 2022 18:24
.devcontainer/scripts/postCreateCommand.sh Outdated Show resolved Hide resolved
.devcontainer/Dockerfile Outdated Show resolved Hide resolved
src/mono/wasm/Makefile Outdated Show resolved Hide resolved
@ilonatommy ilonatommy requested a review from eerhardt February 21, 2022 13:50
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.

Should we print out a diagnostic message saying we've disabled sandboxes?

Copy link
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

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

This approach looks good to me.

thaystg
thaystg previously approved these changes Feb 22, 2022
@thaystg thaystg dismissed their stale review February 23, 2022 00:34

Need to remove the installation part

Copy link
Member

@thaystg thaystg left a comment

Choose a reason for hiding this comment

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

12084e4 remove the installation part as @radical has already implemented it in this other PR

@ghost ghost added the needs-author-action An issue or pull request that requires more info or actions from the author. label Feb 23, 2022
@ghost ghost removed the needs-author-action An issue or pull request that requires more info or actions from the author. label Feb 23, 2022
@ilonatommy ilonatommy merged commit ef785f5 into dotnet:main Feb 25, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Mar 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-Debugger-mono
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants