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

Add [Fact] attributes to JIT regression tests #61540

Merged
merged 1 commit into from
Nov 14, 2021

Conversation

trylek
Copy link
Member

@trylek trylek commented Nov 13, 2021

For now there's no functional change to the behavior of the tests,
I have just copied the bits to inject from Jeremy's example in his
pending PR.

I have spot-checked that some of the tests use Main with the
command-line args argument. I'm not changing them in this PR, the
signature only becomes important once we start actually merging
the IL tests and I presume we'll clean that up at that point.

Thanks

Tomas

Contributes to: #54512

For now there's no functional change to the behavior of the tests,
I have just copied the bits to inject from Jeremy's example in his
pending PR.

I have spot-checked that some of the tests use Main with the
command-line args argument. I'm not changing them in this PR, the
signature only becomes important once we start actually merging
the IL tests and I presume we'll clean that up at that point.

Thanks

Tomas

Contributes to: dotnet#54512
@ghost
Copy link

ghost commented Nov 13, 2021

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

Issue Details

For now there's no functional change to the behavior of the tests,
I have just copied the bits to inject from Jeremy's example in his
pending PR.

I have spot-checked that some of the tests use Main with the
command-line args argument. I'm not changing them in this PR, the
signature only becomes important once we start actually merging
the IL tests and I presume we'll clean that up at that point.

Thanks

Tomas

Contributes to: #54512

Author: trylek
Assignees: -
Labels:

area-Infrastructure-coreclr

Milestone: -

Copy link
Member

@BruceForstall BruceForstall left a comment

Choose a reason for hiding this comment

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

I'm interested in how we prove (or convince ourselves) that the test runs after all this work do the same testing as what was done beforehand.

@trylek
Copy link
Member Author

trylek commented Nov 13, 2021

I think there are two aspects to this: the current PR's don't actually change anything, they just equip the IL tests with a dummy attribute no-one looks at. The second step will involve creating wrapper projects for merged IL tests; my expectation is that we'll be able to validate both the number of tests and the individual results; it's not the intent of this plan to silently sweep all tests under the rug.

@trylek
Copy link
Member Author

trylek commented Nov 14, 2021

I haven't found any bugs related to my change, merging in. It's unfortunate that the ARM queues are so unreliable these days but I believe they shouldn't hamper general infra innovation yielding potential Helix savings and improved developer experience.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants