Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

trylek
Copy link
Member

@trylek trylek commented Jun 16, 2020

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
Copy link

ghost commented Jun 16, 2020

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

@jkotas
Copy link
Member

jkotas commented Jun 16, 2020

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
Copy link
Member Author

trylek commented Jun 16, 2020

@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
Copy link
Member

jkotas commented Jun 16, 2020

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

@trylek
Copy link
Member Author

trylek commented Jun 16, 2020

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

@jkotas
Copy link
Member

jkotas commented Jun 16, 2020

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
Copy link
Member Author

trylek commented Jun 16, 2020

@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
Copy link
Member

safern commented Jun 16, 2020

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.

Copy link
Contributor

@naricc naricc left a comment

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
Copy link
Member Author

trylek commented Jul 19, 2020

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