July infra rollout: Move runtime tests out of the coreclr folder (2nd attempt) #38058
Conversation
|
Tagging subscribers to this area: @ViktorHofer |
|
@trylek does this include the runtimetests subset update in |
|
@safern - not yet, working on that right now to make this a third PR feedback commit. |
trylek
force-pushed
the
CoreCLRTestMoveInMaster
branch
from
0c847f3 to
db91329
Compare
|
@safern - hopefully fixed, can you please take another look when you have a chance? |
|
You'll also need to update the conditions to some jobs so that they run when the runtime tests are changed to include new condition in their or something like: eq(dependencies.checkout.outputs['SetPathVars_runtimetests.containsChange'], true)in the following lines |
trylek
force-pushed
the
CoreCLRTestMoveInMaster
branch
from
02fa61e to
b028598
Compare
trylek
force-pushed
the
CoreCLRTestMoveInMaster
branch
4 times, most recently
from
e4a3983 to
e2677c9
Compare
|
@dotnet/runtime-infrastructure - I believe I'm finally out of the woods w.r.t. the weird difference between PR and CI runs of the change due to an omission in build-test.sh (not passing unprocessed build args to msbuild). I have triggered a CoreCLR outerloop run and I intend to coordinate tomorrow European morning with @ViktorHofer on finally merging the change in unless I hear objections - I would love to avoid further delays if possible, I hit two conflicts just this afternoon during lab testing, the change is vast and the test churn makes it really painful. Thanks a lot Tomas |
| @@ -598,6 +602,7 @@ jobs: | |||
| eq(dependencies.checkout.outputs['SetPathVars_libraries.containsChange'], true), | |||
| eq(dependencies.checkout.outputs['SetPathVars_coreclr.containsChange'], true), | |||
| eq(dependencies.checkout.outputs['SetPathVars_mono.containsChange'], true), | |||
| eq(dependencies.checkout.outputs['SetPathVars_runtimetests.containsChange'], true), | |||
There was a problem hiding this comment.
Just double checking to refresh my memory, libraries tests are needed for runtime tests, right?
There was a problem hiding this comment.
I don't think we need any library test build artifacts for building or running runtime tests, just the libraries build. I can easily remove this line if you believe it's superfluous, I guess I'll probably catch a few more conflicts before the night is over, I just hope that around PST midnight the churn will calm down a bit. I'm not going to update the PR right away as that would cancel the running tests and that would be a waste of lab resources considering this is ultimately just a "perf optimization".
There was a problem hiding this comment.
I think we can update this as a subsequent PR to not risk getting more merge conflicts.
|
One more comment - I'm planning to merge this in without squashing i.e. as two commits, one doing the mechanical move and another with the script patches, for easier orientation. Please let me know if for whatever reason you find a full squash merge preferable. |
(*) Path changes in the test build scripts; (*) Modify runtime.yml filtering based on Nathan's and Santi's feedback; (*) Fix runtime pipeline filtering clauses per Santi's PR feedback; (*) Fix path to Coreclr.TestWrapper.csproj; (*) Pass unprocessed build args to test wrapper creation; (*) Fix missing $(TestRoot) on groups in Pri0 test build mode. Thanks Tomas
trylek
force-pushed
the
CoreCLRTestMoveInMaster
branch
from
e2677c9 to
b64d1b4
Compare
ghost
locked as resolved and limited conversation to collaborators
I have messed up my previous version of this change as of #37995 while rebasing the change against current master. The current expectation is that the change is going to be merged in around 7/6 i.e. at the July infra rollout, fixing the bug #37440.
Thanks
Tomas
Fixes: #37440
/cc: @dotnet/runtime-infrastructure