Skip to content
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

Async Task in Multiple Tests #1456

Open
jtleaming opened this issue Apr 29, 2024 · 8 comments
Open

Async Task in Multiple Tests #1456

jtleaming opened this issue Apr 29, 2024 · 8 comments
Labels
input needed When an issue requires input or suggestions investigate This issue require further investigation before closing.

Comments

@jtleaming
Copy link

Describe the bug
Testing component with Task.Run inside of async Task OnInitializedAsync results in intermittent test failures. When running the test by itself, it will always succeed. When running with all other tests in the class, certain tests will intermittently fail. It's important to note below is an example of what we see causing, we have 14 tests inside our tests class, verifying different aspects of functionality. The tests themselves and the component are more robust, and I can added in a more detailed example if need be. We've also attempted to use both WaitForState and WaitForAssertion in both cases, neither results in the expected behavior. Additionally, adding timespans of upwards of 15 seconds has not fixed the behavior either.

I assume this has something to do with trying to run multiple threads with the tests and some asynchronous action locking the completion of the task.

I've verified:

  1. Removing all but on test will result in the single test passing every time
  2. Removing the _ = Task.Run and doing all that work inside the initialization results in the tests passing every time.
  3. Running the tests inside of the IDE (Rider) has the same results as running through the cli with the dotnet tests command.

Example:
Testing this component:

@if(loading)
{
    <div class="is-loading">LOADING ICON</div>
}
else
{
    <div class="assert-data">DATA HAS LOADED</div>
}

@code{
    bool loading;

    protected override async Task OnInitializedAsync()
    {
        loading = true;
        ...async work being done
        
        _ = Task.Run(async () => {
                ...async work to be done on separate thread
                loading = false;
                await InvokeAsync(StateHasChanged);
        });
     }
}

With this test:

    [Fact(DisplayName = "Check really important functionality")]
    public void Test()
    {
        var subject = RenderComponent<MyTestComponent>();

        subject.WaitForState(() => _subject.Find(".assert-data").TextContent == "DATA HAS LOADED");
        subject.WaitForAssertion(() => _subject.Find(".assert-data").TextContent.Should.Be("DATA HAS LOADED"));
    }

Results in this output:

Bunit.Extensions.WaitForHelpers.WaitForFailedException : The assertion did not pass within the timeout period. Check count: 6. Component render count: 9. Total render count: 9.

Expected behavior:

I expect the test passes every single time, whether ran individually or ran with all other tests.

Version info:

  • bUnit version: 1.28.9 (saw the same behavior in 1.27.17)
  • .NET Runtime and Blazor version: .net 8
  • OS type and version: windows 11

Additional context:
Using XUnit, FluentAssertions, and MOQ.

@egil
Copy link
Member

egil commented Apr 30, 2024

This should actually work, as far as I can tell. When you call await InvokeAsync(StateHasChanged); at the end of your task, it should trigger a re-render.

However, you may want to change the code slightly, to ensure that changes to loading is visible across CPU core boundaries:

await InvokeAsync(() => 
{
  // set other data.
  loading = false;
  StateHasChanged();
});

This should ensure that the changes you are making in your UI component get dispatched back to the UI thread at a point when the UI thread is free to observe the changes.

If the background task is looong running, you can change the timeout for the WaitFor like so.

subject.WaitForAssertion(() => _subject.Find(".assert-data"), TimeSpan.FromSeconds(5));

An alternative is to replace the background work being done by a test double that just returns data immediately. That way you are testing your UI functionality in isolation from the background work being done.

@jtleaming
Copy link
Author

Thanks for the response. I will test this out today and confirm whether it works or not. If it is still giving me trouble I will try and recreate this in a simpler component I can share here.

All of our data retrieval methods are being mocked, and their return values should be returning immediately. I did think it was possible we forgot to mock a return value, and the test was getting stuck there, but I've confirmed all data requests are in fact mocked and have a specified return value.

@egil
Copy link
Member

egil commented Apr 30, 2024

Question: What are your reasons for spinning up a task on another thread?

