-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Use the "live" NETCoreApp shared framework in CoreCLR and Libraries tests #487
Comments
Alluding to Davis' comment - CoreCLR way of crossgenning the framework is totally hacky too. My initial strawman was to switch this over to the SuperIlc tool I originally wrote for the purpose of local Crossgen2 testing - as one of its side effects it's capable of crossgenning framework (using both the legacy and new Crossgen) employing parallelization so it's quite a bit faster. As an additional bonus it would give us at least some code coverage for the tool too. |
I agree that it would make sense to use the installer sharedfx generation infra to produce the bits that the tests runs against (e.g. in CI).
The tight dev loop should not be going through any of the crossgening, etc.
SuperIlc is a test harness. I do not think it makes sense to use it to build the product. |
(Issues should be marked with a single area only: https://github.com/dotnet/runtime/blob/master/docs/issues-pr-management.md#scenarios-where-we-all-have-to-work-together) |
Well, OK, I think that my point is - on the one hand we have super hacky code in runtime/src/coreclr/build-test.cmd Line 653 in a76c402
or another copy in runtime/src/coreclr/build-test.sh Line 145 in a76c402
or another copy in the installer pipeline. What would be your preferred way to get rid of this wasteful redundancy? [Sorry about the issue tags.] |
Have just the one in the installer pipeline. The installer pipeline has to have it obviously. And switch the tests to run against the bits produced by the installer pipeline. We have a test hole today in that we do not actually run tests against the bits that we ship (https://github.com/dotnet/corefx/issues/33958). Switching the tests to run against the bits produced by the installer pipeline (crossgened, signed, etc.) would fix that. |
Something to consider here is to run a set of libraries tests in the single PR pipeline when the installer subset is changed. Look at discussion: #1473 (comment) |
Closing as done. |
Reopening as I misread the title. |
When ever we get to this we should make use of the shared framework's SDK functionality to dump the bundle layout onto disk and use that for both the runtime and libraries. Moving to 7.0. |
* Add integration tests for the MacCatalyst target * Make Helix tests run on local * Remove EnableXUnitReporter * Create template proj for integration tests
Now that the repo is consolidated, we can look at ways to reduce some redundant infrastructure around the shared framework.
Before consolidation, Core-Setup was downstream from CoreCLR and CoreFX. This means the crossgenned NETCoreApp shared framework produced by Core-Setup doesn't go through CoreCLR and CoreFX's tests.
CoreCLR tests do their own crossgen to set up a R2R testing infrastructure. I think CoreFX does something similar, but I'm not sure if it crossgens.
With consolidation, we could have CoreCLR and Libraries tests use the R2R shared framework prepared by Installer. This might save build time, and seems like it will remove some redundant infra so we don't have to maintain it.
The current Installer sharedfx generation infra might not be clean enough yet for this to work in a repo that has much higher dev workflow standards (IMO). I have concerns around incrementality and efficiency in a tight dev loop. I'm not active on that part of the infra quite enough to pinpoint the problems offhand, but I want to make sure we consider it before going all-in.
(I thought I remembered idea/plan in an issue or a roadmap, but I can't find it. Filing to make sure it's tracked regardless.)
/cc @trylek @stephentoub
The text was updated successfully, but these errors were encountered: