Skip to content

Perf Tests | Default Config File - #4606

Open
benrr101 wants to merge 2 commits into
mainfrom
dev/russellben/perf-default-config
Open

Perf Tests | Default Config File#4606
benrr101 wants to merge 2 commits into
mainfrom
dev/russellben/perf-default-config

Conversation

@benrr101

Copy link
Copy Markdown
Contributor

Description

I got annoyed that my changes to the runnerconfig file kept getting blown away when I'd switch branches. We don't have this problem with the manual tests project despite it having a config file. So, I instructed the 🤖 to mirror the pattern in the perf tests project. This moves the runnerconfig.jsonc file to the runnerconfig.default.jsonc, and adds targets to the perf test project to use the default config as the live config (if it doesn't exist already) and copy the live config to the build output. It then adds the live config file as a gitignore, to deter accidentally committing one's DB credentials.

Issues

N/A

Testing

Perf test pipeline should be reran to ensure that the changes play nice.

@benrr101 benrr101 added this to the 8.0.0-preview1 milestone Aug 27, 2026
Copilot AI balanced review requested due to automatic review settings August 27, 2026 19:43
@benrr101
benrr101 requested a review from a team as a code owner August 27, 2026 19:43
@benrr101 benrr101 added the Area\Engineering Use this for issues that are targeted for changes in the 'eng' folder or build systems. label Aug 27, 2026
@github-project-automation github-project-automation Bot moved this to To triage in SqlClient Board Aug 27, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Moves performance-test configuration to a generated, locally editable file.

Changes:

  • Adds a default runner configuration template.
  • Generates and copies the live configuration during builds.
  • Documents and ignores the local configuration file.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

File Description
runnerconfig.default.jsonc Provides the default configuration template.
Microsoft.Data.SqlClient.PerformanceTests.csproj Generates and copies the live configuration.
BUILDGUIDE.md Documents the new configuration workflow.
.gitignore Excludes the local runner configuration.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +28 to +29
<Target Name="CopyRunnerConfig" BeforeTargets="Compile">
<Copy SourceFiles="runnerconfig.default.jsonc" DestinationFiles="runnerconfig.jsonc" Condition="!Exists('runnerconfig.jsonc')" />

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would suggest updating the run-perf-tests scripts to build, then look at the config, then dotnet run --no-build the benchmarks.

For the Kusto script, should the benchmark results contain the config file used? Then this script would be guaranteed to see the same config that produced the results.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Alright, I've got an update ready for the scripts that builds the project before looking modifying the config.

I personally would wonder if we should consider instead following the pattern that the manual tests config uses in the sqlclient-pr pipeline. That pipeline reads the default config, updates the necessary values then writes it out to the config.jsonc file. Easily done with powershell, but with bash it'd a pain - and we currently have scripts of both flavors running the perf tests. I'm not sure why, but I don't want this to grow anymore in scope. Let me know your thoughts.

@github-project-automation github-project-automation Bot moved this from To triage to Waiting for customer in SqlClient Board Aug 27, 2026
Copilot AI review requested due to automatic review settings August 31, 2026 20:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

Review details
  • Files reviewed: 10/11 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area\Engineering Use this for issues that are targeted for changes in the 'eng' folder or build systems.

Projects

Status: Waiting for customer

Development

Successfully merging this pull request may close these issues.

4 participants