Add basic check that the correct number of tests is built

[BruceForstall]

Fixes #19286

[BruceForstall]

@dotnet-bot test Windows_NT x64 Build and Test
@dotnet-bot test Windows_NT x86 Checked Build and Test

[BruceForstall]

@dotnet-bot test Windows_NT x86 Checked Build and Test

[BruceForstall]

@dotnet-bot test OSX10.12 x64 Checked CoreFX Tests

[jashook]

I understand the need for this type of check, it is something that was manually required to verify before #18695 was merged. As it is I suggest changes to this PR, either to add this information in one place in runtest.proj (as awful as that sounds it at least unifies it), or duplicate the logic in build-test.sh.

Third option is to wait for a build-test unification similar to runtest unification done in #19213, and have a unified script to do these post-build checks.

[jashook]

See above for opinions on changes

[BruceForstall]

@dotnet-bot test Windows_NT x64 Build and Test
@dotnet-bot test Windows_NT x86 Checked Build and Test

[BruceForstall]

@jashook How's this look?

[jashook]

Just needs the call from build-test.sh as well thanks!

[BruceForstall]

@dotnet-bot test Ubuntu x64 Build and Test
@dotnet-bot test Windows_NT x64 Build and Test
@dotnet-bot test Windows_NT x86 Checked Build and Test

[AaronRobinsonMSFT]

The property shouldn't be Priority, it should be CLRTestPriorityToBuild. This mapping is defined in coreclr/config.json <- this appears to be passed to run.exe.

[AaronRobinsonMSFT]

Ugh.. Sorry, you are using Priority in your target.... instead, can you please use the standard property for tests: CLRTestPriorityToBuild

[AaronRobinsonMSFT]

BTW that is the core issue for why the previous PR was failing....

[BruceForstall]

@AaronRobinsonMSFT I'm not invoking run.exe, I'm using msbuild directly.

[AaronRobinsonMSFT]

I realize that post statement. My point is we have too many properties/variables that hold priority. We already have one in MSBuild (i.e. CLRTestPriorityToBuild) so we should reuse that one instead of the generic Priority.

[BruceForstall]

@dotnet-bot test Ubuntu x64 Build and Test
@dotnet-bot test Windows_NT x64 Build and Test
@dotnet-bot test Windows_NT x86 Checked Build and Test

[AaronRobinsonMSFT]

@BruceForstall I am generally against these kind of checks. This is the exact kind of code that atrophies within a week and then inevitably gets abused and confuses future team members, however I can appreciate the concern about loss of coverage - a real issue as demonstrated by the recent issue. What is the long term maintenance approach for this code? At the very least it should be documented in one of the test markdown files so people know this mechanism exists and should be considered when working with tests in coreclr.

[BruceForstall]

@dotnet-bot test Ubuntu x64 Build and Test
@dotnet-bot test Windows_NT x64 Build and Test
@dotnet-bot test Windows_NT x86 Checked Build and Test

[BruceForstall]

@BruceForstall I am generally against these kind of checks. This is the exact kind of code that atrophies within a week and then inevitably gets abused and confuses future team members, however I can appreciate the concern about loss of coverage - a real issue as demonstrated by the recent issue. What is the long term maintenance approach for this code? At the very least it should be documented in one of the test markdown files so people know this mechanism exists and should be considered when working with tests in coreclr.

@AaronRobinsonMSFT I don't see how this code atrophies: if it fails, it will be a noisy failure. I don't see this as a maintenance issue: if it fails, because someone maybe added or removed a lot of tests, they'll see it. I've added a bunch of comments to the msbuild target. Note that the coverage lost last week was by no means the first time we've had this problem.

[AaronRobinsonMSFT]

@BruceForstall This code atrophies because when more tests are added and the the count becomes 20000, when 8000 go missing it is again an issue. I am not saying there is no value, but any hard-coded value that represents a point in time quantity by definition atrophies if the quantity can change over time.

Adding comments in MSBuild isn't where people look for how to integrate into the system, it is where they look when something breaks. Knowing ahead of time that this mechanism exists is helpful so if someone adds many new tests or removes many, there is a heads up in the process document. Basically, this is exactly the kind of thing that should be mentioned in the documentation for working with tests in coreclr.

[Maoni0]

I just got this:

BUILDTEST: Check the managed tests build
D:\coreclr\tests\runtest.proj(75,5): error : Unexpected test count. Expected < 3000, found 7242.'

D:\coreclr\tests\runtest.proj(75,5): error : Unexpected test count. Expected < 3000, found 7242.'
BUILDTEST: Error: build failed.