Skip to content

Migrate Keyvault tool to recorded tests#1080

Merged
scbedd merged 107 commits into
microsoft:mainfrom
scbedd:migrate-kv-to-recordings
Nov 20, 2025
Merged

Migrate Keyvault tool to recorded tests#1080
scbedd merged 107 commits into
microsoft:mainfrom
scbedd:migrate-kv-to-recordings

Conversation

@scbedd

@scbedd scbedd commented Nov 6, 2025

Copy link
Copy Markdown
Contributor

This PR migrates keyvault livetests to recorded tests, adds and assets.json.

Day-to-day development is not yet affected. I need to add a couple more PRs to add stages that will run recorded tests in CI before it will mean anything to devs.

…getting closer to actually trying some of this on a per-test level
…the inheriting classes. maybe that's not a bad thing?
…. It actually stores a recording and properly locates the assets.json now! time to regenerate the client, ad the delegating handler, and get it ready for PR!
… now hitting random build failures because of nuget SOMEHOW

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

This PR migrates the KeyVault tool from live tests to recorded tests, enabling test recording and playback functionality. This migration includes infrastructure changes to support per-test matcher settings and comprehensive sanitization of sensitive test data.

Key Changes:

  • Migrated test base class from CommandTestsBase to RecordedCommandTestsBase with sanitizer configurations
  • Integrated IHttpClientService into KeyVaultService to enable HTTP traffic recording/playback
  • Added pre-generated test certificate assets and loading infrastructure

Reviewed Changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
KeyVaultCommandTests.cs Migrated to RecordedCommandTestsBase with sanitizers, test variable registration, and certificate asset handling
KeyVaultService.cs Added IHttpClientService dependency and created helper methods to inject HttpClient into Azure SDK clients
Azure.Mcp.Tools.KeyVault.LiveTests.csproj Added configuration to copy test certificate file to output directory
assets.json Added test proxy assets configuration for recording management
fake-pfx.pfx Added pre-generated test certificate binary for import tests

Comment thread tools/Azure.Mcp.Tools.KeyVault/src/Services/KeyVaultService.cs
Comment thread tools/Azure.Mcp.Tools.KeyVault/src/Services/KeyVaultService.cs Outdated
Comment thread tools/Azure.Mcp.Tools.KeyVault/src/Services/KeyVaultService.cs Outdated
@scbedd scbedd requested a review from a team as a code owner November 13, 2025 23:16
@scbedd

scbedd commented Nov 17, 2025

Copy link
Copy Markdown
Contributor Author

After running recorded tests, we're seeing a new failure on MacOS only. windows and linux both passing recorded tests. Exploring for solution.

scbedd and others added 2 commits November 19, 2025 16:43
…LiveTests/KeyVaultCommandTests.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@scbedd scbedd merged commit 4274efc into microsoft:main Nov 20, 2025
26 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Azure MCP Server Nov 20, 2025
@github-project-automation github-project-automation Bot moved this from 🤔 Triage to 🔬 Dev in PR in Azure SDK EngSys 🚀🌒🧑‍🚀 Nov 20, 2025
@kurtzeborn kurtzeborn moved this from 🔬 Dev in PR to 🎊 Closed in Azure SDK EngSys 🚀🌒🧑‍🚀 Nov 20, 2025
colbytimm pushed a commit to colbytimm/microsoft-mcp that referenced this pull request Dec 8, 2025
* inject IHttpClientService into KeyVaultService
* Update KeyvaultCommandTests for recordings
* Add assets.json for recordings run

Co-authored-by: Patrick Hallisey <pahallis@microsoft.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants