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

Split integration test build from test run #71166

Merged
merged 1 commit into from
Dec 15, 2023

Conversation

dibarbet
Copy link
Member

@dibarbet dibarbet commented Dec 8, 2023

This is part 1 of running integration tests on helix. This PR separates the build and testing stages in preparation for running on helix, it does not actually send the tests to helix to run.

This PR updates the integration test pipeline to use the same payload creation that we use for our helix unit tests. The integration test pipeline now does the following:

  1. First build the assets once per configuration
  2. Re-use the PrepareTests.csproj to create a minimized test assets payload containing the VSIX and integration test project outputs.
  3. Upload that payload to the pipeline artifacts.
  4. In a separate stage, we then download the test payload
  5. Rehydrate the payload
  6. Install the VSIXs and run the tests

In followup PRs, instead of running the integration tests on a normal ADO machine, we will instead partition them and upload them to helix, using the same payload support that I've added in this PR.

Example PR run:
https://dev.azure.com/dnceng-public/public/_build/results?buildId=495861&view=results
image

Example CI run:
https://dev.azure.com/dnceng-public/public/_build/results?buildId=496084&view=results
image

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Infrastructure untriaged Issues and PRs which have not yet been triaged by a lead labels Dec 8, 2023
@dibarbet dibarbet force-pushed the dev/dibarbet/integration_test_helix branch 15 times, most recently from 12a3423 to d13eb4f Compare December 8, 2023 22:15
@dibarbet dibarbet force-pushed the dev/dibarbet/integration_test_helix branch 3 times, most recently from 83a8c24 to 6589974 Compare December 11, 2023 19:12
@dibarbet dibarbet marked this pull request as ready for review December 11, 2023 22:17
@dibarbet dibarbet changed the title [WIP] Split integration test build from test run Split integration test build from test run Dec 11, 2023
@dibarbet dibarbet requested review from a team as code owners December 11, 2023 22:19
eng/build.ps1 Outdated
$vsixDir = Get-PackageDir "RoslynTools.VSIXExpInstaller"
$vsixExe = Join-Path $vsixDir "tools\VsixExpInstaller.exe"

$vsixExe = Join-Path $ArtifactsDir "bin\Microsoft.VisualStudio.LanguageServices.New.IntegrationTests\$configuration\net472\VSIXExpInstaller\VSIXExpInstaller.exe"
Copy link
Member Author

Choose a reason for hiding this comment

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

Now that we're not building on the same machine, the nuget package directory is not available. Adding the vsix installer to the integration test project is an easy way to ensure that it is available in the test payload.

Copy link
Member

Choose a reason for hiding this comment

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

❗ This is an unsupported VSIX installer, and really needs to be removed from usage. The new integration tests already reference the new one, so we definitely should not be using that project to set this bad example. The solution here is to package up the solution packages folder separately from other build artifacts, similar to what I showed in the other example PR I sent you.

- name: testRuns
type: object
default:
- oop64bit: true
Copy link
Member Author

Choose a reason for hiding this comment

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

the template accepts a list of input parameters so that we can re-use the build assets for multiple different configurations of test run.

env:
HELIX_CORRELATION_PAYLOAD: '$(Build.SourcesDirectory)\.duplicate'

# This is a temporary step until the actual test run moves to helix (then we would need to rehydrate the tests there instead)
Copy link
Member Author

Choose a reason for hiding this comment

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

these will be removed in the next followup where the rehydration happens in the helix script.

filePath: eng/build.ps1
arguments: -ci -prepareMachine -testVsi -configuration ${{ parameters.configuration }} -oop64bit:$${{ testParameters.oop64bit }} -oopCoreClr:$${{ testParameters.oopCoreClr }} -collectDumps -lspEditor:$${{ testParameters.lspEditor }}

# These are temporary publishing steps - once the tests run on helix, the artifacts will be attached to the helix payload.
Copy link
Member Author

Choose a reason for hiding this comment

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

These are basically unchanged from the normal template. Will be removed when on helix as we'll just copy the files into the helix upload root.

@dibarbet
Copy link
Member Author

@sharwell @dotnet/roslyn-infrastructure PTAL

@dibarbet
Copy link
Member Author

If I don't hear any further comments - planning on merging tomorrow.

eng/build.ps1 Outdated
$vsixDir = Get-PackageDir "RoslynTools.VSIXExpInstaller"
$vsixExe = Join-Path $vsixDir "tools\VsixExpInstaller.exe"

$vsixExe = Join-Path $ArtifactsDir "bin\Microsoft.VisualStudio.LanguageServices.New.IntegrationTests\$configuration\net472\VSIXExpInstaller\VSIXExpInstaller.exe"
Copy link
Member

Choose a reason for hiding this comment

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

❗ This is an unsupported VSIX installer, and really needs to be removed from usage. The new integration tests already reference the new one, so we definitely should not be using that project to set this bad example. The solution here is to package up the solution packages folder separately from other build artifacts, similar to what I showed in the other example PR I sent you.

@dibarbet dibarbet force-pushed the dev/dibarbet/integration_test_helix branch from 6589974 to 365fc48 Compare December 14, 2023 00:52
@dibarbet dibarbet requested a review from sharwell December 14, 2023 05:34
@dibarbet
Copy link
Member Author

@sharwell believe I've addressed your comments if you wouldn't mind taking another look

@dibarbet dibarbet merged commit 618b085 into main Dec 15, 2023
@dibarbet dibarbet deleted the dev/dibarbet/integration_test_helix branch December 15, 2023 21:38
@ghost ghost added this to the Next milestone Dec 15, 2023
@Cosifne Cosifne modified the milestones: Next, 17.9 P3 Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Infrastructure untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants