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

Remove CoreCLR TargetSpecific test build jobs #35645

Closed
wants to merge 6 commits into from

Conversation

sdmaclea
Copy link
Contributor

Refactor targetSpecific test build
Remove targetSpecific build-test-jobs.yml jobs
Build targetSpecific tests in run-test-job.yml jobs

@sdmaclea sdmaclea requested review from trylek and safern April 30, 2020 01:46
@sdmaclea sdmaclea self-assigned this Apr 30, 2020
@sdmaclea sdmaclea marked this pull request as draft April 30, 2020 01:47
Refactor targetSpecific test build
Remove targetSpecific build-test-jobs.yml jobs
Build targetSpecific tests in run-test-job.yml jobs
# Build targetSpecific managed test components
- script: $(coreClrRepoRootDir)build-test$(scriptExt) targetSpecific skipnative skipgeneratelayout skiptestwrappers $(buildConfig) $(archType) $(crossArg) $(priorityArg) ci $(librariesOverrideArg)
displayName: Build managed test components

Copy link
Contributor Author

@sdmaclea sdmaclea Apr 30, 2020

Choose a reason for hiding this comment

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

The earliest draft of this was also adding an archive of the tests.
I was fiddling with two variants.

  1. Archive all tests
  2. Archive target specific tests built here.

I thought that if performance was really critical, no artifact would be fastest (and most reliable).

However having the test artifact would help with debugging. Having all tests in the archive is easier and probably less risky as the script ordering can stay as is.

Trying to archive only the target specific tests is harder and less performant.

  • Basic target specific test archive approach:
    • Alter the build order so that the target specific tests are built and archived before downloading generic tests.
    • Risks -- In itself that is safe, but it does run the risk that a generic test overwrites a specific test.
  • Move copy target specific test archive approach:
    • Download generic tests to a test staging directory
    • Build the specific tests in a test build directory
    • Archive the specific tests
    • Copy the specific tests onto the generic tests in the test staging directory
    • Run the helix jobs from the test staging directory
    • Risks -- More moving pieces

@sdmaclea
Copy link
Contributor Author

One potential other problem with this approach is when we run multiple jobs on the same tests. R2R & !R2R & JitStress* &&&&....

Every job would be building the target specific tests.

@sdmaclea
Copy link
Contributor Author

sdmaclea commented Apr 30, 2020

Artifact MicrosoftNetSdkIlPackage_*_checked not found for build

I guess the target specific build was generating this too...

Fixed Now using the locally generated one.

@sdmaclea
Copy link
Contributor Author

sdmaclea commented May 1, 2020

This seems to be working now.
I am going to push a change to the display names, but this could use at least a preliminary review.

@sdmaclea sdmaclea marked this pull request as ready for review May 1, 2020 01:05
@hoyosjs
Copy link
Member

hoyosjs commented May 1, 2020

The other disadvantage here is retrying a test run will cause a rebuild too. Not sure if this matters too much.

@safern
Copy link
Member

safern commented May 1, 2020

The other disadvantage here is retrying a test run will cause a rebuild too. Not sure if this matters too much.

While that is true, I think this will bring faster builds, less legs meaning less prone to infra errors, and less times hitting the network to download artifacts, etc. So as long as the tests are reliable enough, we shouldn't be specializing for retry.

@sdmaclea
Copy link
Contributor Author

sdmaclea commented May 1, 2020

There are 5 readytorun tests which are not happy with this change. I am assuming it is because the tests were crossgen2 for x64.

I assume therefore, these tests need to be marked ArchSpecific

Copy link
Member

@trylek trylek left a comment

Choose a reason for hiding this comment

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

Looks great, thank you for all the improvements you're making to our pipelines!

eng/pipelines/common/templates/runtimes/run-test-job.yml Outdated Show resolved Hide resolved
sdmaclea added 2 commits May 1, 2020 15:24
These tests are failing in PR runs when built on OSX and
run on specific platforms
@sdmaclea
Copy link
Contributor Author

sdmaclea commented May 1, 2020

I marked the 5 tests failing in p0 target specific..

Running an outerloop p1 run here https://dev.azure.com/dnceng/public/_build/results?buildId=627124&view=results

I am expecting to have to mark some p1 tests too.

@sdmaclea
Copy link
Contributor Author

sdmaclea commented May 5, 2020

Closing in favor of #35783

@sdmaclea sdmaclea closed this May 5, 2020
@sdmaclea sdmaclea deleted the runBuildsTests branch September 26, 2020 16:52
@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 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