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] Fix support for using ActiveIssue, and other related attributes #62660

Closed
radical opened this issue Dec 10, 2021 · 1 comment · Fixed by #65684
Closed

[wasm][debugger] Fix support for using ActiveIssue, and other related attributes #62660

radical opened this issue Dec 10, 2021 · 1 comment · Fixed by #65684

Comments

@radical
Copy link
Member

radical commented Dec 10, 2021

This PR adds support for running wasm debugger tests on helix. But currently, it is using dotnet test to run these.

40d6570 (#62431):

[wasm] Add Wasm.Debugger.Tests wrapper project  …
This is a proxy/wrapper project for `src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestSuite.csproj`.

- Building the project as part of the regular browser-wasm build
  presents some issues, because of part the tests need use the aspnet
  sdk, and that doesn't work with `browser-wasm`.
- This wrapper project essentially builds the `DebuggerTestSuite`
  project, with some
  properties(`TargetFramework;TargetFrameworks;Configuration;TargetOS;TargetArchitecture`)
  removed so they don't propogate from the parent build.
  - And it packages it up for running the tests on helix

- I did try to convert `DebuggerTestSuite` into a `Wasm.Debugger.Tests`,
  and make it use the library tests infrastructure, but ran into an
  issue
  - it built the project with no `testhost.dll`, so can't use `dotnet
    test`
  - it did get `xunit.console.dll`, but that would fail to run the tests
    because of missing `System.Runtime` (and I'm guessing, other
    assemblies)
    - attempts to publish the project failed
    - So, for now, this is what we have!```

IIUC, the test discoverers are not being run with this, which means that we can't use `ActiveIssue`, and other similar attributes. As a workaround, it uses `windows-failing`, and `linux-failing` traits.
@ghost
Copy link

ghost commented Dec 10, 2021

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

Issue Details

This PR adds support for running wasm debugger tests on helix. But currently, it is using dotnet test to run these.

40d6570 (#62431):

[wasm] Add Wasm.Debugger.Tests wrapper project  …
This is a proxy/wrapper project for `src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestSuite.csproj`.

- Building the project as part of the regular browser-wasm build
  presents some issues, because of part the tests need use the aspnet
  sdk, and that doesn't work with `browser-wasm`.
- This wrapper project essentially builds the `DebuggerTestSuite`
  project, with some
  properties(`TargetFramework;TargetFrameworks;Configuration;TargetOS;TargetArchitecture`)
  removed so they don't propogate from the parent build.
  - And it packages it up for running the tests on helix

- I did try to convert `DebuggerTestSuite` into a `Wasm.Debugger.Tests`,
  and make it use the library tests infrastructure, but ran into an
  issue
  - it built the project with no `testhost.dll`, so can't use `dotnet
    test`
  - it did get `xunit.console.dll`, but that would fail to run the tests
    because of missing `System.Runtime` (and I'm guessing, other
    assemblies)
    - attempts to publish the project failed
    - So, for now, this is what we have!```

IIUC, the test discoverers are not being run with this, which means that we can't use `ActiveIssue`, and other similar attributes. As a workaround, it uses `windows-failing`, and `linux-failing` traits.

<table>
  <tr>
    <th align="left">Author:</th>
    <td>radical</td>
  </tr>
  <tr>
    <th align="left">Assignees:</th>
    <td>-</td>
  </tr>
  <tr>
    <th align="left">Labels:</th>
    <td>

`arch-wasm`, `area-Build-mono`, `area-Debugger-mono`

</td>
  </tr>
  <tr>
    <th align="left">Milestone:</th>
    <td>-</td>
  </tr>
</table>
</details>

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Dec 10, 2021
@radical radical removed the untriaged New issue has not been triaged by the area owner label Dec 10, 2021
radical added a commit to radical/runtime that referenced this issue Dec 10, 2021
Issue: dotnet#62661

Using `[Trait..` instead of `ActiveIssue` because: dotnet#62660
thaystg pushed a commit that referenced this issue Dec 16, 2021
* [wasm] Don't emit warning if runtimeconfig.json cannot be found

Library projects don't have runtimeconfig.json files by default. So,
don't make it a warning. Instead, emit a low importance which might be
useful when debugging.

But library projects can have runtimeconfig.json, like the runtime test
projects. So, don't limit processing that by OutputType.

IOW, if it's found then use it.

* [wasm] Add timestamp to logs

* Download dotnet-install script for installing workloads

Instead of trying to use the script from `.dotnet`, download the script.
`.dotnet` might not exist, for example, when the `global.json` version
matches the system installed one.

* [wasm] WasmAppBuilder: catch UnauthorizedAccessException also

* [wasm] Fix bug in tests

Some helper methods have a `Action<JToken>` parameter. Many tests
pass an async lambda to this, expecting it to get awaited upon.

```csharp
    EvaluateAndCheck (Action<T> locals_fn)
    {
        ...
        locals_fn(); // no await
        ...
    }

    async Task Test()
    {
        EvaluateAndCheck( async (locals) => {
            ...
            CheckNumber(locals, ...);

            await CheckDateTime(locals, ..);
            ...

        } );
    }
```

In the above example, roslyn generates an async-void lambda, so the
compiler never complains about the async lambda being passed.
`EvaluateAndCheck` cannot, and does not await this, but if the lambda happens to
block, then it will return at that point, and the test(calling method) will end,
without ever completing the lambda. And for most tests, the actual
checks are done in that lambda.

This gets hit when `CheckDateTimeValue` tries to fetch properties of a
`DateTime` object. And it started to show up when adding
`ConfigureAwait(false)` to some calls.

* [wasm] Add Wasm.Debugger.Tests wrapper project

This is a proxy/wrapper project for `src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestSuite.csproj`.

- Building the project as part of the regular browser-wasm build
  presents some issues, because of part the tests need use the aspnet
  sdk, and that doesn't work with `browser-wasm`.
- This wrapper project essentially builds the `DebuggerTestSuite`
  project, with some
  properties(`TargetFramework;TargetFrameworks;Configuration;TargetOS;TargetArchitecture`)
  removed so they don't propogate from the parent build.
  - And it packages it up for running the tests on helix

- I did try to convert `DebuggerTestSuite` into a `Wasm.Debugger.Tests`,
  and make it use the library tests infrastructure, but ran into an
  issue
  - it built the project with no `testhost.dll`, so can't use `dotnet
    test`
  - it did get `xunit.console.dll`, but that would fail to run the tests
    because of missing `System.Runtime` (and I'm guessing, other
    assemblies)
    - attempts to publish the project failed
    - So, for now, this is what we have!

* [wasm][tests] Make them friendly to running outside the tree

.. like on helix.

Add new `DEBUGGER_TEST_PATH`, and `CHROME_PATH_FOR_DEBUGGER_TESTS` which
will be set for helix.

And change the appbundle directory name from the misleading `publish/`
to `AppBundle/`.

* [wasm] Tests.cs -> MiscTests.cs

* [wasm] Add support for submitting debugger tests to helix

Also, added `eng/testing/scenarios/WasmDebuggerTestsJobsList.txt` which
is a manually generated list of test classes. This will be changed to be
generated at runtime, in an upcoming PR.

* [wasm] Add debugger tests job for linux, and windows

They follow the same pattern as other wasm jobs:

- build when isFullMatrix
- build in runtime-manual
- Additionally, build when there are changes in:

```
  - src/mono/wasm/debugger/*
  - src/mono/wasm/runtime/*
  - src/mono/mono/*
```

* [wasm] Add new make targets to submit tests to helix

`submit-debugger-tests-helix`
`submit-tests-helix` - submits any library test archives

* Build Wasm.Debugger.Tests from src/libraries/tests.proj

* DebuggerTestSuite: Copy files for the test archive

* [wasm] Fix HarnessTests.BrowserClose

* [wasm] Fix building `ApplyUpdateReferencedAssembly` project on CI

Essentially, disable use of SourceLink which gets enabled by default
when using `-p:ContinuousIntegrationBuild=true`.

Issue: #62618

* cleanup

* Wasm.Build.Tests: add missing file

* [wasm] sendtohelixhelp.proj: Error out if there is more than one zip

.. file when running for Wasm.Build.Tests, or Wasm.Debugger.Tests .

* [wasm] Disable DebuggerTests.ArrayTests on helix

Issue: #62661

Using `[Trait..` instead of `ActiveIssue` because: #62660

* disable non-wasma builds

* sendtohelixhelp.proj: guard against no payload found

* Disable more tests

* add back builds

* [wasm][debugger] Disable failing debugger test

`DebuggerTests.BreakpointTests.BreakpointInAssemblyUsingTypeFromAnotherAssembly_BothDynamicallyLoaded`

Issue: #62823

* Try to fix windows command line

* Move debugger-tests for linux to runtime-staging

* Revert "[wasm][debugger] Fix source-link test (#62786)"

.. as it is breaking debugger tests build on windows.

Issue: #62892
This reverts commit 8151740.
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Feb 22, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Feb 22, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Mar 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant