Use aspire in bit Boilerplate test project (if enabled) (#11481) #11482
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR integrates Aspire support into the Bit Boilerplate test project, enabling it to use Aspire when enabled in the template. The main purpose is to enhance the testing infrastructure by leveraging Aspire's distributed application testing capabilities while maintaining backward compatibility.
Key changes include:
- Added Aspire hosting support to the test initializer with proper connection string management
- Updated test methods to use cancellation tokens consistently
- Modified project configuration to conditionally include Aspire dependencies
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| TestsInitializer.cs | Added Aspire host initialization and cleanup, plus connection string environment variable setup |
| IdentityPagesTests.cs | Updated test methods to use cancellation tokens |
| IdentityApiTests.cs | Updated test methods to use cancellation tokens |
| Boilerplate.Tests.csproj | Added conditional Aspire package and project references |
| AppTestServer.cs | Modified Start method to accept cancellation token and removed unused import |
| Program.cs | Updated Aspire resource naming for consistency |
| appsettings.json | Simplified Aspire comments |
| Directory.Packages.props | Added Aspire.Hosting.Testing package version |
| bit.full.ci.yml | Reordered tests and removed SQL Server installation step |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughUpdated multiple GitHub Actions workflows to use actions/setup-node@v6. Significant workflow restructuring in bit.full.ci.yml replaces SQL Server tests with Sqlite and introduces a PostgreSQL-focused flow and renamed SQLServer build. Boilerplate template updates include package bumps, config key renames (sqldb→mssqldb), resource identifier changes in AppHost, and test infrastructure changes adding optional Aspire hosting and cancellation support. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Tester
participant Init as TestsInitializer
participant Aspire as Aspire App (optional)
participant Srv as AppTestServer
Tester->>Init: Initialize(TestContext)
alt aspire enabled
Init->>Aspire: Build & Start distributed app
note right of Aspire: Prune non-essential resources
Init->>Aspire: Wait for DBs (mssqldb/postgresdb/mysqldb)
Init->>Aspire: Wait for storage (azureblobstorage/s3)
Init->>Aspire: Wait for smtp
Init->>Init: Set env vars from Aspire
else aspire disabled
Init->>Init: Use sqlite in-memory defaults
end
Init->>Srv: Start(cancellationToken)
Srv-->>Init: Ready
Init-->>Tester: Test server handle
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/TestsInitializer.cs (2)
37-79: Solid Aspire host strategy; a few small improvements
- Template condition quoting is mixed ('Sqlite' vs "SqlServer"/"PostgreSql"/"MySql"). Standardize quoting to avoid template-engine mismatches.
- Reduce repetition: factor SetEnvironmentVariable + WaitForResource into a small helper to cut duplication and typos.
- Consider restoring overridden ConnectionStrings__* env vars in AssemblyCleanup to avoid cross-run contamination (store originals before SetEnvironmentVariable).
Also verify resource names (mssqldb/postgresdb/mysqldb/azureblobstorage/s3/smtp) match AppHost identifiers; otherwise WaitForResourceAsync will time out.
115-125: Make Cleanup unconditional and dispose the SQLite connectionCurrently Cleanup only compiles when aspire==true, so with Sqlite + aspire==false the in-memory connection is never disposed. Recommend always emitting Cleanup and guarding per-feature inside it.
Apply this diff:
- //#if (aspire == true) [AssemblyCleanup] public static async Task Cleanup() { - if (aspireApp is not null) - { - await aspireApp.StopAsync(); - await aspireApp.DisposeAsync(); - } + //#if (aspire == true) + if (aspireApp is not null) + { + await aspireApp.StopAsync(); + await aspireApp.DisposeAsync(); + } + //#endif + + //#if (database == 'Sqlite') + if (connection is not null) + { + await connection.DisposeAsync(); + } + //#endif } - //#endif
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (21)
.github/workflows/admin-sample.cd.yml(5 hunks).github/workflows/bit.ci.yml(2 hunks).github/workflows/bit.full.ci.yml(3 hunks).github/workflows/blazorui.demo.cd.yml(4 hunks).github/workflows/nuget.org.yml(1 hunks).github/workflows/prerelease.nuget.org.yml(1 hunks).github/workflows/sales-module-demo.cd.yml(2 hunks).github/workflows/todo-sample.cd.yml(7 hunks)src/Templates/Boilerplate/Bit.Boilerplate/.github/workflows/cd-template.yml(4 hunks)src/Templates/Boilerplate/Bit.Boilerplate/.github/workflows/ci.yml(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Directory.Build.props(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Directory.Packages.props(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/appsettings.json(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.AppHost/Program.cs(5 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Web/appsettings.json(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/AppTestServer.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/Boilerplate.Tests.csproj(3 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/IdentityApiTests.cs(3 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/IdentityPagesTests.cs(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/TestsInitializer.cs(3 hunks)src/Templates/Boilerplate/Bit.Boilerplate/vs-spell.dic(1 hunks)
🧰 Additional context used
🪛 Biome (2.1.2)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/appsettings.json
[error] 4-4: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 4-4: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 4-4: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 4-6: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 7-7: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 7-7: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 7-7: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
🔇 Additional comments (26)
src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/AppTestServer.cs (1)
49-51: LGTM! Cancellation token support properly added.The
Startmethod now correctly accepts and propagates aCancellationTokentoWebApp.StartAsync, enabling test scenarios to cancel startup operations. The existingOperationCanceledExceptionhandling inDisposeAsync(lines 62) aligns well with this change.src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/IdentityPagesTests.cs (1)
16-16: LGTM! Consistent cancellation token propagation.Both test methods correctly pass
TestContext.CancellationTokento the updatedStartmethod, ensuring proper cancellation support during test server initialization.Also applies to: 27-27
src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/IdentityApiTests.cs (1)
21-21: LGTM! Consistent cancellation token usage.All test methods now consistently use
TestContext.CancellationTokenfor server startup and API calls. This simplifies the code compared to accessingTestContext.CancellationTokenSource.Tokendirectly and ensures proper test cancellation support.Also applies to: 31-31, 35-35, 49-49, 55-55
.github/workflows/nuget.org.yml (1)
31-31: LGTM! Node.js action upgraded.The setup-node action has been upgraded from v4 to v6, maintaining node-version 23. This aligns with the broader workflow updates across the repository.
.github/workflows/todo-sample.cd.yml (1)
34-34: LGTM! Consistent Node.js action upgrades across all jobs.All setup-node actions have been consistently upgraded from v4 to v6 across the workflow's multiple jobs (build_api_blazor, deploy_blazor_wasm_standalone_aot, deploy_blazor_wasm_standalone_offlineDb, deploy_blazor_wasm_standalone_small, build_blazor_hybrid_windows, build_blazor_hybrid_android, build_blazor_hybrid_ios), maintaining node-version 23 throughout.
Also applies to: 147-147, 199-199, 255-255, 292-292, 350-350, 445-445
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/appsettings.json (1)
4-4: LGTM! Aspire comment simplified.The Aspire comment has been simplified from a multi-sentence explanation to a concise description. The simplified version clearly conveys that running the AppHost overrides connection strings at runtime.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.AppHost/Program.cs (5)
15-15: LGTM! Database identifier updated.The SQL Server database identifier has been updated from "sqldb" to "mssqldb", aligning with the connection string key rename in appsettings.json files.
43-43: LGTM! Blob storage container name updated.The Azure Blob storage container name has been changed from "blobs" to "azureblobstorage", providing clearer semantics and consistency with connection string naming conventions.
46-46: LGTM! S3 storage identifier updated.The Minio container name has been changed from "minio" to "s3", providing clearer indication of the S3-compatible storage being used.
110-110: SMTP MailPit configuration verified. “smtp” matches the connection string keys in appsettings.json (Server.Web & Server.Api) andWithReference(mailpit)correctly resolves the SMTP settings.
83-83: Verify WithReference signature and connection string resolutionWithReference calls now pass only the resource object (azureBlobStorage / s3Storage); confirm this matches the updated Aspire API and that the connection string names provided in AddBlobs("azureblobstorage") / AddMinioContainer("s3") are still correctly resolved at runtime.
src/Templates/Boilerplate/Bit.Boilerplate/vs-spell.dic (1)
80-80: LGTM! Spell check dictionary updated.The dictionary entry has been updated from "sqldb" to "mssqldb" to align with the codebase rename, ensuring the spell checker recognizes the new identifier.
.github/workflows/bit.full.ci.yml (5)
37-37: LGTM! Node.js action upgraded.The setup-node action has been upgraded from v4 to v6, consistent with other workflow updates in this PR.
53-64: LGTM! PostgreSQL test now includes Aspire support.The test infrastructure has been updated to use PostgreSQL as the primary database with Aspire enabled (
--aspireflag), which directly addresses the PR objective (#11481) to ensure the test project uses Aspire when enabled. The TestPostgreSQL project also includes Sales module and SignalR for comprehensive testing.Also applies to: 68-68, 71-71
74-86: LGTM! Sqlite testing preserved.The workflow now includes a dedicated Sqlite test block (SimpleTest) that includes the
dotnet ef database updatestep, ensuring Sqlite-specific scenarios are still validated. This complements the PostgreSQL/Aspire test coverage.Also applies to: 90-90, 93-93
99-99: LGTM! Cleanup updated for new project names.The cleanup step now removes TestPostgreSQL (replacing the old reference), aligning with the test infrastructure changes.
101-107: LGTM! SQL Server build step renamed and updated.The build step has been appropriately renamed from "Build PostgreSQL and Other database options" to "Build SQLServer and Other database options", with the project name changed from TestPostgreSQL to SQLServer. This ensures SQL Server configurations are still tested while PostgreSQL is now used for the primary test execution with Aspire.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Web/appsettings.json (1)
4-4: All "sqldb" references updated to "mssqldb" – no occurrences of"sqldb"remain in code or configuration.src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/Boilerplate.Tests.csproj (2)
1-1: LGTM!The MSTest.Sdk version bump from 4.0.0 to 4.0.1 is a routine patch update.
11-11: Approve Aspire.Hosting.Testing usage in tests: TestsInitializer.cs leveragesDistributedApplicationandCreateAsync<Projects.Boilerplate_Server_AppHost>to initialize Aspire hosting, satisfying the package reference.src/Templates/Boilerplate/Bit.Boilerplate/src/Directory.Packages.props (3)
53-53: LGTM!The Aspire.Hosting.Testing package version is consistent with the reference in Boilerplate.Tests.csproj. The condition pattern aligns with other Aspire packages in this file.
121-121: LGTM!The Twilio package update from 7.13.3 to 7.13.4 is a routine patch update.
114-114: Verify Microsoft.Identity.Web 4.0.0 migration and compatibility
- Confirm your projects target .NET 8/9 (v4 no longer supports .NET 6/7).
- Reconcile the MSAL bump and update any dependent package versions.
- Follow MIGRATION_GUIDE_V4 to adjust AddMicrosoftIdentityWebApp usage, credential loading, HttpClient/message-handler changes, and agent/managed identity scenarios.
- Run tests and validate all authentication flows (sign-in, token acquisition, downstream APIs, certificates).
src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/TestsInitializer.cs (3)
3-8: Conditional Aspire usings look correctImports match the types used below (DistributedApplicationTestingBuilder, ProjectResource, DevTunnelResource).
Please confirm Aspire packages (Hosting, Testing, DevTunnels) are conditionally referenced in the template (Directory.Packages.props / csproj) when aspire==true.
20-23: Static DistributedApplication field is appropriateAssembly-scoped lifetime for the Aspire host is fine; no threading concerns here.
27-33: Init flow/order and CancellationToken propagation are correctStarting Aspire first, then building/starting the test server with the testContext token is the right order.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/appsettings.json
Show resolved
Hide resolved
src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/Boilerplate.Tests.csproj
Show resolved
Hide resolved
1 participant
closes #11481
Summary by CodeRabbit