Add an non-CI integration test for KeyVault#615
Conversation
|
Warning Review limit reached
More reviews will be available in 39 minutes and 23 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR introduces a conditional Azure Key Vault integration smoke test. The test project gains user secrets support via configuration; a new smoke test class writes, reads, and deletes Key Vault secrets when explicitly enabled; and documentation explains how to opt in to running the test. ChangesKey Vault Smoke Test Feature
Sequence DiagramsequenceDiagram
participant Test as KeyVaultSmokeTests
participant Config as UserSecrets
participant Service as SecretsService
participant KeyVault as Azure Key Vault
Test->>Config: Load AzureSettings from user secrets
Config-->>Test: Return ClientId, ClientSecret, KeyVaultUrl
Test->>Test: Validate required settings
Test->>Service: Instantiate with Azure credentials
Service->>KeyVault: Write secret with unique name
KeyVault-->>Service: Secret created
Service->>KeyVault: Read secret by name
KeyVault-->>Service: Secret value returned
Service-->>Test: Assert value matches
Test->>KeyVault: Delete secret (finally block)
KeyVault-->>Test: Secret deleted
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
README.md (1)
38-40: ⚡ Quick winDocument shell-specific command variants for broader developer usability.
Line 39 is POSIX-style env assignment; it won’t run as-is in Windows
cmdor PowerShell. Add shell-specific examples to reduce setup friction.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 38 - 40, The README shows a POSIX-style env assignment for the test command (RUN_KEYVAULT_SMOKE_TESTS=true dotnet test ...), which won't work in Windows cmd/PowerShell; add equivalent Windows variants: a cmd.exe form that uses setlocal && set RUN_KEYVAULT_SMOKE_TESTS=1 && dotnet test ... and a PowerShell form that uses $env:RUN_KEYVAULT_SMOKE_TESTS = 'true'; dotnet test ... (and mention reverting/unsetting if needed), so developers on different shells can run the KeyVaultSmokeTests easily.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Test/KeyVaultSmokeTests.cs`:
- Around line 44-58: The cleanup is fragile because secretWasCreated can remain
false if SetSecret persisted remotely but threw; change the finally block to
always attempt to delete the secret (call DeleteSecret(secretName)) and swallow
only "not found" errors so legitimate missing secrets don't fail the test—i.e.,
remove the secretWasCreated guard and wrap await
secretsService.DeleteSecret(secretName) in its own try/catch that ignores
not-found exceptions (but surfaces other errors); reference SetSecret,
GetSecret, DeleteSecret and the secretWasCreated flag when making the change.
---
Nitpick comments:
In `@README.md`:
- Around line 38-40: The README shows a POSIX-style env assignment for the test
command (RUN_KEYVAULT_SMOKE_TESTS=true dotnet test ...), which won't work in
Windows cmd/PowerShell; add equivalent Windows variants: a cmd.exe form that
uses setlocal && set RUN_KEYVAULT_SMOKE_TESTS=1 && dotnet test ... and a
PowerShell form that uses $env:RUN_KEYVAULT_SMOKE_TESTS = 'true'; dotnet test
... (and mention reverting/unsetting if needed), so developers on different
shells can run the KeyVaultSmokeTests easily.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fb4ac8ae-bdeb-4db0-ab9f-1377fd5e1d18
📒 Files selected for processing (3)
README.mdTest/KeyVaultSmokeTests.csTest/Test.csproj
Summary by CodeRabbit
Documentation
Tests