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

Deduplicate runtime tests using the class name 'Test' #61125

Merged
merged 2 commits into from
Nov 3, 2021

Conversation

trylek
Copy link
Member

@trylek trylek commented Nov 2, 2021

This change deduplicates test class names 'Test' by appending the identifier-sanitized filename of the source file containing the Main method for the test. The next group will be 553 occurrences of 'BringUpTest'.

Thanks

Tomas

Contributes to: #54512

@ghost
Copy link

ghost commented Nov 2, 2021

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

Issue Details

This change deduplicates test class names 'Test' by appending the identifier-sanitized filename of the source file containing the Main method for the test. The next group will be 553 occurrences of 'BringUpTest'.

Thanks

Tomas

Contributes to: #54512

Author: trylek
Assignees: -
Labels:

area-Infrastructure-coreclr

Milestone: -

@BruceForstall
Copy link
Member

BruceForstall commented Nov 2, 2021

Looks like Loader\classloader\DictionaryExpansion\DictionaryExpansion.cs depends on the test class name.

Can you make sure to run the outerloop test job on these big changes, where you need to run Pri-1 tests to validate?

@trylek
Copy link
Member Author

trylek commented Nov 2, 2021

@BruceForstall - as you can see, I'm running outerloop runs on each of these changes, I completely agree with you.

@BruceForstall
Copy link
Member

There's kind of an interesting implication of this work, that all tests need to have globally unique names. So when I create a new test, there will presumably be a build failure if I create one with a conflicting (e.g., simple) name. Hopefully that's not too annoying.

@trylek
Copy link
Member Author

trylek commented Nov 3, 2021

@BruceForstall - strictly speaking not all tests need to have unique names; our current thinking is that the grouping will be based on explicit markers in the tree and that implies that test class names need to be unique just within a single group. One way or another, the grouped build (that I expect to become the default, both locally and in the lab) should immediately identify an inadvertently introduced duplication when adding a new test and bail out where the error message should clearly indicate the problem.

@trylek
Copy link
Member Author

trylek commented Nov 3, 2021

Merging in with 1 known outerloop failure #60152.

@trylek trylek merged commit ab56392 into dotnet:main Nov 3, 2021
@trylek trylek deleted the DeduplicateClassName_Test branch November 3, 2021 15:35
@ghost ghost locked as resolved and limited conversation to collaborators Dec 3, 2021
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.

3 participants