Skip to content
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

Official test runs should use crossgened bits #964

Closed
danmoseley opened this issue Dec 10, 2018 · 12 comments
Closed

Official test runs should use crossgened bits #964

danmoseley opened this issue Dec 10, 2018 · 12 comments
Assignees
Labels
area-Infrastructure-libraries untriaged New issue has not been triaged by the area owner
Milestone

Comments

@danmoseley
Copy link
Member

Right now we do not test crossgen'ed bits in the Corefx official test runs. This is because the product is not crossgen'ed until the code reaches core-setup. This means we are not testing what the customer actually uses and are implicitly relying on higher level repos to do this testing.

It should be possible to crossgen in CoreFX official builds purely for the purpose of tests (we would throw the images away after).

cc @ViktorHofer @jkotas

@jkotas
Copy link
Member

jkotas commented Dec 10, 2018

It should be possible to crossgen in CoreFX official builds

I do not think it is the right way to address this hole. I think the right way to address this hole is to run the CoreFX tests against official binaries of the product (i.e. you take the zip produced by core-setup, unzip it - without changing a single bit in it, and run the tests against it).

I think the tests needs to be treated as separate package to pull this off, like what we are using the run the CoreFX tests in the default CoreCLR CI legs. Ideally, it would use the same package as what we use to run the CoreFX tests in CoreCLR CI.

@danmoseley
Copy link
Member Author

@jkotas that would mean a delay between a corefx commit and the test failure as the code has to be ingested into core-setup?

@danmoseley
Copy link
Member Author

Which is probably ok as we aren't looking at the results every day right now, but it would make root causing a harder : both the delay and loss of granularity

@jkotas
Copy link
Member

jkotas commented Dec 11, 2018

There is a high fidelity between the crossgened code and JITed code. It is the exact same JIT generating the code, just with a slightly different settings. Bugs that repro with crossgen, but do not repro with JIT are relatively rare. I think it is ok to depend on official builds to catch crossgen specific bugs.

@ViktorHofer
Copy link
Member

I agree, this should run as part of an official build.

@danmoseley
Copy link
Member Author

@jkotas my point was that right now, official runs include perhaps 1/3rd of a day's commits into CoreFX. If we test bits out of core-setup, we will get CoreFX commits in chunks of at least 1 day and possibly several or a week depending on whether the ingestion job got merged (this may improve with Prodcon of course). So testing with core-setup bits would mean runs repeatedly testing the same bits in some cases, and when there's a failure it will take longer to bisect.

If we cross-genned in CoreFX these would not be issues. Is it just a matter of running the crossgen tool on the binaries we built? That's not the identical bits from core-setup but it's way better than the IL we use today?

@danmoseley
Copy link
Member Author

cc @ericstj

@jkotas
Copy link
Member

jkotas commented Dec 12, 2018

I think we should snap to two key places to run tests:

  • CI test runs: Balance of coverage and cost. Not comprehensive.
  • Official build test runs: Comprehensive coverage. This should be testing the exact bits we ship.

We can have 3rd thing somewhere in between too. I would do it only once we have data that it is really needed and that it is not possible to address it by adjusting the CI coverage or other processes (like Prodcon flow).

@ericstj
Copy link
Member

ericstj commented Mar 27, 2019

What if we handled this by using the test archives that are now coming out of CoreFx official builds and ran those as part of the core-setup build?

@ViktorHofer
Copy link
Member

That's also an option. Where do we want tests to fail, in corefx or core-setup? Presumably in the first as that's the place where we will need to diagnose and fix issues.

Setting the 3.0 milestone again as we need at least a manual validation until we have some automation. We don't want to ship bits without first testing them.

@ViktorHofer
Copy link
Member

dotnet/arcade#2704 will do most of the heavy-lifting here and the remaining work will be to validate that build assets are cross-gend in Release mode.

@maryamariyan maryamariyan transferred this issue from dotnet/corefx Dec 16, 2019
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-Infrastructure-libraries untriaged New issue has not been triaged by the area owner labels Dec 16, 2019
@maryamariyan maryamariyan added this to the Future milestone Dec 16, 2019
@ViktorHofer
Copy link
Member

Closing in favor of #487

@ghost ghost locked as resolved and limited conversation to collaborators Dec 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Infrastructure-libraries untriaged New issue has not been triaged by the area owner
Projects
None yet
Development

No branches or pull requests

6 participants