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

Modernize the performance scripts - add the ability to test a locally built NuGet #5974

Merged
merged 17 commits into from
Aug 20, 2024

Conversation

nkolev92
Copy link
Member

@nkolev92 nkolev92 commented Aug 15, 2024

Bug

Fixes: https://github.com/NuGet/Client.Engineering/issues/2962

Description

  • This makes the test scripts more current, updating the test beds to more recent versions.
  • It also disables NuGetAudit in every test case. Most of the time we don't want to test that since audit changes over time.
  • Adds the ability to disable certain runs from the PerformanceTestRunner, allowing people to run more targeted tests.
  • Adds a git clean option in some of the runs as existing assets files affect the numbers
  • I also added the ability to specify a different test case directory, since sometimes the defaults simply aren't good enough.
  • Add offline restores option.
  • Removed orleans. The repo is simply too small for current standards. No real big learnings from that.

For reference here's the command that I've been running from the root of my local repo:

.\scripts\perftests\PerformanceTestRunner.ps1 -resultsFolderPath resultsFolder -nugetClientFilePaths "C:\Program Files\Microsoft Visual Studio\2022\Preview\MSBuild\Current\Bin\amd64\MSBuild.exe" -skipRepoCleanup -additionalOptions "-skipColdRestores -skipWarmup -skipNoOpRestores -useLocallyBuiltNuGet"

Here's how test run results look after that.
OrchardCore-git.csv
NuGet-Client-git.csv

PR Checklist

  • Meaningful title, helpful description and a linked NuGet/Home issue
  • Added tests
  • Link to an issue or pull request to update docs if this PR changes settings, environment variables, new feature, etc.

@nkolev92 nkolev92 added the Engineering Changes related to the build infrastructure of the repo and that do not change product code label Aug 15, 2024
@nkolev92 nkolev92 marked this pull request as ready for review August 15, 2024 17:55
@nkolev92 nkolev92 requested a review from a team as a code owner August 15, 2024 17:55
zivkan
zivkan previously approved these changes Aug 16, 2024
scripts/perftests/PerformanceTestUtilities.ps1 Outdated Show resolved Hide resolved
scripts/perftests/PerformanceTestUtilities.ps1 Outdated Show resolved Hide resolved
@nkolev92 nkolev92 requested a review from zivkan August 16, 2024 20:26
@nkolev92 nkolev92 enabled auto-merge (squash) August 20, 2024 17:59
Copy link
Contributor

@jebriede jebriede left a comment

Choose a reason for hiding this comment

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

Nit: There's a mix of casing for If and Else throughout. LGTM otherwise! Approving.

@@ -479,9 +484,44 @@ Function RunRestore(
$staticGraphOutputValue = "N/A"
}

$stopwatch = [System.Diagnostics.Stopwatch]::StartNew()
If($isClientDotnetExe -Or $isClientMSBuild)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: capital If inconsistent with others.

Suggested change
If($isClientDotnetExe -Or $isClientMSBuild)
if($isClientDotnetExe -Or $isClientMSBuild)

@nkolev92 nkolev92 merged commit 1599103 into dev Aug 20, 2024
28 checks passed
@nkolev92 nkolev92 deleted the dev-nkolev92-perfScripts branch August 20, 2024 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Engineering Changes related to the build infrastructure of the repo and that do not change product code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants