Add non-generic version of DistributedApplicationTestingBuilder.CreateAsync#3875
Conversation
DamianEdwards
requested review from
ReubenBond,
davidfowl,
eerhardt and
mitchdenny
ghost
added
the
area-app-model
DamianEdwards
added
the
breaking-change
eerhardt
left a comment
eerhardt
left a comment
There was a problem hiding this comment.
2 minor-ish comments. This looks good to me.
| /// <typeparam name="TEntryPoint"> | ||
| /// A type in the entry point assembly of the target Aspire AppHost. Typically, the Program class can be used. | ||
| /// </typeparam> | ||
| public class DistributedApplicationFactory<TEntryPoint> : IDisposable, IAsyncDisposable where TEntryPoint : class |
There was a problem hiding this comment.
Are we going to continue to validate where TEntryPoint : class. Was that important?
There was a problem hiding this comment.
I'm not sure why that was there honestly. @ReubenBond?
There was a problem hiding this comment.
Seems just a carry over from WebApplicationFactory<TEntryPoint> which this was based on but I can't see a good reason why it's restricted there too. I don't think it matters too much. We can always loosen the generic type parameter constraint if we ever get feedback on it and folks aren't willing to use the Type overload.
Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
|
/backport to release/8.0 |
|
Started backporting to release/8.0: https://github.com/dotnet/aspire/actions/runs/8807909101 |
|
@DamianEdwards backporting to release/8.0 failed, the patch most likely resulted in conflicts: $ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: Add non-generic DistributedApplicationFactory
.git/rebase-apply/patch:1072: new blank line at EOF.
+
warning: 1 line adds whitespace errors.
Using index info to reconstruct a base tree...
A src/Aspire.Hosting.Testing/PublicAPI.Unshipped.txt
Falling back to patching base and 3-way merge...
CONFLICT (modify/delete): src/Aspire.Hosting.Testing/PublicAPI.Unshipped.txt deleted in HEAD and modified in Add non-generic DistributedApplicationFactory. Version Add non-generic DistributedApplicationFactory of src/Aspire.Hosting.Testing/PublicAPI.Unshipped.txt left in tree.
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Add non-generic DistributedApplicationFactory
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128Please backport manually! |
|
@DamianEdwards an error occurred while backporting to release/8.0, please check the run log for details! Error: git am failed, most likely due to a merge conflict. |
|
LGTM! |
3 participants
This was a bit harder than I had originally anticipated due to API compat requirements now that we're locked down for GA. Unfortunately that meant I had to perform some less-than-ideal gymnastics to share implementation and preserve API details across the existing generic version and new non-generic version (inner types that override members and forward to outer type, etc.).This is a breaking change for GA to unwind the generics and simply have a generic version of the top level factory method.
Fixes #3874
Microsoft Reviewers: Open in CodeFlow