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

[wasm] Add Wasm.Build tests, for testing wasm app builds #47683

Merged
merged 23 commits into from
Mar 9, 2021
Merged

[wasm] Add Wasm.Build tests, for testing wasm app builds #47683

merged 23 commits into from
Mar 9, 2021

Conversation

radical
Copy link
Member

@radical radical commented Jan 30, 2021 •
edited
Loading

These tests will build wasm test projects, as part of each test method,
and run them.

Other library tests are run with xharness, and the test assembly is run
under wasm. But here we want to run them with xunit, outside wasm. So,
this has different requirements for the helix payload, eg, the sdk,
xunit console runner etc.

To make it work, a new Scenario - BuildWasmApps is added, which emits
it's archives in a buildwasmapps/ folder, which makes it easy to pick
up for the helix test run.

The tests are added under src/tests/BuildWasmApps/Wasm.Build.Tests, but
they use Directory.Build* from src/libraries, similar to how
FunctionalTests do it.

Another use case of this kinda scenario are the wasm debugger tests,
in which the individual test methods launch wasm apps, and then debug
them. (TBD)

Tests:

  • The initial set of tests are just proof-of-concept, and more will be
    added once this is merged.

Note: The individual tests build test projects, and then run them with
xharness, under v8, and Chrome.

Note: Emscripten doesn't seem to be available on helix currently. So, this
PR packages that up as a helix correlation payload.

@runfoapp runfoapp bot mentioned this pull request Feb 1, 2021
Closed
@radical radical force-pushed the tmp-wasm-build-tests branch 2 times, most recently from 67c9c10 to 307199c Compare February 22, 2021 06:16
@radical radical changed the title [IGNORE] trying tests for wasm project builds trying tests for wasm project builds Feb 22, 2021
akoeplinger
akoeplinger reviewed Feb 23, 2021
View reviewed changes
src/libraries/Wasm.Build/tests/Wasm.Build.Tests.csproj Outdated Show resolved Hide resolved
eng/pipelines/runtime.yml Outdated Show resolved Hide resolved
src/libraries/Wasm.Build/tests/Directory.Build.props Outdated Show resolved Hide resolved
@radical radical changed the title trying tests for wasm project builds [wasm] Add Wasm.Build tests, for testing wasm app builds Feb 24, 2021
@radical radical force-pushed the tmp-wasm-build-tests branch 2 times, most recently from 7bc4acc to 203db70 Compare February 24, 2021 00:27
@radical radical marked this pull request as ready for review February 24, 2021 00:28
@radical radical added the arch-wasm WebAssembly architecture label Feb 24, 2021
@ghost
Copy link

ghost commented Feb 24, 2021

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

Issue Details

These tests will build wasm test projects, as part of each test method,
and run them.

Other library tests are run with xharness, and the test assembly is run
under wasm. But here we want to run them with xunit, outside wasm. So,
this has different requirements for the helix payload, eg, the sdk,
xunit console runner etc.

To make it work, a new Scenario - RunWithXUnit is added, which emits
it's archives in a runwithxunit/ folder, which makes it easy to pick
up for the helix test run.

The tests are added under src/tests/TestsRunWithXUnit/Wasm.Build/tests, but
they use Directory.Build* from src/libraries, similar to how
FunctionalTests do it.

Another use case of this kinda scenario are the wasm debugger tests,
in which the individual test methods launch wasm apps, and then debug
them. (TBD)

Tests:

  • The AOT ones are disabled right now, because I couldn't find
    emscripten installed on helix (eg in /usr/local/emscripten).
  • The initial set of tests are just proof-of-concept, and more will be
    added once this is merged.
Author: radical
Assignees: -
Labels:

arch-wasm, area-Build-mono
Milestone: -

@radical
Copy link
Member Author

radical commented Feb 24, 2021

Moving it to src/tests seems to causing other issues, iterating on that now.

This was referenced Feb 24, 2021
Closed
Open
@radical radical added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Feb 26, 2021
@radical
Copy link
Member Author

