Runtime: July infra rollout: move tests shared between coreclr and mono from src/coreclr/tests/src to src/tests

Created on 4 Jun 2020  路  13Comments  路  Source: dotnet/runtime

As what was previously the "CoreCLR test suite" is now shared with mono, it no longer makes sense for it to sit under src/coreclr/tests in the runtime tree. The purpose of this task is to move the tests into a new common folder under the tree and modify the paths in all scripts and pipelines so that they continue running after the change. This task doesn't address any changes to the underlying harness used to run the tests.

Current root location of CoreCLR test source code and scripts:

src/coreclr/tests/src

Proposed new location for the test root:

src/tests

/cc: @dotnet/runtime-infrastructure

area-Infrastructure-coreclr

Most helpful comment

Are you in favor of tests/runtime or something under src like src/tests/runtime?

My thoughts:

  • For better or worse, we have the extra src level at the root and all sources under it. We should be consistent with it.

    • If we believe that this extra src level is not useful (e.g. it looks odd to have two src in the path like src\libraries\System.AppContext\src), we should get rid of it from everywhere and have e.g. libraries as the top level directory.

  • What other directories would we expect to have under tests? I think it is hard to tell where the "runtime" ends and the "other" stuff starts. In fact, the whole repo is the runtime. It may be better to make this a general purpose directory for all non-API tests, and have area-specific subdirectories under it. I think we can agree on what are the GC-specific or interop-specific tests.

So my pick would be src/tests with area-specific subdirectories like src/tests/Interop, src/tests/GC, etc.

Separately, it would be great to make it easier to write XUnit-like tests under this sub-tree. There is a hand-rolled xunit runner that is used to author tests like https://github.com/dotnet/runtime/blob/master/src/coreclr/tests/src/Interop/ICustomMarshaler/Primitives/ICustomMarshaler.cs#L46 . It works, but I think there are opportunities to make it work better.

All 13 comments

Is this just going to involve a lot of updating paths? Or are there more substantive changes involved?

@jkotas what's your stance on the new path? Are you in favor of tests/runtime or something under src like src/tests/runtime?

@naricc - I believe we should just "update the paths" in this one step and tackle any deeper semantic changes regarding test execution later. I think that even in its limited extent this will be a bit of fun :-).

Are you in favor of tests/runtime or something under src like src/tests/runtime?

My thoughts:

  • For better or worse, we have the extra src level at the root and all sources under it. We should be consistent with it.

    • If we believe that this extra src level is not useful (e.g. it looks odd to have two src in the path like src\libraries\System.AppContext\src), we should get rid of it from everywhere and have e.g. libraries as the top level directory.

  • What other directories would we expect to have under tests? I think it is hard to tell where the "runtime" ends and the "other" stuff starts. In fact, the whole repo is the runtime. It may be better to make this a general purpose directory for all non-API tests, and have area-specific subdirectories under it. I think we can agree on what are the GC-specific or interop-specific tests.

So my pick would be src/tests with area-specific subdirectories like src/tests/Interop, src/tests/GC, etc.

Separately, it would be great to make it easier to write XUnit-like tests under this sub-tree. There is a hand-rolled xunit runner that is used to author tests like https://github.com/dotnet/runtime/blob/master/src/coreclr/tests/src/Interop/ICustomMarshaler/Primitives/ICustomMarshaler.cs#L46 . It works, but I think there are opportunities to make it work better.

It feels a bit confusing to me that a src/tests folder wouldn't contain _all_ the tests including libraries tests, maybe something like src/runtime-tests is better?

Or at the very least explain in the src/tests/README.md that it doesn't include libraries tests and where to find them.

src/tests/README.md is certainly a good idea.

I do not think runtime-tests helps with clarity. nonapi-tests or nonlibraries-tests would be better, but it is a mouthful, so I would rather go with just tests.

Thanks everyone for your initial feedback. I like @jkotas' counterproposal to my original suggestion and I'm completely fine going with it. According to my understanding of his suggestion, we should basically map today subfolders of src/coreclr/tests/src under src/tests, e.g. src/coreclr/tests/src/JIT will become src/tests/JIT.

As a first cut we can just move the test source code and keep the scripts under src/coreclr/tests/src and src/coreclr/tests/src/Common in their current locations; we can iterate on their relocation and / or refactoring independently after the tests themselves have been moved.

For now I treat this as the champion proposal. If anyone believes they have a more reasonable suggestion, feel free to chime in and we can do some voting via emojis :-).

As a slight update I'm starting to think that it would be useful to have another intermediate folder level under src/tests, something like src/tests/runtime, the rationale being that CoreCLR test processing is based on scanning a folder subtree; once we start adding e.g. installer tests to the folder, it's going to be much easier to filter out those tests that are supposed to be run via the CoreCLR / Mono "runtime test harness" no matter how the meaning of this term is going to evolve.

I do not see a fundamental difference between e.g. interop tests and installer tests. Why can't all be discovered using the same folder scanning?

Well, at the very least they use different test harnesses today. I'm not against unifying them over time but doing all that in one fell swoop is beyond my imagination logistically. I may be mistaken but it also seems to me that the audience and purposes of runtime vs. library vs. installer tests are often different, making these test groups worth being observable by any runtime contributor.

~Installer~ Host is a well defined narrow area. Having src/tests/HostActivation or src/tests/HostModel would be perfectly fine with me if we wanted to move the current installer tests there.

Runtime is poorly defined broad area. It is why I dislike src/tests/runtime.

The CoreCLR tests have very few requirements. It is just an entrypoint .exe and error code that it is expected to return, the rest is up to you. You can fit any test into this. The feature specific tests often customize it today. For example, compare these feature area specific tests:

Each of these have their own feature-specific test harness and shape of tests that is not one like other. Installer tests can be on this plan too if we choose to move them here.

@trylek You might want to update the initial message in this issue to be up-to-date with the current plan.

Was this page helpful?
0 / 5 - 0 ratings