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

chore(testing): use correct JSruntime in the sample test project #259

Merged
merged 4 commits into from
Aug 21, 2023

Conversation

KristianMariyanov
Copy link
Member

No description provided.

@egil
Copy link

egil commented Aug 10, 2023

Ideally, you do not set the JSInterop globally to Loose, since that affects not only your javascript but also any other javascript there could be in use.

If your javascript is accessed via modules, you can instead make that configuration change just on the module level.

More details here: https://bunit.dev/docs/test-doubles/emulating-ijsruntime.html#support-for-importing-javascript-modules

@KristianMariyanov
Copy link
Member Author

Ideally, you do not set the JSInterop globally to Loose, since that affects not only your javascript but also any other javascript there could be in use.

If your javascript is accessed via modules, you can instead make that configuration change just on the module level.

More details here: https://bunit.dev/docs/test-doubles/emulating-ijsruntime.html#support-for-importing-javascript-modules

Unfortunately, it's not accessed via a module. However, all of the js functions are inside the telerikBlazor namespace and I guess I can use something like:

JSInterop.SetupVoid(x => x.Identifier.StartsWith("TelerikBlazor", StringComparison.InvariantCultureIgnoreCase));

@egil Is this seems good to you?

… inside Telerik js package, but not return result for other js functions.
@egil
Copy link

egil commented Aug 10, 2023

For a void call, that should be good enough.

Do you have any calls that return values? If that is the case, you may need to do something like JSInterop.Setup<string>(x => x.Identifier.StartsWith("TelerikBlazor", StringComparison.InvariantCultureIgnoreCase)).SetResult("response data");

@egil
Copy link

egil commented Aug 10, 2023

What loose mode does is that it just returns whatever is the default for the requested return type, e.g. default(string).

@KristianMariyanov
Copy link
Member Author

What loose mode does is that it just returns whatever is the default for the requested return type, e.g. default(string).

We have just a few. I will start with setting bool and string, and I will have to see if there are other variances. Thanks for the example.

@KristianMariyanov
Copy link
Member Author

It was pretty easy to check. We have a few methods for bool, string and double

I use the approach you suggested for them.

Comment on lines 30 to 32
Services.AddSingleton(jsRuntimeMock);
Services.AddSingleton(this.JSInterop.JSRuntime);
Copy link

@egil egil Aug 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed?

bUnit TestContext performs the same registration during its ctor. I think that runs before this ctor runs.

@egil
Copy link

egil commented Aug 10, 2023

Not directly related to this PR, but this code could also cause issues if stars align just right:

private void EnsureRootComponent()
        {
            if (rootComponent is not null) return;
            // add a Telerik Root Component to hold all Telerik components and other content
            rootComponent = (IRenderedComponent<TelerikRootComponent>)Renderer.RenderComponent<TelerikRootComponent>(new ComponentParameterCollection());
            // provide the Telerik Root Component to the child components that need it (the Telerik components)
            RenderTree.TryAdd<CascadingValue<TelerikRootComponent>>(p =>
            {
                p.Add(parameters => parameters.IsFixed, true);
                p.Add(parameters => parameters.Value, RootComponent.Instance);
            });
        }

The problem with rendering TelerikRootComponent separately from the component under testing is that they will exists in different component trees.

A better solution is to do the following in the ctor:

RenderTree.Add<TelerikRootComponent>();

This will create a unique instance of the TelerikRootComponent as the parent for each component that is rendered via the RenderComponent/Render methods. That does ensure there is no shared state between render calls. This is not compatible with exposing RootComponent as a property though. Wondering what the use case is for that though.

@radkostanev
Copy link
Member

radkostanev commented Aug 15, 2023

This will create a unique instance of the TelerikRootComponent as the parent for each component that is rendered via the RenderComponent/Render methods. That does ensure there is no shared state between render calls. This is not compatible with exposing RootComponent as a property though. Wondering what the use case is for that though.

A typical setup relies on a TelerikRootComponent wrapping the Body fragment of the layout - something along the lines of:

@inherits LayoutComponentBase
<TelerikRootComponent>
@Body
</TelerikRootComponent>

There are two main use cases that this component solves - (1) it serves as a config container for any app-wide settings, and (2) all popups are rendered as its direct children, with the latter bit being the important one when it comes to testing. Because of this specific, we need to capture a reference to this fragment, as when someone wants to assert against a given markup rendered within a popup, he will not find said markup within the component under test. Instead, he would have to execute the Find method against the TelerikRootComponent, as this is where the popup has been rendered, despite wherever it has been declared.

@egil
Copy link

egil commented Aug 15, 2023

That is a use case we should really be supporting better out of the box.

One thing we are considering is the possibility to walk up the component tree to the root/parent component instead of just down to child components via FindComponents. That will come in V2.

@egil
Copy link

egil commented Aug 18, 2023

Anyway, if you remove the JSInterop registration the changes look good to me.

@KristianMariyanov
Copy link
Member Author

@egil, Thanks for the invaluable feedback.

@KristianMariyanov KristianMariyanov merged commit c35aa57 into master Aug 21, 2023
@KristianMariyanov KristianMariyanov deleted the bunit-use-correct-jsruntime branch August 21, 2023 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants