Fix flaky SqlDatabaseProjects test: give each project its own database - #1609
Merged
Merged
Conversation
…abase The example AppHost had sdk-project and chinook both deploying to the shared TargetDatabase with no ordering between them. This caused contention on the SQL Server container that delayed and destabilized the sibling other-sdk-project deployment, leading to an intermittent "Login failed for user 'sa'" error when the test opened its verification connection right after deployment finished. Give each deployed project/package its own dedicated database (Database1/Database2/Database3) so concurrent deployments no longer contend for the same database, and update AppHostTests accordingly. Validated by running the SqlDatabaseProjects AppHostTests 3 times locally under Docker; all passed (9/9 total). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/CommunityToolkit/Aspire/main/eng/scripts/dogfood-pr.sh | bash -s -- 1609Or
iex "& { $(irm https://raw.githubusercontent.com/CommunityToolkit/Aspire/main/eng/scripts/dogfood-pr.ps1) } 1609" |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The database assignments and corresponding test expectations are aligned, with no unresolved blocking issues identified.
Review effort: Lite
Findings: None
What changed in this PR
This PR fixes flaky SQL deployment tests by assigning each project its own database and updating test expectations.
Changes:
- Adds dedicated
Database1,Database2, andDatabase3resources. - Updates database mappings in
AppHostTests.
| File | Summary |
|---|---|
tests/CommunityToolkit.Aspire.Hosting.SqlDatabaseProjects.Tests/AppHostTests.cs |
Verifies each project against its dedicated database. |
examples/sql-database-projects/CommunityToolkit.Aspire.Hosting.SqlDatabaseProjects.AppHost/Program.cs |
Assigns separate databases to SQL deployments. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
|
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the intermittent failure in
AppHostTests.ProjectBasedResourceStartsAndRespondsOkseen in this failing job (Login failed for user 'sa').Root cause: in the example AppHost,
sdk-projectandchinookwere both deploying to the same sharedTargetDatabasewith no ordering between them. That contention delayed/destabilized the siblingother-sdk-projectdeployment on the SQL Server container, so the test's verification connection hit a transient login failure right after deployment.Fix: give each deployed project its own dedicated database (
Database1/Database2/Database3) instead of sharing two databases across three deployments, and updateAppHostTeststo match.Validation: ran the
AppHostTestssuite locally under Docker 3 times (9/9 tests passed each run).