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

Simplify runtime test projects #60724

Merged
merged 2 commits into from
Oct 22, 2021

Conversation

BruceForstall
Copy link
Member

Don't specify defaults in the test project files, to simplify them:

  1. CLRTestPriority of zero is the default, so don't specify that: commit 3d37868
  2. CLRTestKind of BuildAndRun is the default, so don't specify that: commit 799129b

@BruceForstall
Copy link
Member Author

/azp run runtime-coreclr outerloop

@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.

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ghost
Copy link

ghost commented Oct 21, 2021

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

Issue Details

Don't specify defaults in the test project files, to simplify them:

  1. CLRTestPriority of zero is the default, so don't specify that: commit 3d37868
  2. CLRTestKind of BuildAndRun is the default, so don't specify that: commit 799129b
Author: BruceForstall
Assignees: -
Labels:

area-Infrastructure-coreclr

Milestone: -

@BruceForstall
Copy link
Member Author

The only test failure is #60152; others are infra failures.

The total number of outerloop tests remained the same (465,858).

@BruceForstall
Copy link
Member Author

@dotnet/runtime-infrastructure PTAL.

Comments?

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.

I love the simplification!

@akoeplinger
Copy link
Member

Nice! I assume we could simplify them even further by moving OutputType and AllowUnsafeBlocks to a common Directory.Build.props too?

@BruceForstall
Copy link
Member Author

Nice! I assume we could simplify them even further by moving OutputType and AllowUnsafeBlocks to a common Directory.Build.props too?

Yeah, maybe it would make sense to make OutputType Exe the default and set that in src\tests\Directory.Build.targets along with the CLRTestKind/CLRTestPriority defaults.

AllowUnsafeBlocks is only set when needed, so I don't think that needs any cleanup.

@BruceForstall BruceForstall merged commit 4b0be56 into dotnet:main Oct 22, 2021
@BruceForstall BruceForstall deleted the RemoveDefaultTestPriority branch October 22, 2021 15:08
@ghost ghost locked as resolved and limited conversation to collaborators Nov 21, 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