bUnit-dev / bUnit

bUnit is a testing library for Blazor components that make tests look, feel, and runs like regular unit tests. bUnit makes it easy to render and control a component under test’s life-cycle, pass parameter and inject services into it, trigger event handlers, and verify the rendered markup from the component using a built-in semantic HTML comparer.
https://bunit.dev
MIT License
1.12k stars 104 forks source link

bUnit does not set parameters as a result of two-way binding or chained binding when values are updated. #1434

Closed celophi closed 5 months ago

celophi commented 5 months ago

Describe the bug:

When using two-way binding or chained binding, bUnit fails to update parameters as a result of event callbacks. This happens even after using the .Bind option in the ParameterBuilder. I have a discussion here; https://github.com/bUnit-dev/bUnit/discussions/1432.

Example:

@* Child.razor *@
<h3>@currentValue</h3>

@code {
    [Parameter] public string Value { get; set; } = string.Empty;
    [Parameter] public EventCallback<string> ValueChanged { get; set; }

    private string currentValue;

    protected override void OnParametersSet()
    {
        this.currentValue = Value;
    }
}
@* Parent.razor *@
<h3>Parent</h3>

<Child @bind-Value="Value" />

<button id="mybtn" type="button" @onclick="OnbuttonClick">clickme</button>

@code {
    [Parameter] public string Value { get; set; } = string.Empty;
    [Parameter] public EventCallback<string> ValueChanged { get; set; }

    private void OnbuttonClick()
    {
        this.ValueChanged.InvokeAsync("button clicked");
    }
}
public class TwoWayBindingTest : TestContext
{
    private string Value { get; set; } = "no event";

    [Fact(DisplayName = "Markup should contain the button clicked text that was bound.")]
    public async Task Test1()
    {
        var component = RenderComponent<Parent>(p => p.Bind(c => c.Value, Value, newValue => Value = newValue));

        component.Find("#mybtn").Click();

        component.Markup.Should().Contain("button clicked");
    }
}

Results in this output:

Expected cut.Markup "<h3>Parent</h3>

<h3>no event</h3>

<button id="mybtn" type="button" blazor:onclick="1">clickme</button>" to contain "button clicked".

Stack Trace: 
XUnit2TestFramework.Throw(String message)
TestFrameworkProvider.Throw(String message)
DefaultAssertionStrategy.HandleFailure(String message)
AssertionScope.FailWith(Func`1 failReasonFunc)
AssertionScope.FailWith(Func`1 failReasonFunc)
AssertionScope.FailWith(String message, Object[] args)
StringAssertions`1.Contain(String expected, String because, Object[] becauseArgs)
Test.Test1() line 23
RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

Expected behavior: Aside from the test passing, once the bind delegate is invoked with the new value in the test, I would expect that OnParametersSet is called with Value having the updated value as a result of the .Bind() delegate instead of the initial value.

Version info: bUnit 1.27.17 OS type and version: Windows 10 .NET 8.0

Additional Context: It appears like using SetParametersAndRender after the "Act" test step does result in the test passing and the parameter being updated, but this is not an appropriate way to write tests.

linkdotnet commented 5 months ago

After investigating the issue I come to the conclusion that this is not a bug and more or less expected behavior. Before I explain why, let's disect a bit what is going on:

     +------------------+  
     |                  |  
     |   Unit Test      |  
     |   Component      |  
     |  (derives from   |  
     |   TestContext)   |  
     +------------------+  
             |  
   @bind-Value   
             |  
     +------------------+  
     |                  |  
     |  Parent          |  
     |  Component       |  
     +------------------+  
             |  
   @bind-Value   
             |  
     +------------------+  
     |                  |  
     |  Child           |  
     |  Component       |  
     +------------------+  

Now, setting the initial value works - and also when you click your button, everything bubbles from Child to your unit test "Value". So the @bind directive works. The "issue" why you don't any update once it reaches your unit test component is that you derive (which you should) from TestContext. The TestContext is not the same as a ComponentBase so, there will be no re-render after the event is invoked. As Parent and Child are derived from ComponentBase it works without a problem.

The same happens if you do this: var component = Render<BindParent>(@<BindParent @bind-Value="Value"/>);.

So yes calling SetParametersAndRender is the approach here as the "control" is inside your test-code and not inside your component. @egil Correct me, if I missed something here.

celophi commented 5 months ago

The TestContext is not the same as a ComponentBase so, there will be no re-render after the event is invoked. As Parent and Child are derived from ComponentBase it works without a problem.

If this is the case, may I see an example of how we could set up a resetting of parameters and a re-render after the event is invoked within the parameter builder? Needing to call SetParametersAndRender after the "act" step in a test implies that you need to have knowledge of what the code is doing and cannot adopt black box testing.

I'm also confused as to why you wouldn't want TestContext to behave similarly to ComponentBase specifically in this regard. If either of you could explain that, I would appreciate it.

linkdotnet commented 5 months ago

The main reason that TestContext does not derive from ComponentBase is that TestContext (or your test class for that matter) is not part of the Renderer. With RenderComponent you explicitly tell (more or less) what the root of your test is. So just making it a ComponentBase wouldn't change a single thing - well besides throwing errors as there is no RenderHandle around.

If you don't want to use SetParametersAndRender, which I can understand, you can create a wrapper component inside your test-assembly that does the job for you:

<Parent @bind-Value="Value"></BindParent>
@code {
    [Parameter] public string Value { get; set; }
}

And here the modified test:

var component = RenderComponent<Wrapper>(ps => ps.Add(p => p.Value, "no event"));

component.Find("#mybtn").Click();

component.Markup.ShouldContain("button clicked");

I do understand that it isn't the same as using bind - but this would "mimick" the Blazor behavior as close as possible.

celophi commented 5 months ago

Thanks for the example, and I can start to understand the points you are making.

I'm still not seeing the reason for the .Bind() option in the parameter builder. What purpose is this intended to serve then if it does not operate similarly to @bind-Value or @bind-SomeProperty? Is it only so that you can hook into the event callback and inspect values etc.? Can you clarify the intended purpose and usage of this?

linkdotnet commented 5 months ago

Basically we give you two ways of writing tests: In plain old cs files or in (the recommended way) razor files. While folks don't want or can't use razor files, we have to make sure we have a feature parity between those two worlds.

The test you have can be modeled in razor as described above: Render<Parent>(@<Parent @bind-Value="Value"/>); but there wasn't something convenient like that in the "old" cs tests. Therefore we introduced Bind as method.

Now, those two behave exactly the same. The problem, as described earlier, is that crossing the boundary between something that is a component (your production code) and something that isn't (the test) creates some side effects. We might tackle that in the future, but it takes a bigger chunk, I suspect, to make that happen.

Is it only so that you can hook into the event callback and inspect values etc.?

Yes that is the primary use case for that - checking if your stuff bubbles up and down. Maybe we could extend our documentation as such, that you should call SetParametersAndRender (or just Render in the future) after the value bubbles up to your test and you want to propagate the change downwards again. I feel okay because (for me) unit tests are white-box tests. You need a certain knowledge about your code to make it run (that is why we have an arrange part).

celophi commented 5 months ago

Thanks for taking the time to explain and answer questions!