`IRenderedComponent.InvokeAsync` should support return values about bunit HOT 8 CLOSED

I can open a PR for this if it's wanted.

from bunit.

Hey @Jcparkyn

I am not sure if I can follow entirely here. Especially the part about returning a value - it seems you want to invoke something on the component under test and want to retrieve the method return value. Do I grasp that correctly?

If not, can you create a small example and show where the current InvokeAsync isn't enough?

from bunit.

Yes exactly, I need to access the method return value so that I can assert on it, and in some cases also need it for the rest of the test (see below). Here's a small example:

The CallbackObject is a domain object that triggers a render for subscribed components, and also returns a value. The same use case would apply for methods on components directly, but they're probably less likely to be part of the public interface and less likely to have return values.

Helper class:

public class CallbackObject
{
    public event Action? SomethingChanged;

    public int DoSomething()
    {
        SomethingChanged?.Invoke();
        return 123;
    }
}

InvokeAsyncTestComponent.razor:

@implements IDisposable

@_count

@code {
    [Parameter]
    public CallbackObject CallbackObject { get; set; } = null!;
    private int _count = 0;

    private void CountUp()
    {
        _count++;
        StateHasChanged();
    }

    protected override void OnInitialized() => CallbackObject.SomethingChanged += CountUp;
    public void Dispose() => CallbackObject.SomethingChanged -= CountUp;
}

Test:

[Fact]
public async Task Test() // passes
{
    var obj = new CallbackObject();
    using var cut = RenderComponent<InvokeAsyncTestComponent>(
        ("CallbackObject", obj)
    );

    cut.MarkupMatches("0");

    // Can't do this because it triggers a render:
    // int doSomethingResult = obj.DoSomething();

    // Workaround using proposed extension method:
    int doSomethingResult = await cut.InvokeAsync(obj.DoSomething);

    // Another workaround I don't particularly like:
    // int doSomethingResult = -1;
    // await cut.InvokeAsync(() => doSomethingResult = obj.DoSomething());

    cut.MarkupMatches("1");
    Assert.Equal(123, doSomethingResult);
}

My workaround extension method:

static class RenderedFragmentExtensions
{
    /// Version of
    /// <see cref="RenderedFragmentInvokeAsyncExtensions.InvokeAsync(IRenderedFragmentBase, Action)"/>
    /// with a return value.
    public static Task<T> InvokeAsync<T>(this IRenderedFragmentBase renderedFragment, Func<T> workItem)
    {
        ArgumentNullException.ThrowIfNull(renderedFragment);

        return renderedFragment.Services.GetRequiredService<ITestRenderer>()
            .Dispatcher.InvokeAsync(workItem);
    }
}

My real use case is a bit more complicated, because the return value I want to access is actually a Task<T>, and I want to await this after InvokeAsync() returns. The new overloads still support this use-case though, by specifying the overload to use.

from bunit.

Thanks for the clarification.

I am going back and forth in my mind whether or not I think it is a good idea to
a) Have the overloads proposed by you and
b) In general: Testing methods on your instance is a good idea.

My issue goes into the direction that testing the functions inside your component leads to exposing implementation details¹ and doesn't test observable behavior. Now, that can be legitimate (component libraries, maybe?).
The problem I see is that adding the function opens the door to overusing that pattern.

The way you solved it is nice and is "somewhat contained" in your code base - and maybe as last resort because testing the observable behavior isn't possible/has "high cost".

That is why I am leaning more towards not adding this function for a general audience. Maybe @egil has different thoughts here.

¹ EDIT: Maybe some more explanation. While methods might be public (and maybe they shouldn't be), that doesn't mean they are public to your users. Obviously there are cases, like dialogs, where you have to surface the API to pen and close it - but that would be observable without knowing the specifics on how it is achieved.

from bunit.

This is an interesting problem. On one hand, leading users to write better tests and being opinionated about it is a noble endeavor, and on the other hand .net libraries from Microsoft in general, including bUnit, tend to be unopinionated about specific styles/patterns.

I think I am leaning towards adding for completeness sake, since we already have the other InvokeAsync method on IRenderedFragment that already does expose this.

But I really like the idea of having a guiding principles section in our docs that showcase what is considered best practices for maintainable/valuable tests in bUnit, like https://testing-library.com/docs/guiding-principles.

from bunit.

.net libraries from Microsoft in general, including bUnit, tend to be unopinionated about specific styles/patterns.

Interesting thought - I, personally, would argue .NET is very much opinionated. Even more so in recent years. Prominent examples might be EF Core, Minimal API's and the most recent example .NET Aspire . You could argue Blazor / ASP.NET Core itself too. There are many conventions, analyzers and what not pushing you towards a certain direction.
It boils down to every very own definition of opinionated.

from bunit.

@linkdotnet In my case the method I'm testing (and its return value) are both parts of the public API for my library. The test accesses it via a component parameter for convenience, but the method is actually on an object which is exposed to the user (see example below). I know this is a little unconventional, but from my perspective there's not really any other way to write similar functionality in Blazor. I'd definitely be curious to hear an opposing view though.

Here's an example use-case for the API I'm trying to test, taken from my sample app. Note the call to context.LoadNextPage near the bottom. This also has an async version with a return value, which is what I need the new overloads for.

@inject HackerNewsApi Api

<UseEndpointInfinite Endpoint="Api.GetTopStories"
                     Arg="Arg"
                     GetNextPageArg="pages => (pages[^1].Arg?.GetNextPageArgs(), pages.Count < pages[^1].Data?.NbPages)">
    <ul class="post-list">
        @foreach (var query in context.Pages)
        {
            @if (query.HasData)
            {
                var posts = query.Data;
                foreach (var post in posts.Hits)
                {
                    <li>
                        <PostPreview @key="post.Id" Post="post" />
                    </li>
                }
            }
            else if (query.IsError)
            {
                <strong>
                    Something went wrong!
                    @query.Error?.Message
                    <button @onclick="query.Refetch">Retry</button>
                </strong>
            }
        }
    </ul>
    @if (context.HasNextPage || context.IsLoadingNextPage)
    {
        <div style="display: flex; justify-content: center; margin: 2em">
            <button @onclick="context.LoadNextPage" disabled="@context.IsLoadingNextPage" style="width: 10em; font-size: 1.3em;">
                @(context.IsLoadingNextPage
                    ? "Loading..."
                    : "Load More")
            </button>
        </div>
    }
</UseEndpointInfinite>

@code {
    [Parameter, EditorRequired]
    public HackerNewsApi.SearchStoriesArgs Arg { get; set; } = null!;
}

I think it's also quite common for libraries to have public API methods that result in component renders. These usually don't have return values, but I don't see any distinction that makes ones with return values "worse". Here are a couple of examples from libraries I've used:

• https://github.com/Blazored/Modal/blob/8c39c9bb04cab8f9bac58f03d7a44c055f5fd762/src/Blazored.Modal/Services/ModalService.cs#L13: In this case, the relevant tests don't actually use InvokeAsync, because the component is already doing await InvokeAsync(StateHasChanged). (maybe I should be doing that too, but my library is WASM-only and I don't want too much async in my event handlers when I can avoid it).
• https://github.com/Blazored/Toast/blob/634106d79dd2d5533b3fcda9d9c6e9bcb8f7e1d4/src/Blazored.Toast/Services/ToastService.cs#L48
• https://github.com/Blazor-Diagrams/Blazor.Diagrams/blob/46d9800d0d81ee3130208e9565b160829b19e02d/src/Blazor.Diagrams.Core/Diagram.cs#L200

from bunit.

@Jcparkyn - convinced. For component libraries, the picture is different. That said - feel free to create a Pull Request that adds those two extensions.

from bunit.