Skip to content

DistributedApplicationTests improvements#7325

Merged
karolz-ms merged 4 commits intomainfrom
dev/karolz/test-container-names
Jan 31, 2025
Merged

DistributedApplicationTests improvements#7325
karolz-ms merged 4 commits intomainfrom
dev/karolz/test-container-names

Conversation

@karolz-ms
Copy link
Copy Markdown
Contributor

This change does 3 things

  1. Stops killing DCP process when DcpExecutor.RunApplicationAsync() throws (which makes some DistributedApplicationExecutor tests hang).
  2. Makes DistributedApplicationExecutor tests use unique names for resources they create (in particular, Containers). This should help diagnose any leaking resources.
  3. Disposes of DcpExecutor when DistributedApplication is disposed. This means only executing tests have DCP instances running, and as soon as a test completes, the associated DCP instance goes through the shutdown. This in turn minimizes resource consumption on the machine, in particular there will be less Docker containers/networks at any given time.

@karolz-ms karolz-ms requested a review from mitchdenny as a code owner January 30, 2025 03:34
@karolz-ms karolz-ms changed the title Dev/karolz/test container names DistributedApplicationTests improvements Jan 30, 2025
Comment thread src/Aspire.Hosting/Dcp/DcpExecutor.cs Outdated
Comment thread src/Aspire.Hosting/Dcp/DcpExecutor.cs Outdated
Comment thread src/Aspire.Hosting/Dcp/DcpHost.cs Outdated
Comment thread src/Aspire.Hosting/Orchestrator/ApplicationOrchestrator.cs Outdated
Comment thread src/Aspire.Hosting/Orchestrator/ApplicationOrchestrator.cs Outdated
Comment thread tests/Aspire.Hosting.Tests/DistributedApplicationTests.cs
Comment thread tests/Aspire.Hosting.Tests/DistributedApplicationTests.cs
@davidfowl
Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

Comment thread src/Aspire.Hosting/Dcp/DcpExecutor.cs
@davidfowl
Copy link
Copy Markdown
Contributor

Looks like a real test failure

@karolz-ms karolz-ms force-pushed the dev/karolz/test-container-names branch from 3184722 to 8f2b438 Compare January 30, 2025 18:40
@karolz-ms
Copy link
Copy Markdown
Contributor Author

karolz-ms commented Jan 30, 2025

@davidfowl the failing Aspire.Workload.Tests.BuildAndRunTemplateTests.ProjectWithNoHTTPSRequiresExplicitOverrideWithEnvironmentVariable does not seem to be related to this change on cursory glance. Relevant log fragment

[xUnit.net 00:06:47.94]     Aspire.Workload.Tests.BuildAndRunTemplateTests.ProjectWithNoHTTPSRequiresExplicitOverrideWithEnvironmentVariable [OUTPUT] [first-run] fail: Microsoft.Extensions.Hosting.Internal.Host[11]
[xUnit.net 00:06:47.94]     Aspire.Workload.Tests.BuildAndRunTemplateTests.ProjectWithNoHTTPSRequiresExplicitOverrideWithEnvironmentVariable [OUTPUT] [first-run]       Hosting failed to start
[first-run] fail: Microsoft.Extensions.Hosting.Internal.Host[11]
[first-run]       Hosting failed to start
[xUnit.net 00:06:47.94]     Aspire.Workload.Tests.BuildAndRunTemplateTests.ProjectWithNoHTTPSRequiresExplicitOverrideWithEnvironmentVariable [OUTPUT] [first-run]       Microsoft.Extensions.Options.OptionsValidationException: The 'applicationUrl' setting must be an https address unless the 'ASPIRE_ALLOW_UNSECURED_TRANSPORT' environment variable is set to true. This configuration is commonly set in the launch profile. See https://aka.ms/dotnet/aspire/allowunsecuredtransport for more details.
[first-run]       Microsoft.Extensions.Options.OptionsValidationException: The 'applicationUrl' setting must be an https address unless the 'ASPIRE_ALLOW_UNSECURED_TRANSPORT' environment variable is set to true. This configuration is commonly set in the launch profile. See https://aka.ms/dotnet/aspire/allowunsecuredtransport for more details.
[xUnit.net 00:06:47.94]     Aspire.Workload.Tests.BuildAndRunTemplateTests.ProjectWithNoHTTPSRequiresExplicitOverrideWithEnvironmentVariable [OUTPUT] [first-run]          at Microsoft.Extensions.Options.OptionsFactory`1.Create(String name)
[first-run]          at Microsoft.Extensions.Options.OptionsFactory`1.Create(String name)
[xUnit.net 00:06:47.94]     Aspire.Workload.Tests.BuildAndRunTemplateTests.ProjectWithNoHTTPSRequiresExplicitOverrideWithEnvironmentVariable [OUTPUT] [first-run]          at System.Lazy`1.ViaFactory(LazyThreadSafetyMode mode)
[first-run]          at System.Lazy`1.ViaFactory(LazyThreadSafetyMode mode)
[xUnit.net 00:06:47.94]     Aspire.Workload.Tests.BuildAndRunTemplateTests.ProjectWithNoHTTPSRequiresExplicitOverrideWithEnvironmentVariable [OUTPUT] [first-run]          at System.Lazy`1.ExecutionAndPublication(LazyHelper executionAndPublication, Boolean useDefaultConstructor)
[first-run]          at System.Lazy`1.ExecutionAndPublication(LazyHelper executionAndPublication, Boolean useDefaultConstructor)
[xUnit.net 00:06:47.94]     Aspire.Workload.Tests.BuildAndRunTemplateTests.ProjectWithNoHTTPSRequiresExplicitOverrideWithEnvironmentVariable [OUTPUT] [first-run]          at System.Lazy`1.CreateValue()
[first-run]          at System.Lazy`1.CreateValue()
[xUnit.net 00:06:47.94]     Aspire.Workload.Tests.BuildAndRunTemplateTests.ProjectWithNoHTTPSRequiresExplicitOverrideWithEnvironmentVariable [OUTPUT] [first-run]          at Microsoft.Extensions.Options.OptionsCache`1.GetOrAdd[TArg](String name, Func`3 createOptions, TArg factoryArgument)
[first-run]          at Microsoft.Extensions.Options.OptionsCache`1.GetOrAdd[TArg](String name, Func`3 createOptions, TArg factoryArgument)
[xUnit.net 00:06:47.94]     Aspire.Workload.Tests.BuildAndRunTemplateTests.ProjectWithNoHTTPSRequiresExplicitOverrideWithEnvironmentVariable [OUTPUT] [first-run]          at Microsoft.Extensions.Options.OptionsMonitor`1.Get(String name)
[first-run]          at Microsoft.Extensions.Options.OptionsMonitor`1.Get(String name)
[xUnit.net 00:06:47.94]     Aspire.Workload.Tests.BuildAndRunTemplateTests.ProjectWithNoHTTPSRequiresExplicitOverrideWithEnvironmentVariable [OUTPUT] [first-run]          at Microsoft.Extensions.DependencyInjection.OptionsBuilderExtensions.<>c__DisplayClass0_1`1.<ValidateOnStart>b__1()
[first-run]          at Microsoft.Extensions.DependencyInjection.OptionsBuilderExtensions.<>c__DisplayClass0_1`1.<ValidateOnStart>b__1()
[xUnit.net 00:06:47.94]     Aspire.Workload.Tests.BuildAndRunTemplateTests.ProjectWithNoHTTPSRequiresExplicitOverrideWithEnvironmentVariable [OUTPUT] [first-run]          at Microsoft.Extensions.Options.StartupValidator.Validate()
[first-run]          at Microsoft.Extensions.Options.StartupValidator.Validate()
[xUnit.net 00:06:47.94]     Aspire.Workload.Tests.BuildAndRunTemplateTests.ProjectWithNoHTTPSRequiresExplicitOverrideWithEnvironmentVariable [OUTPUT] [first-run]       --- End of stack trace from previous location ---
[first-run]       --- End of stack trace from previous location ---
[xUnit.net 00:06:47.94]     Aspire.Workload.Tests.BuildAndRunTemplateTests.ProjectWithNoHTTPSRequiresExplicitOverrideWithEnvironmentVariable [OUTPUT] [first-run]          at Microsoft.Extensions.Options.StartupValidator.Validate()
[first-run]          at Microsoft.Extensions.Options.StartupValidator.Validate()
[xUnit.net 00:06:47.94]     Aspire.Workload.Tests.BuildAndRunTemplateTests.ProjectWithNoHTTPSRequiresExplicitOverrideWithEnvironmentVariable [OUTPUT] [first-run]          at Microsoft.Extensions.Hosting.Internal.Host.StartAsync(CancellationToken 

I have asked @joperezr to take a peek too (TIA!)

Other than that no test failures. Optimistically merge?

EDIT: this PR caused the test failure, but it had nothing to do with the error above

@karolz-ms
Copy link
Copy Markdown
Contributor Author

Still working with @joperezr to see if this change made a difference

@karolz-ms
Copy link
Copy Markdown
Contributor Author

Turns out it was indeed my change that caused the test to fail. Fix available.

@karolz-ms karolz-ms enabled auto-merge (squash) January 31, 2025 02:15
@karolz-ms karolz-ms disabled auto-merge January 31, 2025 02:24
@karolz-ms karolz-ms force-pushed the dev/karolz/test-container-names branch from a139781 to 313d250 Compare January 31, 2025 02:32
@karolz-ms karolz-ms enabled auto-merge (squash) January 31, 2025 02:35
@karolz-ms karolz-ms merged commit 3dfdb8d into main Jan 31, 2025
@karolz-ms karolz-ms deleted the dev/karolz/test-container-names branch January 31, 2025 03:25
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 2, 2025
@github-actions github-actions Bot added the area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication label Mar 10, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants