[tests] workload tests for test project templates by radical · Pull Request #4240 · microsoft/aspire

radical · 2024-05-21T00:53:39Z

Add tests for test framework specific templates - aspire-xunit, aspire-nunit, and aspire-mstest.
Also, test these with different project names to validate the generated code
And add a test for validating a aspire-starter template with the various project names
The tests revealed a missing using in the test cs file, fixed here

Microsoft Reviewers: Open in CodeFlow

radical · 2024-06-15T08:37:14Z

@DamianEdwards the xunit test file in the template seems to be broken. I think this should fix it.

edd5812 (#4240)

The test failures: https://dev.azure.com/dnceng-public/public/_build/results?buildId=708682&view=ms.vss-test-web.build-test-results-tab

``` ... testroot/aspire-aspire-xunit/aspire-aspire-xunit.aspire-xunitTests/IntegrationTest1.cs(21,27): error CS1061: 'IServiceCollection' does not contain a definition for 'ConfigureHttpClientDefaults' and no accessible extension method 'ConfigureHttpClientDefaults' accepting a first argument of type 'IServiceCollection' could be found (are you missing a using directive or an assembly reference?) [/datadisks/disk1/work/B74F0975/t/testroot/aspire-aspire-xunit/aspire-aspire-xunit.aspire-xunitTests/aspire-aspire-xunit.aspire-xunitTests.csproj] ```

…e produced. Useful for local builds

joperezr · 2024-06-18T22:37:52Z

+            }
+
+            var files = Directory.EnumerateFiles(SourceDirectory, "*.csproj", SearchOption.AllDirectories);
+            PackageNames = files.Where(f => File.ReadAllText(f).Contains("<IsPackable>true</IsPackable>"))


This would break if we move the IsPackable to a Directory.Build.props file, right? I wonder if instead of using an inline task if it's best to just use MSBuild task for all of these projects and call a target that just returns the value of IsPackable. We don't have to do it that way but I just think that would probably be more future proof and resilient against refactorings.

Thanks for the suggestion! Changed.

joperezr

Left couple of suggestions, but looks good otherwise.

eerhardt · 2024-06-18T23:56:56Z

+
+    <!-- check against an arbitrary minimum limit to catch any issues getting the list of packages -->
+    <Error Text="Too few packages? count: @(ExpectedPackageNames->Count())"
+           Condition="@(ExpectedPackageNames->Count()) &lt; 60"


We are already shipping over 60 packages from this repo? Wow.

eerhardt · 2024-06-18T23:58:24Z

+            // ActiveIssue for windows: https://github.com/dotnet/aspire/issues/4555
+            yield return "aspire_龦唉丂荳_㐁ᠭ_ᠤསྲིདخەلꌠ_1ᥕ";
+        }
+
+        // ActiveIssue: https://github.com/dotnet/aspire/issues/4550
+        // yield return "aspire  sta-rter.test"; // Issue: two spaces
+
+        yield return "aspire_starter.1period then.34letters";
+        yield return "aspire-starter & with.1";
+
+        // ActiveIssue: https://github.com/dotnet/aspnetcore/issues/56277
+        // yield return "aspire_😀";


Wow - this is catching a lot of bugs. 😄

I think falls under "naming is one of the hardest problems in computer science" right? 😉

For the curious this might be a bug in the template engine - dotnet/sdk#41667 !

eerhardt · 2024-06-19T00:01:47Z

+
+                if (inTest && CommentLineRegex().IsMatch(line))
+                {
+                    sb.AppendLine(CommentLineRegex().Replace(line, "    "));


Hopefully we don't ever add real comments to the code, or else this would start failing.

Lol! Yeah, but that would cause a failure on the PR, and then that person gets to figure it out :/

Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>

.. packable, instead of a custom task. Doing this instead of a custom task looking for a `<IsPackable>true</IsPackable>` string is better because it would not be affected by refactorings, or the property being set elsewhere. Suggestion from @jopererz .

radical added the testing ☑️ label May 21, 2024

ghost added the area-integrations Issues pertaining to Aspire Integrations packages label May 21, 2024

radical mentioned this pull request May 23, 2024

[tests] Follow up work for Workload tests PR #4222

Closed

7 tasks

radical force-pushed the workload-tests-follow-up branch from 4dd5cee to c7364bc Compare May 30, 2024 05:52

radical force-pushed the workload-tests-follow-up branch from 842b3c1 to e914e75 Compare June 17, 2024 18:34

radical added 3 commits June 17, 2024 16:56

[tests] Workload: add tests for aspire-{testFramework} templates

55ad364

[tests] Workload: Add check to ensure that all the expected nugets ar…

ee6ed0a

…e produced. Useful for local builds

radical force-pushed the workload-tests-follow-up branch from e914e75 to ee6ed0a Compare June 17, 2024 20:59

radical requested a review from DamianEdwards June 17, 2024 20:59

radical marked this pull request as ready for review June 17, 2024 20:59

radical requested review from eerhardt and joperezr June 17, 2024 20:59

radical changed the title ~~[tests] Additional workload tests~~ [tests] workload tests for test framework templates Jun 17, 2024

radical changed the title ~~[tests] workload tests for test framework templates~~ [tests] workload tests for test project templates Jun 17, 2024

radical added 4 commits June 17, 2024 17:48

fix typo

9a44365

disable failing tests, and reference the relevant issues

596f75d

fix

83a1371

Merge remote-tracking branch 'origin/main' into workload-tests-follow-up

9408fe3

joperezr reviewed Jun 18, 2024

View reviewed changes

Comment thread tests/Shared/Aspire.Workload.Testing.props

joperezr reviewed Jun 18, 2024

View reviewed changes

joperezr approved these changes Jun 18, 2024

View reviewed changes

radical added 2 commits June 18, 2024 19:43

address review feedback from @ joperezr

7084c39

Merge remote-tracking branch 'origin/main' into workload-tests-follow-up

890d70b

eerhardt approved these changes Jun 19, 2024

View reviewed changes

radical and others added 4 commits June 18, 2024 20:23

Update tests/Aspire.Workload.Tests/WorkloadTestsBase.cs

6e21706

Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>

Update tests/Aspire.Workload.Tests/PerTestFrameworkTemplatesTests.cs

3aa1a90

Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>

Merge remote-tracking branch 'origin/main' into workload-tests-follow-up

89d6a5a

radical merged commit 95aca25 into microsoft:main Jun 19, 2024

radical deleted the workload-tests-follow-up branch June 19, 2024 03:12

github-actions Bot locked and limited conversation to collaborators Jul 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[tests] workload tests for test project templates#4240

[tests] workload tests for test project templates#4240
radical merged 13 commits intomicrosoft:mainfrom
radical:workload-tests-follow-up

radical commented May 21, 2024 •

edited

Loading

Uh oh!

radical commented Jun 15, 2024

Uh oh!

Uh oh!

joperezr Jun 18, 2024

Uh oh!

radical Jun 19, 2024

Uh oh!

joperezr left a comment

Uh oh!

eerhardt Jun 18, 2024

Uh oh!

Uh oh!

eerhardt Jun 18, 2024

Uh oh!

DamianEdwards Jun 20, 2024

Uh oh!

radical Jun 20, 2024

Uh oh!

eerhardt Jun 19, 2024

Uh oh!

radical Jun 19, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

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

Uh oh!

Microsoft Reviewers: Open in CodeFlow

Uh oh!

radical commented Jun 15, 2024

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joperezr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

radical commented May 21, 2024 •

edited

Loading