Why not just do:

    protected override async Task OnInitializedAsync()
    {
        loading = true;
        await Task.WhenAll(
            ...async work being done,
            ...async work to be done on separate thread);
        loading = false;                
     }

@jtleaming
Copy link
Author

So the UI thread can release while other work is completed. I do also get similar test failures if I await our Task.Run(() =>{}).

@egil
Copy link
Member

egil commented Apr 30, 2024

the UI thread is free to run either way, I do not think you get any benefit of explicitly using Task.Run. However, when you do, you have to be careful about where continuations are run, what scheduler is being used. I have never had any reason to do this in my blazor apps, even for slow database queries.

@egil
Copy link
Member

egil commented Apr 30, 2024

That is, I dont think you are getting the benefit you think by doing Task.Run.

    protected override async Task OnInitializedAsync()
    {
        loading = true;
        await Task.WhenAll(
            DoTask1()
            DoTask2());
        loading = false;                
     }

     private async Task DoTask1() => some async work;
     private async Task DoTask2() => some async work;

This works just as well, as far as I know.

@jtleaming
Copy link
Author

In testing in my particular use case I've found that Task.WhenAll and Task.Run behave similar. Awaiting them, as you have above, will block any further execution from happening. Firing and forgetting, like the example above, will continue any additional initialization work while completing any necessary background work. I'll write up a more real life example with and share it soon.

@egil
Copy link
Member

egil commented Apr 30, 2024

Not true. It will block the rest of the initialization method from running, but the component still completes a render cycle. Take for example this sample component from standard Blazor template:

@page "/weather"
@attribute [StreamRendering]

<PageTitle>Weather</PageTitle>

<h1>Weather</h1>

<p>This component demonstrates showing data.</p>

@if (forecasts == null)
{
    <p><em>Loading...</em></p>
}
else
{
    <table class="table">
        <thead>
            <tr>
                <th>Date</th>
                <th>Temp. (C)</th>
                <th>Temp. (F)</th>
                <th>Summary</th>
            </tr>
        </thead>
        <tbody>
            @foreach (var forecast in forecasts)
            {
                <tr>
                    <td>@forecast.Date.ToShortDateString()</td>
                    <td>@forecast.TemperatureC</td>
                    <td>@forecast.TemperatureF</td>
                    <td>@forecast.Summary</td>
                </tr>
            }
        </tbody>
    </table>
}

@code {
    private WeatherForecast[]? forecasts;

    protected override async Task OnInitializedAsync()
    {
        // Simulate asynchronous loading to demonstrate streaming rendering
        await Task.Delay(500);

        var startDate = DateOnly.FromDateTime(DateTime.Now);
        var summaries = new[] { "Freezing", "Bracing", "Chilly", "Cool", "Mild", "Warm", "Balmy", "Hot", "Sweltering", "Scorching" };
        forecasts = Enumerable.Range(1, 5).Select(index => new WeatherForecast
        {
            Date = startDate.AddDays(index),
            TemperatureC = Random.Shared.Next(-20, 55),
            Summary = summaries[Random.Shared.Next(summaries.Length)]
        }).ToArray();
    }

    private class WeatherForecast
    {
        public DateOnly Date { get; set; }
        public int TemperatureC { get; set; }
        public string? Summary { get; set; }
        public int TemperatureF => 32 + (int)(TemperatureC / 0.5556);
    }
}

This will complete a render cycle and render <p><em>Loading...</em></p>. Then, when the task in OnInitializedAsync, another render cycle is kicked of, and the content is rendered to screen.

In other words, you are not achieving what you think you are by creating task with Task.Run and not awaiting them.

Learn more here: https://learn.microsoft.com/en-us/aspnet/core/blazor/components/lifecycle?view=aspnetcore-8.0

@egil egil added input needed When an issue requires input or suggestions investigate This issue require further investigation before closing. labels May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
input needed When an issue requires input or suggestions investigate This issue require further investigation before closing.
Projects
None yet
Development

No branches or pull requests

2 participants