Skip to content

Move runtime tests out of the coreclr folder (dev/infrastructure branch)#37977

Closed
trylek wants to merge 3 commits into
dotnet:dev/infrastructurefrom
trylek:dev/trylek/CoreCLRTestMoveV2
Closed

Move runtime tests out of the coreclr folder (dev/infrastructure branch)#37977
trylek wants to merge 3 commits into
dotnet:dev/infrastructurefrom
trylek:dev/trylek/CoreCLRTestMoveV2

Conversation

@trylek

@trylek trylek commented Jun 16, 2020

Copy link
Copy Markdown
Member

This is the initial bulky change aimed towards cleanup and better
convergence of runtime tests between the CoreCLR and Mono
runtimes. In this change I have moved all test source code and
build scripts from

src/coreclr/tests/src

to

src/tests

and I made what I believe to be the minimum number of additional
script changes to make the tests build and run end to end on
Windows and Linux.

Thanks

Tomas

/cc: @dotnet/runtime-infrastructure

@ghost

ghost commented Jun 16, 2020

Copy link
Copy Markdown

Tagging subscribers to this area: @ViktorHofer
Notify danmosemsft if you want to be subscribed.

@jkotas

jkotas commented Jun 16, 2020

Copy link
Copy Markdown
Member

Could you please rebase this against master and make sure that all tests are still moved correctly? There were test changes made in the meantime that the change is not picking up.

@trylek

trylek commented Jun 16, 2020

Copy link
Copy Markdown
Member Author

@jkotas - Thanks for pointing that out. I have already sent out the PR for a new FI from master to dev/infrastructure,

#37982

I plan to rebase the change and trigger a new round of testing exactly as you suggest once that's merged in.

@jkotas

jkotas commented Jun 16, 2020

Copy link
Copy Markdown
Member

Ok, I have not noticed that this is against dev/infrastructure. You will be on a treadmill until this gets to master.

@trylek

trylek commented Jun 16, 2020

Copy link
Copy Markdown
Member Author

I know but I'm afraid this is exactly the kind of change the branch is meant for :-).

@jkotas

jkotas commented Jun 16, 2020

Copy link
Copy Markdown
Member

I know but I'm afraid this is exactly the kind of change the branch is meant for :-).

I am not sure about that. This is very straightforward change (60 line delta + directory rename). It is going to have some fallout. There will be a few places that you will miss no matter how hard you try.

It is the kind of change we want to save for the infrastructure rollout (#37706), but I think it is fine for it to go directly to master.

@trylek

trylek commented Jun 16, 2020

Copy link
Copy Markdown
Member Author

@jkotas - Thanks for your feedback, that sounds reasonable to me; after all, while the change is bulky, I have tried to make it minimal in the sense that all the outside-facing scripts remain in their preexisting locations so that right now the change doesn't require any pipeline changes or changes to local developer inner loop. I'll send out a rebased PR for this change against master and I'll merge it in tomorrow assuming it's been approved and there's no pushback from the @dotnet/runtime-infrastructure team.

@safern

safern commented Jun 16, 2020

Copy link
Copy Markdown
Member

I'll send out a rebased PR for this change against master and I'll merge it in tomorrow assuming it's been approved and there's no pushback from the @dotnet/runtime-infrastructure team.

I think we would need to hold until the next infra rollout for this change to be merged.

@naricc naricc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to update the runtimetests subset in runtime.yml, since it refers to the test path: https://github.com/trylek/runtime/blob/6ca9f624c774b613d96d29e786881b1cd772f5ff/eng/pipelines/runtime.yml#L112

We also probably want to change it so the coreclr tests are run if the runtimetest path is updated.

@trylek trylek changed the title Move runtime tests out of the coreclr folder Move runtime tests out of the coreclr folder (dev/infrastructure branch) Jun 17, 2020
@trylek

trylek commented Jul 19, 2020

Copy link
Copy Markdown
Member Author

Merged into master on 7/7, closing.

@trylek trylek closed this Jul 19, 2020
@trylek trylek deleted the dev/trylek/CoreCLRTestMoveV2 branch September 10, 2020 21:27
@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2020
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.

4 participants