Fixes unit test templates for spaces in names by timheuer · Pull Request #3959 · microsoft/aspire

timheuer · 2024-04-25T21:35:05Z

Renamed placeholder project name
Manual validation
Validated dotnet new aspire-starter -t xunit.net -o asp-starter-with-redis-with-xunit0
Validated dotnet new with aspire-xunit,aspire-nunit,aspire-mstest using various hyphenated names
Follows same pattern as project templates
Changed to look for new symbol name in SLN file

Microsoft Reviewers: Open in CodeFlow

radical · 2024-04-25T22:09:43Z

 using System.Net;

-namespace Aspire.Tests1;
+namespace Aspire.Tests._1;


Yeah I was going to ask this too.

Oh this is required to instruct the template engine which naming convention to use.

This is just a pattern that instructs the template engine to do this replacement. See also: https://github.com/dotnet/aspire/blob/main/src/Aspire.ProjectTemplates/templates/aspire-starter/AspireStarterApplication.1.Web/WeatherApiClient.cs#L1

I'm curious why it's not needed in the ASP.NET Core templates though https://github.com/dotnet/aspnetcore/blob/eb18cb23cf2be9db70ae799e9b23a74a26e78d0b/src/ProjectTemplates/Web.ProjectTemplates/content/StarterWeb-CSharp/Controllers/HomeController.cs#L18

ah

namespace Template._1; //uses `namespace` form

Isn't that link suggesting that AspireStarterApplication.1 would be the correct one, and would allow picking which "form" to use for namespace/classname etc, and AspireStarterApplication1 would break it?

Yeah I made it AspireStarterApplication.1 when we first hit this issue but it still doesn't make the auto-value forms work. But, Aspire.StarerApplication1 would work, so we should just change to that to match what ASP.NET Core and others do.

I think I'd recommend sticking with this change only. The change to the others will affect path names as well and you'll change the csproj reference paths. Using this convention allows you to use the value forms and keep the 'identity' form for what the user put in.

Example, if we were to make the change to all. and you dotnet new foo-bar-baz then the file-paths would not be translated (foo-bar-baz.csproj) but then you'd have sln/project references as foo_bar_baz.csproj.

Honestly it feels safer to look at just this change and keep the others as-is as they are functioning within the template engine value forms (identity + namespace + class).

visualized:

I admit I'm a bit lost now lol.
Are you saying that if you change the form to Aspire.StarterApplication1 it breaks? Why is it that the ASP.NET Core and Blazor templates work? Also how can you run Explorer with file extensions hidden!!? 😱

RE: File extensions LOL -- just a sandbox so no customizations -- view hidden always FTW!

But yes, As Ankit also points out the value form scheme allows us to actually achieve what we want. I can't explain why the Blazor ones work (as they have proj ref paths as well) other than the source name uses a dash rather than dot. But the Blazor one is actually broken as well with dashes :-) (root namespace doesn't get corrected)

radical · 2024-04-25T23:48:16Z

/private/tmp/test/asp-starter-with-redis-with-xunit0/asp-starter-with-redis-with-xunit0.Tests/WebTests.cs(11,131): error CS1525: Invalid expression term ')' [/private/tmp/test/asp-starter-with-redis-with-xunit0/asp-starter-with-redis-with-xunit0.Tests/asp-starter-with-redis-with-xunit0.Tests.csproj]

line 11: var appHost = await DistributedApplicationTestingBuilder.CreateAsync<Projects.asp-starter-with-redis-with-xunit0_AppHost>();

radical · 2024-04-25T23:52:37Z

This works;

diff --git a/src/Aspire.ProjectTemplates/templates/aspire-starter/AspireStarterApplication.1.Tests/WebTests.cs b/src/Aspire.ProjectTemplates/templates/aspire-starter/AspireStarterApplication.1.Tests/WebTests.cs
index 26eb9fb66..a0a114dde 100644
--- a/src/Aspire.ProjectTemplates/templates/aspire-starter/AspireStarterApplication.1.Tests/WebTests.cs
+++ b/src/Aspire.ProjectTemplates/templates/aspire-starter/AspireStarterApplication.1.Tests/WebTests.cs
@@ -17,7 +17,7 @@ public class WebTests
     public async Task GetWebResourceRootReturnsOkStatusCode()
     {
         // Arrange
-        var appHost = await DistributedApplicationTestingBuilder.CreateAsync<Projects.AspireStarterApplication.1_AppHost>();
+        var appHost = await DistributedApplicationTestingBuilder.CreateAsync<Projects.AspireStarterApplication._1_AppHost>();
         await using var app = await appHost.BuildAsync();
         await app.StartAsync();

timheuer · 2024-04-26T17:49:19Z

@radical I implemented the last one you found...did your local tests find anything else in the template tests?

DamianEdwards · 2024-04-26T19:10:43Z

 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "AspireStarterApplication.1.ApiService", "AspireStarterApplication.1.ApiService\AspireStarterApplication.1.ApiService.csproj", "{9FEB877E-015D-4E20-AE63-06C596E242E4}"
 EndProject
-#if (CreateTestsProject)
+#if (TestFramework != 'None')


Good catch! I assume you verified this fixes it.

@radical do we have test coverage the starter template creation? Can we add this as a case?

Yep. I assume the conditionals may be case-insensitive, but matched the literal default value in template config and verified 'None' and 'none' both work

What's the case that was caught here? Using none type as the framework generated a test project when it shouldn't have? I check for non-existence of a test project in case of none but I didn't see it failing.

In the current shipping templates the option is CreateTestsProject since there was only one type: xunit. Now that we added unit test framework choice, the option changed to TestFramework and is either none (no arg provided so no tests projects), xunit.net, nunit, or mstest

timheuer · 2024-04-26T19:49:43Z

/backport to release/8.0

github-actions · 2024-04-26T19:49:54Z

Started backporting to release/8.0: https://github.com/dotnet/aspire/actions/runs/8853529459

github-actions · 2024-04-26T19:49:56Z

@timheuer an error occurred while backporting to release/8.0, please check the run log for details!

Error: @timheuer is not a repo collaborator, backporting is not allowed. If you're a collaborator please make sure your dotnet team membership visibility is set to Public on https://github.com/orgs/dotnet/people?query=timheuer

DamianEdwards · 2024-04-26T20:37:35Z

/backport to release/8.0

github-actions · 2024-04-26T20:37:45Z

Started backporting to release/8.0: https://github.com/dotnet/aspire/actions/runs/8854015349

github-actions · 2024-04-26T20:37:49Z

@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: Fixes unit test templates for spaces in names Fix #3957
error: mode change for src/Aspire.ProjectTemplates/templates/aspire-mstest/Aspire.Tests1.csproj, which is not in current HEAD
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Fixes unit test templates for spaces in names Fix #3957
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 128

Please backport manually!

github-actions · 2024-04-26T20:37:49Z

@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.

DamianEdwards · 2024-04-26T20:39:19Z

@timheuer looks like you'll need to manually backport

* Fixes unit test templates for spaces in names Fix #3957 * PR Feedback: missed form change in starter test * Fixes #3974

Fixes unit test templates for spaces in names

50c5c60

Fix #3957

timheuer requested a review from DamianEdwards April 25, 2024 21:35

ghost added the area-templates label Apr 25, 2024

radical reviewed Apr 25, 2024

View reviewed changes

PR Feedback: missed form change in starter test

f275582

DamianEdwards approved these changes Apr 26, 2024

View reviewed changes

Fixes #3974

86f80f6

DamianEdwards reviewed Apr 26, 2024

View reviewed changes

DamianEdwards approved these changes Apr 26, 2024

View reviewed changes

DamianEdwards merged commit bfbc49e into main Apr 26, 2024

DamianEdwards deleted the dev/th/3957 branch April 26, 2024 21:13

timheuer added a commit that referenced this pull request Apr 26, 2024

Fixes unit test templates for spaces in names (#3959)

2d56102

* Fixes unit test templates for spaces in names Fix #3957 * PR Feedback: missed form change in starter test * Fixes #3974

DamianEdwards mentioned this pull request Apr 29, 2024

[release/8.0] Ensure xUnit project is in .sln file & fix hyphen in name #4023

Merged

github-actions Bot locked and limited conversation to collaborators Jun 1, 2024

Conversation

timheuer commented Apr 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Microsoft Reviewers: Open in CodeFlow

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

radical Apr 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

timheuer Apr 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

radical commented Apr 25, 2024

Uh oh!

radical commented Apr 25, 2024

Uh oh!

timheuer commented Apr 26, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

timheuer commented Apr 26, 2024

Uh oh!

github-actions Bot commented Apr 26, 2024

Uh oh!

github-actions Bot commented Apr 26, 2024

Uh oh!

DamianEdwards commented Apr 26, 2024

Uh oh!

github-actions Bot commented Apr 26, 2024

Uh oh!

github-actions Bot commented Apr 26, 2024

Uh oh!

github-actions Bot commented Apr 26, 2024

Uh oh!

DamianEdwards commented Apr 26, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

timheuer commented Apr 25, 2024 •

edited

Loading

radical Apr 25, 2024 •

edited

Loading

timheuer Apr 25, 2024 •

edited

Loading