Call PlatformDetection checks manually in IL tests

[tomeksowi]

Regression after #119878, these tests were entered with BuildAllTestsAsStandalone despite ConditionalFacts. See #112399 (comment) for context.

[dotnet-policy-service]

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

[jkotas]

I do not think we want to be inlining the checks like this into IL tests. We want to have a consistent approach to mark the tests with XUnit metadata.

What makes the IL tests different from C# tests in this regard? Can we fix the standalone mode to make that work instead of this?

cc @jkoritzinsky

[jkotas]

Ok, I see we have done that in #112399 . I think it is broken approach. I would like to see that PR reverted.

[jkotas]

Ok, I see we have done that in #112399 . I think it is broken approach. I would like to see that PR reverted.

It looks like #120992 reverted it, so there is a lot more tests that have this issue now.

[jkoritzinsky]

If possible, I'd rather we move each test out of the Main method and duplicate the conditional check in Main (for standalone) and the attribute (for the merged runner). It provides a better experience for our test runs.

[jkotas]

duplicate the conditional check in Main

Duplication will mean that the checks are mismatched all the time.

[jkoritzinsky]

Okay we can revert it. The guidance for writing IL tests should be updated to specify to never use the conditional attributes, at least for the cases where our partners that use BuildAllTestsAsStandalone work (Linux, no stress modes).

[tomeksowi]

It's unclear what is to be done for this PR. Remove ConditionalFactAttributes from all other IL tests and mention it in docs/workflow/testing/coreclr/testing.md?

[jkotas]

• We have been merging tests into fewer bigger assemblies to make building and running the tests more efficient. For example, Consolidate some jit64 tests into less assemblies #120992 deleted 455 test projects and merged them together into a single test assembly. There have been other similar changes before. It makes BuildAllTestsAsStandalone less relevant over time. I expect that we will hit a point where the IL-based tests are the top bottleneck and merge most of them too. Could you please elaborate why you need BuildAllTestsAsStandalone for your test runs?

• The metadata annotations give us more flexibility in how to orchestrate the test runs. I do not think we want to be inlining them into the test code. If there is a good reason for BuildAllTestsAsStandalone to do the filtering, I think we should figure out how to generate it from the metadata..

[jkoritzinsky]

We could hook something up with the ILTestRunner project to have it provide the Main method for each IL test, but only in BuildAllTestsAsStandalone. That way we could use the conditional attributes in IL and reuse our generator to provide the entry point with the basic filtering.

[tomeksowi]

• We have been merging tests into fewer bigger assemblies to make building and running the tests more efficient. For example Consolidate some jit64 tests into less assemblies #120992 deleted 455 test projects and merged them together into a single test assembly. There have been other similar changes before. It makes BuildAllTestsAsStandalone less relevant over time. I expect that we will hit a point where the IL-based tests are the top bottleneck and merge most of them too. Could you please elaborate why you need BuildAllTestsAsStandalone for your test runs?

OK, I didn't realize it was part of a larger work in progress. I was building with BuildAllTestsAsStandalone because it's easy to work on failing tests in isolation but it should be possible to change this habit and filter at build time.

cc @clamp03, I know you're using BuildAllTestsAsStandalone too