-
Notifications
You must be signed in to change notification settings - Fork 685
Increase timeout for applicaton start in DistributedApplicationTests #9473
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Addresses intermittent test failures by extending the application startup timeout in DistributedApplicationTests to accommodate slower environments (e.g., Defender scans).
- Replaces shorter
DefaultOrchestratorTestTimeout
withDefaultOrchestratorTestLongTimeout
forapp.StartAsync()
calls across various tests. - Some subsequent resource retrieval and wait calls still use the shorter timeout—these should be made consistent.
- Suggests reducing duplication in timeout invocations for maintainability.
Comments suppressed due to low confidence (4)
tests/Aspire.Hosting.Tests/DistributedApplicationTests.cs:683
- This resource retrieval still uses the shorter DefaultOrchestratorTestTimeout after extending the startup timeout. Update to DefaultOrchestratorTestLongTimeout to keep timeouts consistent and avoid flakiness.
var aspireDashboard = await KubernetesHelper.GetResourceByNameMatchAsync<Executable>(kubernetes, $"aspire-dashboard-{ReplicaIdRegex}-{suffix}", r => r.Status?.EffectiveEnv is not null).DefaultTimeout(TestConstants.DefaultOrchestratorTestTimeout);
tests/Aspire.Hosting.Tests/DistributedApplicationTests.cs:723
- This resource retrieval uses the shorter timeout while the startup call was extended. Switch to DefaultOrchestratorTestLongTimeout for consistency.
var aspireDashboard = await KubernetesHelper.GetResourceByNameMatchAsync<Executable>(kubernetes, $"aspire-dashboard-{ReplicaIdRegex}-{suffix}", r => r.Status?.EffectiveEnv is not null).DefaultTimeout(TestConstants.DefaultOrchestratorTestTimeout);
tests/Aspire.Hosting.Tests/DistributedApplicationTests.cs:1083
- Waiting for text uses the shorter timeout after startup now uses the extended timeout. Consider using DefaultOrchestratorTestLongTimeout here as well to prevent premature failures.
await app.WaitForTextAsync("Application started.").DefaultTimeout(TestConstants.DefaultOrchestratorTestTimeout);
tests/Aspire.Hosting.Tests/DistributedApplicationTests.cs:1133
- This wait call still relies on the default short timeout. Update to DefaultOrchestratorTestLongTimeout to align with the extended startup timeout.
await app.WaitForTextAsync("Application started.").DefaultTimeout(TestConstants.DefaultOrchestratorTestTimeout);
We need to open an issue for this. |
It might be related to the new DCP. Will try again when DCP 0.15.1 is merged UPDATE nope the 0.15.1 hit the same issue |
Garnet test failure was due to a new Garnet image being pushed that requires a specific command line argument in order to enable binding to 0.0.0.0. |
I guess I need to rebase... |
b28032e
to
df89579
Compare
Description
I am often hitting a timeout associated with application startup when running these tests on my dev machine. I am rebuilding DCP often and Defender takes time to scan new bits.
Checklist
<remarks />
and<code />
elements on your triple slash comments?doc-idea
templatebreaking-change
templatediagnostic
template