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] Disable failing runtime tests #93712

Merged
merged 1 commit into from
Oct 20, 2023

Conversation

radical
Copy link
Member

@radical radical commented Oct 19, 2023

Interop/PInvoke/Vector2_3_4/ tests are failing as they depend on native test libraries. But
wasm build for runtime tests does not support that yet.

Issue: #93669

@radical radical added NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) NO-REVIEW Experimental/testing PR, do NOT review it arch-wasm WebAssembly architecture area-Infrastructure-mono labels Oct 19, 2023
@ghost
Copy link

ghost commented Oct 19, 2023

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

Issue Details

null

Author: radical
Assignees: -
Labels:

NO-MERGE, NO-REVIEW, arch-wasm, area-Infrastructure-mono

Milestone: -

@radical
Copy link
Member Author

radical commented Oct 19, 2023

@jkoritzinsky @trylek IIUC, excluding tests uses the paths specified in issues.targets. For a test like Interop/PInvoke/Vector2_3_4, a BasicTestMethod is used which uses the full method name to the filter's ShouldRunTest, and that won't match the path from issues.targets. If that is correct, then what is the recommended way of excluding such tests?
I see there is support for a dotnet-test like filter expression, but is that for manual runs?

There are existing exclusions for this like

<ExcludeList Include="$(XunitTestBinBase)/Interop/PInvoke/Vector2_3_4/Vector2_3_4/**">
<Issue>https://github.com/dotnet/runtime/issues/82859</Issue>
</ExcludeList>

.. but these do not work any more, AFAIU!

Update: There might be other similar exclusions which are not effective now, and I guess not needed either? There is another instance of the above for <ItemGroup Condition="'$(TargetArchitecture)' == 'wasm' or '$(TargetsAppleMobile)' == 'true'">.

cc @steveisok

@radical
Copy link
Member Author

radical commented Oct 19, 2023

cc @lewing

@radical radical removed NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) NO-REVIEW Experimental/testing PR, do NOT review it labels Oct 19, 2023
Copy link
Member

@jkoritzinsky jkoritzinsky left a comment

Choose a reason for hiding this comment

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

Please use the PlatformSpecific attribute or one of the other xunit/XunitExtensions attributes instead of issues.targets for merged tests like this.

@ghost ghost added the needs-author-action An issue or pull request that requires more info or actions from the author. label Oct 19, 2023
These tests are failing as they depend on native test libraries. But
wasm build for runtime tests does not support that yet.

Issue: dotnet#93669
@ghost ghost removed the needs-author-action An issue or pull request that requires more info or actions from the author. label Oct 19, 2023
@radical radical marked this pull request as ready for review October 19, 2023 22:12
@radical radical closed this Oct 19, 2023
@radical radical reopened this Oct 19, 2023
Copy link
Member

@jkoritzinsky jkoritzinsky left a comment

Choose a reason for hiding this comment

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

We should probably also disable these tests on the mobile platforms (all xharness platforms currently have the same infra limitations).

@radical
Copy link
Member Author

radical commented Oct 20, 2023

We should probably also disable these tests on the mobile platforms (all xharness platforms currently have the same infra limitations).

@steveisok @SamMonoRT do you know if these tests fail for ios, and other platforms? Look for Interop/PInvoke/Vector2_3_4/Vector2_3_4/** in src/tests/issues.targets, it is disabled for a few combinations.
If these tests are not failing on those platforms, then we can simply remove the exclusion. If they are failing, then we can add to the ActiveIssue for these.

This can be done in a follow up PR.

@lewing lewing merged commit 0621d07 into dotnet:main Oct 20, 2023
94 of 101 checks passed
@radical radical deleted the wasm-runtime-tests-exclude branch October 20, 2023 05:21
@ghost ghost locked as resolved and limited conversation to collaborators Nov 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants