WaitBehavior being ignored when waiting for resource to be healthy in a test.
#7601
Comments
afscrome
changed the title
WaitBehavior being ignored waiting for resource to be healthy in a test.WaitBehavior being ignored when waiting for resource to be healthy in a test.
danmoseley
added
the
area-app-model
|
@eerhardt do you have cycles to look? |
It won't be until next week. |
|
Like like it is working but WaitForHeathy does what it says, waits for the resource to be healthy. I have a change with an overload of wait for health that also takes a wait behavior but that method currently only looks at the health state |
|
Hey @afscrome just getting back into things after being out of office for a few months. Looking at your test case it should fail due to timeout - but what you are suggesting is that it should fail due to dependency startup error? This is what I currently see with your test (adapted) in the repo:
|
|
Welcome back @mitchdenny. In the above test, the This is particularly important for tests (which default to |
|
We have to consider two modes of interaction with this API. One is in test cases like we are discussing here and another is in interactive modes via the dashboard. In the dashboard if a dependency fails to start we can actually restart the dependency and then restart the resource so its not necessarily terminal - and in unit tests we need to be able to simulate this (at least in the aspire repo :)). But yes I agree in the normal case if your dependency moves into the terminal state, unless you are specifically writing code to test this behavior you probably don't want to endlessly wait. I've put up a PR with a proposed fix for this - feel free to take a look. There are a few API design considerations that need to be resolved before we merge (it might end up needing to be a new enumeration). |
|
Just tried this out - almost there, but I think there's one last quirk. [Test]
public async Task Test()
{
using var appHost = DistributedApplicationTestingBuilder.Create();
appHost.Services.AddLogging(x => x
.AddFilter("Default", LogLevel.Information)
.AddFilter("Microsoft.AspNetCore", LogLevel.Warning)
.AddFilter("Aspire.Hosting.Dcp", LogLevel.Warning)
.AddFilter("Aspire.Hosting", LogLevel.Debug)
);
var failToStart = appHost.AddExecutable("failToStart", "does-not-exist", ".");
var dependency = appHost.AddContainer("redis", "redis");
dependency.WaitFor(failToStart);
using var app = appHost.Build();
await app.StartAsync();
}This works if I explicitly provide public async Task<ResourceEvent> WaitForResourceHealthyAsync(string resourceName, CancellationToken cancellationToken = default)
{
return await WaitForResourceHealthyAsync(
resourceName,
- WaitBehavior.WaitOnResourceUnavailable, // Retain default behavior.
+ DefaultWaitBehavior,
cancellationToken).ConfigureAwait(false);
}If not, should the default test templates be updated to include - await app.ResourceNotifications.WaitForResourceHealthyAsync(dependency.Resource.Name).WaitAsync(TimeSpan.FromSeconds(15));
+ await app.ResourceNotifications.WaitForResourceHealthyAsync(dependency.Resource.Name, WaitBehavior.StopOnResourceUnavailable).WaitAsync(TimeSpan.FromSeconds(15));
|
Is there an existing issue for this?
Describe the bug
Trying the following in 9.1. I'm expecting the below test to fail since
dependencyshould fail to start due tofailToStartfailing to start.Expected Behavior
I'd expect waiting for
redisto become healthy to fail as the resource has entered the terminalFailedToStartstate.Steps To Reproduce
See above code
Exceptions (if any)
.NET Version info
Anything else?
The text was updated successfully, but these errors were encountered: