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 testing into separate pipeline job #4675

Merged
merged 17 commits into from
Aug 1, 2024

Conversation

JohnMcPMS
Copy link
Member

@JohnMcPMS JohnMcPMS commented Jul 25, 2024

Change

The goal of this change is to separate the tests from the build. Then, in the event of a spurious test failure, one needs only re-run the failed job rather than the entire build.

Microsoft Reviewers: Open in CodeFlow

@JohnMcPMS JohnMcPMS requested a review from a team as a code owner July 25, 2024 00:48
azure-pipelines.yml Outdated Show resolved Hide resolved
- job: 'Test'
timeoutInMinutes: 120
dependsOn: 'Build'
condition: succeeded('Build')
Copy link
Member

Choose a reason for hiding this comment

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

Is the condition needed if it already has dependsOn?

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried that, and it turns out, yes. The default of succeeded() will be false because of the skipped jobs we don't run for PRs.

Comment on lines +290 to +310
# Run BimSkim for all the binaries
- task: BinSkim@4
displayName: 'Run BinSkim '
inputs:
arguments: 'analyze
"$(buildOutDir)\AppInstallerCLI\winget.exe"
"$(buildOutDir)\WinGetUtil\WinGetUtil.dll"
"$(buildOutDir)\WindowsPackageManager\WindowsPackageManager.dll"
"$(buildOutDir)\Microsoft.Management.Deployment.InProc\Microsoft.Management.Deployment.InProc.dll"
"$(Build.SourcesDirectory)\src\WinGetUtilInterop\bin\WinGetUtil*Interop.dll"
"$(buildOutDir)\UndockedRegFreeWinRT\winrtact.dll"
"$(buildOutDir)\Microsoft.WinGet.Client.Cmdlets\Microsoft.WinGet.Client*.dll"
"$(buildOutDir)\ConfigurationRemotingServer\ConfigurationRemoting*Server.dll"
"$(buildOutDir)\ConfigurationRemotingServer\ConfigurationRemoting*Server.exe"
"$(buildOutDir)\ConfigurationRemotingServer\Microsoft.Management.Configuration*.dll"
"$(buildOutDir)\Microsoft.Management.Configuration\Microsoft.Management.Configuration*.dll"
"$(buildOutDir)\Microsoft.Management.Configuration.OutOfProc\Microsoft.Management.Configuration*.dll"
--config default --recurse'

- task: securedevelopmentteam.vss-secure-development-tools.build-task-publishsecurityanalysislogs.PublishSecurityAnalysisLogs@3
displayName: 'Publish Security Analysis Logs'
Copy link
Member

Choose a reason for hiding this comment

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

Do we still need these? I think the pipeline templates we are using for production builds may be enough

Copy link
Member Author

Choose a reason for hiding this comment

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

🤷 Wasn't my goal to change that.


variables:
buildOutDir: $(Pipeline.Workspace)\Build.$(artifactIdentifier)
artifactsDir: $(Build.ArtifactStagingDirectory)\$(buildPlatform)
Copy link
Member

Choose a reason for hiding this comment

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

Why is the build platform needed here?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was before we split the architectures into their own jobs (although it probably wasn't done this way). Now it isn't, but it also doesn't hurt things.

This comment has been minimized.

This comment has been minimized.

florelis
florelis previously approved these changes Aug 1, 2024
src/LocalhostWebServer/Run-LocalhostWebServer.ps1 Outdated Show resolved Hide resolved
@JohnMcPMS JohnMcPMS merged commit ecbe61d into microsoft:master Aug 1, 2024
9 checks passed
@JohnMcPMS JohnMcPMS deleted the pipeline-test-split branch August 1, 2024 23:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants