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.

Sign up for GitHub

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
Closed

Remove CoreCLR TargetSpecific test build jobs #35645

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f5e1a72
Remove CoreCLR TargetSpecific test build jobs
sdmaclea Apr 29, 2020
fe28ce5
Fix Microsoft.NET.Sdk.IL package download
sdmaclea Apr 30, 2020
5b8218a
Adjust display names
sdmaclea May 1, 2020
4b04039
Make 5 tests target specific
sdmaclea May 1, 2020
0b77968
Feedback
sdmaclea May 1, 2020
a1e394a
Fix disable of 5 tests
sdmaclea May 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 31 additions & 11 deletions eng/pipelines/common/templates/runtimes/run-test-job.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,11 @@ jobs:

dependsOn:
- ${{ if ne(parameters.corefxTests, true) }}:
- ${{ if eq(parameters.testGroup, 'innerloop') }}:
- '${{ parameters.runtimeFlavor }}_common_test_build_p0_${{ parameters.managedTestBuildOsGroup }}${{ parameters.managedTestBuildOsSubgroup }}_${{ parameters.archType }}_${{parameters.buildConfig }}'
- ${{ if ne(parameters.testGroup, 'innerloop') }}:
- '${{ parameters.runtimeFlavor }}_common_test_build_p1_${{ parameters.managedTestBuildOsGroup }}${{ parameters.managedTestBuildOsSubgroup }}_${{ parameters.archType }}_${{parameters.buildConfig }}'
- ${{ if ne(parameters.testBuildPhased, true) }}:
- ${{ if eq(parameters.testGroup, 'innerloop') }}:
- '${{ parameters.runtimeFlavor }}_common_test_build_p0_${{ parameters.managedTestBuildOsGroup }}${{ parameters.managedTestBuildOsSubgroup }}_${{ parameters.archType }}_${{parameters.buildConfig }}'
- ${{ if ne(parameters.testGroup, 'innerloop') }}:
- '${{ parameters.runtimeFlavor }}_common_test_build_p1_${{ parameters.managedTestBuildOsGroup }}${{ parameters.managedTestBuildOsSubgroup }}_${{ parameters.archType }}_${{parameters.buildConfig }}'
- ${{ if eq(parameters.testBuildPhased, true) }}:
# Also depend on AnyOS AnyCPU tests
- ${{ if eq(parameters.testGroup, 'innerloop') }}:
Expand Down Expand Up @@ -209,13 +210,14 @@ jobs:
artifactName: '$(managedGenericTestArtifactName)'
displayName: 'generic managed test artifacts'

# Download and unzip target specific tests
- template: /eng/pipelines/common/download-artifact-step.yml
parameters:
unpackFolder: '$(managedTestArtifactRootFolderPath)'
artifactFileName: '$(managedTestArtifactName)$(archiveExtension)'
artifactName: '$(managedTestArtifactName)'
displayName: 'managed test artifacts (built on ${{ parameters.managedTestBuildOsGroup }}${{ parameters.managedTestBuildOsSubgroup }})'
# Download and unzip target specific tests (unless we build them below)
- ${{ if ne(parameters.testBuildPhased, true) }}:
- template: /eng/pipelines/common/download-artifact-step.yml
parameters:
unpackFolder: '$(managedTestArtifactRootFolderPath)'
artifactFileName: '$(managedTestArtifactName)$(archiveExtension)'
artifactName: '$(managedTestArtifactName)'
displayName: 'managed test artifacts (built on ${{ parameters.managedTestBuildOsGroup }}${{ parameters.managedTestBuildOsSubgroup }})'


# Download product binaries directory
Expand All @@ -236,6 +238,24 @@ jobs:
artifactName: '$(coreClrProductArtifactName)'
displayName: 'CoreCLR product download for Mono'


# Build target specific tests
- ${{ if ne(parameters.corefxTests, true) }}:
- ${{ if eq(parameters.testBuildPhased, true) }}:
# Install test build dependencies
- ${{ if eq(parameters.osGroup, 'OSX') }}:
- script: sh $(Build.SourcesDirectory)/eng/install-native-dependencies.sh $(osGroup)
displayName: Install native dependencies
- ${{ if eq(parameters.osGroup, 'Windows_NT') }}:
# Necessary to install correct cmake version
- script: $(Build.SourcesDirectory)\eng\common\init-tools-native.cmd -InstallDirectory $(Build.SourcesDirectory)\native-tools -Force
displayName: Install native dependencies

# 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 •
edited
Loading

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


# Download and unzip the Microsoft.NET.Sdk.IL package needed for traversing
# ilproj test projects during copynativeonly.
- ${{ if ne(parameters.corefxTests, true) }}:
Expand Down
17 changes: 0 additions & 17 deletions eng/pipelines/coreclr/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,23 +103,6 @@ jobs:
liveLibrariesBuildConfig: Release
testBuildPhase: targetGeneric

- template: /eng/pipelines/common/platform-matrix.yml
parameters:
jobTemplate: /eng/pipelines/common/templates/runtimes/build-test-job.yml
buildConfig: checked
platforms:
- Linux_arm
- Linux_arm64
- OSX_x64
- Windows_NT_arm
- Windows_NT_arm64
- Windows_NT_x64
- Windows_NT_x86
testGroup: outerloop
jobParameters:
liveLibrariesBuildConfig: Release
testBuildPhase: targetSpecific

#
# Checked JIT test runs
#
Expand Down
40 changes: 0 additions & 40 deletions eng/pipelines/runtime.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -592,46 +592,6 @@ jobs:
eq(dependencies.checkout.outputs['SetPathVars_coreclr.containsChange'], true),
eq(variables['isFullMatrix'], true))

- template: /eng/pipelines/common/platform-matrix.yml
parameters:
jobTemplate: /eng/pipelines/common/templates/runtimes/build-test-job.yml
buildConfig: checked
platforms:
- Linux_arm
- Windows_NT_x86
- Windows_NT_arm
- Windows_NT_arm64
jobParameters:
testGroup: innerloop
liveLibrariesBuildConfig: Release
testBuildPhase: targetSpecific
condition: >-
or(
eq(dependencies.checkout.outputs['SetPathVars_coreclr.containsChange'], true),
eq(variables['isFullMatrix'], true))

#
# CoreCLR Test builds using live libraries debug build
# Only when CoreCLR is changed
#
- template: /eng/pipelines/common/platform-matrix.yml
parameters:
jobTemplate: /eng/pipelines/common/templates/runtimes/build-test-job.yml
buildConfig: checked
platforms:
- OSX_x64
- Linux_x64
- Linux_arm64
- Windows_NT_x64
jobParameters:
testGroup: innerloop
liveLibrariesBuildConfig: ${{ variables.debugOnPrReleaseOnRolling }}
testBuildPhase: targetSpecific
condition: >-
or(
eq(dependencies.checkout.outputs['SetPathVars_coreclr.containsChange'], true),
eq(variables['isFullMatrix'], true))

#
# CoreCLR Test executions using live libraries
# Only when CoreCLR is changed
Expand Down