Update samples for preview.5 #174
Merged
Conversation
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
DamianEdwards
added
the
NO-MERGE
|
Could the non-test changes be split out to a separate PR, and merged first? |
Yes? But I'd kinda like to get some test coverage as part of the preview.5 push honestly. |
Simplify volume name randomization routine too
davidfowl
removed
the
NO-MERGE
davidfowl
approved these changes
Apr 10, 2024
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.
Still a work in progress.Updates the samples for preview.5 and adds integration tests to verify that all samples startup successfully and health checks endpoints in projects return healthy status. Lots of code here to make the integration testing possible including waiting for resources to successfully start, capturing resource console logs and AppHost
ILoggerlogs, and asserting against both that no errors occurred, and more randomization to ensure proper test execution and isolation including parameter values (to ensuring they have values) and volume names. Some of this should make its way back intoAspire.Hosting.Testingat some point.The Dapr sample still fails the integration tests due to a known issue that should be fixed by dotnet/aspire#3305 but that's for preview.6
All samples have been converted to use HTTPS inline with changes in preview.5. Note that the Node.js sample needed to be changed to configure Node.js (as a client) to work with the ASP.NET Core HTTPS dev cert which we should likely improve in the future (see dotnet/aspire#3324). The SPA apps sample (AspireWithJavaScript) is using HTTPS for the .NET pieces but the SPA dev servers are all running with HTTP still.
Samples using SQL Server have had workarounds for dotnet/aspire#1023 applied.
Before merging will need the final preview.5 package version updated and
nuget.configfeeds reverted.Fixes #172
Fixes #168
Fixes #164