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] Make windows wasm build run tests on helix #51951

Merged
merged 12 commits into from
May 12, 2021
Merged

[wasm] Make windows wasm build run tests on helix #51951

merged 12 commits into from
May 12, 2021

Conversation

radekdoulik
Copy link
Member

@radekdoulik radekdoulik commented Apr 27, 2021 •
edited
Loading

Some of the tests were disabled for now: #52138

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@radekdoulik radekdoulik added area-Build-mono arch-wasm WebAssembly architecture labels Apr 27, 2021
@ghost
Copy link

ghost commented Apr 27, 2021

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

Issue Details
Author: radekdoulik
Assignees: -
Labels:

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

@radekdoulik radekdoulik changed the title Make windows wasm build run tests on helix [wasm] Make windows wasm build run tests on helix Apr 28, 2021
@radekdoulik
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Pull request contains merge conflicts.

@radekdoulik radekdoulik mentioned this pull request Apr 30, 2021
Closed
lewing
lewing reviewed May 8, 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.

Is there a reason you're excluding whole test assemblies instead of marking individual tests with ActiveIssue?

@radekdoulik
Copy link
Member Author

Is there a reason you're excluding whole test assemblies instead of marking individual tests with ActiveIssue?

My motivation was to make this PR merged quickly, so that we can use the infrastructure changes for PR to enable few AOT tests on windows helix. So did similar change we do for failing AOT tests. In some libraries there are many tests failing. I can annotate these which have only few tests failing if you think it is worth.

@radekdoulik radekdoulik marked this pull request as ready for review May 11, 2021 07:35
@radekdoulik radekdoulik requested a review from radical May 11, 2021 07:35
@pavelsavara pavelsavara mentioned this pull request May 11, 2021
Closed
3 tasks
lewing
lewing reviewed May 11, 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.

Looks good to me. @steveisok @akoeplinger anything we need to worry about here?

@steveisok
Copy link
Member

Looks good to me. @steveisok @akoeplinger anything we need to worry about here?

Not that I can see. Looks good.

radical
radical requested changes May 11, 2021
View reviewed changes
Copy link
Member

@radical radical left a comment

Choose a reason for hiding this comment

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

Just a couple of comments.

@radekdoulik
Disable the tests only on windows/helix
b4dda6f 
Also make the new property groups conditional to improve perf
radical
radical approved these changes May 11, 2021
View reviewed changes
Copy link
Member

@radical radical left a comment

Choose a reason for hiding this comment

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

LGTM! Excited to have these enabled 👍

@radical
Copy link
Member

radical commented May 11, 2021

Builds failing due to #52596 .

@radekdoulik radekdoulik merged commit 1dc5010 into dotnet:main May 12, 2021
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
@radical radical mentioned this pull request Jun 8, 2021
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Jun 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lewing lewing lewing left review comments

@radical radical radical approved these changes

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.
5 participants
@radekdoulik @steveisok @radical @lewing @karelz