Ignore merged test markers for tests without execution script

[trylek]

This delta should fix the remaining issues discussed on the PR thread

#85183 (comment)

by filtering out those MergedTestAssembly markers where the entire merged test wrappers got filtered out based on CLRTestTargetUnsupported or a similar property (a recently hit previously unseen case).

Thanks

Tomas

P.S. I'm going to be OOF on vacation Thursday thru Sunday in a recording studio session with my music band. I have validated this PR by putting the commit on top of the above Ivan's PR, please see

https://dev.azure.com/dnceng-public/public/_build/results?buildId=254082&view=results

I likely won't be able to merge this in before I sign off for the rest of the week. Please feel free to do anything with my change, either by merging it in assuming it works or by adding it as an extra commit on top of Ivan's change.

[ghost]

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

Issue Details

---

This delta should fix the remaining issues discussed on the PR thread

#85183 (comment)

by filtering out those MergedTestAssembly markers where the entire merged test wrappers got filtered out based on CLRTestTargetUnsupported or a similar property (a recently hit previously unseen case).

Thanks

Tomas

P.S. I'm going to be OOF on vacation Thursday thru Sunday in a recording studio session with my music band. I have validated this PR by putting the commit on top of the above Ivan's PR, please see

https://dev.azure.com/dnceng-public/public/_build/results?buildId=254082&view=results

I likely won't be able to merge this in before I sign off for the rest of the week. Please feel free to do anything with my change, either by merging it in assuming it works or by adding it as an extra commit on top of Ivan's change.

Author:	trylek
Assignees:	-
Labels:	area-Infrastructure-coreclr
Milestone:	-

[BruceForstall]

LGTM

[BruceForstall]

Hmmm.... @trylek I'm not sure it's working.

If I look at the runtime job for win-x86:

https://dev.azure.com/dnceng-public/public/_build/results?buildId=254109&view=logs&j=47409641-83ae-542e-21c9-457256fca231&t=3dfc904f-0ee1-5729-7296-9ffa7943f8dc

And look at all the related log files using this Kusto query:

Files
|  where JobName == "7402b93e-c474-4166-abe2-666bfe2b50aa"
 and FileName contains "console"
| project WorkItemFriendlyName, Uri, FileName

It shows HardwareIntrinsics_Arm_r and HardwareIntrinsics_Arm_ro jobs, and the console log for the former shows:

C:\h\w\A4A9096C\w\A32108EE\e>call JIT\HardwareIntrinsics\HardwareIntrinsics_Arm_r\HardwareIntrinsics_Arm_r.cmd -usewatcher 
'JIT\HardwareIntrinsics\HardwareIntrinsics_Arm_r\HardwareIntrinsics_Arm_r.cmd' is not recognized as an internal or external command,
operable program or batch file.

Looks like it's not working

[BruceForstall]

Maybe someone can explain to me: which step in the CI system creates the per-test wrapper scripts (e.g., HardwareIntrinsics_Arm_r.cmd above, for merged tests, and similarly for non-merged tests). I've never been able to figure out where they are created. Who invokes CLRTest.Execute.targets :: GenerateExecutionScript ?

[Edit] So it looks like the wrapper scripts are created during the test build process here:

runtime/src/tests/Directory.Build.targets

Lines 266 to 270 in 318186b

	<Import Project="$(MSBuildThisFileDirectory)Common\CLRTest.Execute.targets" />
	<Target Name="CreateExecuteScript"
	BeforeTargets="Build;CopyAllNativeProjectReferenceBinaries"
	Condition="'$(CLRTestBuildAllTargets)' != 'allTargets' And '$(GenerateRunScript)' != 'false' And ('$(_WillCLRTestProjectBuild)' == 'true')"
	DependsOnTargets="GenerateExecutionScriptsInternal" />

but NOT if '$(CLRTestBuildAllTargets)' is 'allTargets', which it is in the CI build of the tests:

https://dev.azure.com/dnceng-public/public/_build/results?buildId=254109&view=logs&j=c92f2c34-43c3-5f9c-356c-2e863ce9eb4e&t=1bb8c7f3-fff6-5069-c97d-796b6a1413a0

[Edit 2] By experimentation, it appears to be the src/tests/build.sh buildtestwrappersonly step. It would sure be nice if the msbuild output included every generated wrapper script that was generated, for clarity.

[ivdiazsa]

Maybe someone can explain to me: which step in the CI system creates the per-test wrapper scripts (e.g., HardwareIntrinsics_Arm_r.cmd above, for merged tests, and similarly for non-merged tests). I've never been able to figure out where they are created. Who invokes CLRTest.Execute.targets :: GenerateExecutionScript ?

[Edit] So it looks like the wrapper scripts are created during the test build process here:

runtime/src/tests/Directory.Build.targets

Lines 266 to 270 in 318186b

	<Import Project="$(MSBuildThisFileDirectory)Common\CLRTest.Execute.targets" />
	<Target Name="CreateExecuteScript"
	BeforeTargets="Build;CopyAllNativeProjectReferenceBinaries"
	Condition="'$(CLRTestBuildAllTargets)' != 'allTargets' And '$(GenerateRunScript)' != 'false' And ('$(_WillCLRTestProjectBuild)' == 'true')"
	DependsOnTargets="GenerateExecutionScriptsInternal" />

but NOT if '$(CLRTestBuildAllTargets)' is 'allTargets', which it is in the CI build of the tests:

https://dev.azure.com/dnceng-public/public/_build/results?buildId=254109&view=logs&j=c92f2c34-43c3-5f9c-356c-2e863ce9eb4e&t=1bb8c7f3-fff6-5069-c97d-796b6a1413a0

[Edit 2] By experimentation, it appears to be the src/tests/build.sh buildtestwrappersonly step. It would sure be nice if the msbuild output included every generated wrapper script that was generated, for clarity.

Agree with the fact that MSBuild output should be clearer. But I'm a bit confused about your overall review Bruce, since you dismissed a "stale" comment. Do you mean the problem is still happening in the CI even with Tomas' changes?

[BruceForstall]

Do you mean the problem is still happening in the CI even with Tomas' changes?

yes

[ivdiazsa]

Do you mean the problem is still happening in the CI even with Tomas' changes?

yes

Ok I will take a look then. Thanks for finding that.

[BruceForstall]

I've been looking at this. I think a simple change might fix it: #85476

[tannergooding]

@BruceForstall, this can be closed in favor of your PR, right?

[BruceForstall]

@BruceForstall, this can be closed in favor of your PR, right?

yes