radical commented Feb 26, 2021

Labeled this with no-merge while I get the AOT tests working on helix. But the approach as such is working.

@radical radical force-pushed the tmp-wasm-build-tests branch 3 times, most recently from cce0b75 to 4e0c6cb Compare February 27, 2021 14:25
Base automatically changed from master to main March 1, 2021 09:07
@radical radical force-pushed the tmp-wasm-build-tests branch 3 times, most recently from 7ff0553 to 957c5c5 Compare March 2, 2021 12:18
@radical
[wasm] Enable verbose output for xunit
9600a13 
Instead of writing all the output to stdout also, use `-verbose` which
gives output like:

```
      Wasm.Build.Tests.WasmBuildAppTest.InvariantGlobalization(config: "Debug", aot: False, invariantGlobalization: null) [STARTING]
  ============== wasm test =============
  ============== wasm test-browser =============
      Wasm.Build.Tests.WasmBuildAppTest.InvariantGlobalization(config: "Debug", aot: False, invariantGlobalization: null) [FINISHED] Time: 8.6357275s
```

We log the detailed output to files anyway.
@radical
Merge remote-tracking branch 'origin/main' into dbg-again
d10510e
@radical
[wasm] fix tests
8aeec63
@radical
[wasm] Really enable verbose output for xunit, this time
b94865f
@radical
[wasm] Update tests to track the xharness fix for expected-exit-code
33efd20
@radical
Merge remote-tracking branch 'origin/main' into dbg-again
a31092d
@radical
Merge remote-tracking branch 'origin/main' into dbg-again
204b3ee
@radical
[wasm] Bump browser job's timeout from 120 to 180 mins
ac147c7
@radical
Copy link
Member Author

radical commented Mar 5, 2021

Test failures are unrelated to this PR.

@radical radical mentioned this pull request Mar 5, 2021
Merged
@radical @mdh1418
Improve comment
2bf3845 
Co-authored-by: Mitchell Hwang <mitchhwang1418@gmail.com>
@radical
Copy link
Member Author

radical commented Mar 8, 2021

In a follow up PR, I'll add sharing builds between tests, where possible, and that will reduce the run times.

@radical @mdh1418
Update eng/testing/tests.mobile.targets
31d5d2f 
Co-authored-by: Mitchell Hwang <mitchhwang1418@gmail.com>
mdh1418
mdh1418 approved these changes Mar 8, 2021
View reviewed changes
Copy link
Member

@mdh1418 mdh1418 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@radical
Remove unrelated commit
ead13ee 
Instead, this is moved to a different AOT PR.

Revert "[wasm] Disable il stripping completely"

This reverts commit 25c2340.
@radical
Merge remote-tracking branch 'origin/main' into tmp-wasm-build-tests
17ce3d8
@radical
Revert "Remove unrelated commit"
7be536d 
This is needed because `mono-cil-strip` isn't available on helix. And we
want to disable cil stripping anyway.

This reverts commit ead13ee.
@radical
Copy link
Member Author

radical commented Mar 9, 2021

Re-added the commit that disable assembly stripping. It is useful here too, because mono-cil-strip isn't available on helix.

@radical radical merged commit 8c64e30 into dotnet:main Mar 9, 2021
@radical radical deleted the tmp-wasm-build-tests branch March 9, 2021 19:01
@safern safern mentioned this pull request Mar 11, 2021
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Apr 8, 2021
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@vargaz vargaz vargaz left review comments

@akoeplinger akoeplinger akoeplinger approved these changes

@mdh1418 mdh1418 mdh1418 approved these changes

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

@lewing lewing Awaiting requested review from lewing lewing is a code owner

@steveisok steveisok Awaiting requested review from steveisok

Assignees
No one assigned
Labels
arch-wasm WebAssembly architecture area-Build-mono
Projects
None yet
Milestone
6.0.0
Development

Successfully merging this pull request may close these issues.
6 participants
@radical @vargaz @steveisok @akoeplinger @mdh1418 @karelz