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

Add overload IRenderedComponentBase.InvokeAsync(Func<Task>) #166

Closed
JeroenBos opened this issue Jul 20, 2020 · 1 comment · Fixed by #177
Closed

Add overload IRenderedComponentBase.InvokeAsync(Func<Task>) #166

JeroenBos opened this issue Jul 20, 2020 · 1 comment · Fixed by #177
Labels
enhancement New feature or request good first issue Good for newcomers
Milestone

Comments

@JeroenBos
Copy link
Contributor

JeroenBos commented Jul 20, 2020

Description of unexpected behavior:

I have a test which looks something like this:

[Fact]
async Task MyTest()
{
    IRenderedComponent<MyComponent> cut = ...;
    bool fetched = false;
    await cut.InvokeAsync(async () => {
        await FetchSomething();
        fetched = true;
    });
    
    Assert(fetched == true);  // throws!!
}

To my surprise, fetched is not set to true (you can assume FetchSomething doesn't throw).

Diagnosis of the problem:

await cut.InvokeAsync in fact doesn't await the provided callback.
That's because it has the signature Task InvokeAsync(Action callback) and so the lambda I provide above is converted to type Action, whereas for it to be awaited it should be converted to Func<Task> (which is what I expected).
By the way, the implementation of InvokeAsync simply delegates to Dispatcher.InvokeAsync, which does provide an overload accepting Func<Task>.

Proposed solution:

In my opinion, await cut.InvokeAsync(async () => { ... }) actually awaits the lambda in idiomatic C#. To enable this desired behavior, I propose to add an overload:

// in bunit.core/IRenderedComponentBase.cs:

/// <summary>
/// Invokes the given <paramref name="callback"/> in the context of the associated <see cref="ITestRenderer"/>.
/// </summary>
/// <param name="callback"></param>
/// <returns>A <see cref="Task"/> that will be completed when the action has finished executing.</returns>
Task InvokeAsync(Func<Task> callback);

and a trivial implementation:

// in bunit.web/Rendering/RenderedComponent.cs:

/// <inheritdoc/>
public Task InvokeAsync(Func<Task> callback) => Renderer.Dispatcher.InvokeAsync(callback);
@egil egil added enhancement New feature or request good first issue Good for newcomers labels Jul 20, 2020
@egil
Copy link
Member

egil commented Jul 20, 2020

Hi @JeroenBos

Good catch. Indeed there are overloads missing. My current thinking is that both the IRenderedComponent and IRenderedFragment types needs cleaning up, and I would instead like to move InvokeAsync() methods and SetParametersAndRender methods into extension methods, such that the interface is not polluted with unnecessary methods. That would involve making the property Renderer public, i.e. adding it to the IRenderedFragment interface.

Anyway, thanks for bringing this to my attention. If you fancy contribution a PR that solves this, please let me know, and we can figure out an appropriate design for this and the related issues.

@egil egil added this to the v1.0.0 milestone Jul 29, 2020
@JeroenBos JeroenBos mentioned this issue Aug 5, 2020
9 tasks
@egil egil linked a pull request Aug 5, 2020 that will close this issue
9 tasks
@egil egil closed this as completed Aug 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants