solid-router icon indicating copy to clipboard operation
solid-router copied to clipboard

Writing tests that involve solid-app-router

Open faassen opened this issue 2 years ago • 13 comments

I've been trying to write tests that involve solid-app-router. The idea is to use solid-testing-library with jest in an application that uses solid-app-router.

So far I'm stopped by a very basic problem: node cannot interpret the import statements inside solid-app-router/dist and fails with a Syntax Error. Why isn't this being transformed by babel? I don't know. I exclude solid-app-router from jest's transformIgnorePatterns but there seems to be absolutely no transformation taking place. How solid-js/web works then, which also uses import statements, is a bit of a mystery: it's either due to a custom babel plugin being injected, or, I suspect, because web.cjs is pulled in, which doesn't include any import statements. Why then is the preset solid-js/web including anything in solid-js for transform? It seems to have no effect.

To play with this problem I've created a project. It demonstrates normal tests with solid-testing-library work, but that tests that use solid-app-router don't even get started due the Syntax Error.

https://github.com/faassen/solid-minimal-jest

I suspect that once this import issue is resolved, other issues will come to the fore, we'll see.

faassen avatar May 13 '22 14:05 faassen

I think the issue is the Babel configuration. From Babel v7, .babelrc files are treated as relative configuration which from my understanding wont be applied to external packages. Changing the .babelrc file in your example to babel.config.js with the following content appears to resolve the issue of the router's export not being processed by Babel.

module.exports = {
  presets: ["@babel/preset-env", "@babel/preset-typescript"],
};

rturnq avatar May 17 '22 20:05 rturnq

Thank you! I think I was mislead by this babel config apparently being required to run tests at all but at the same time seemingly having no effect.

It was good enough to transform the code in the project itself but not good enough to transform anything else. I will see about updating the ts jest testing PR too.

faassen avatar May 17 '22 21:05 faassen

I've updated https://github.com/faassen/solid-minimal-jest

The tests involving solid-app-router now start, but the router doesn't appear to route when I use testing library to click on the routes.

The basic attempt I'm trying to make it this:

    const link = screen.getByRole("link"); // a Link
    expect(link).toBeInTheDocument();
    user.click(link); // expect the routing to take place
    // the event loop takes one Promise to resolve to be finished
    await Promise.resolve();

This mirrors code for react-router that I found here (and that uses user-event): https://testing-library.com/docs/example-react-router/

But this doesn't appear to result in any routing taking place -- the page stays at root instead.

I noticed solid-app-router itself doesn't appear to have tests for its components, only for the underlying implementation, so that's unfortunately no help for this problem.

faassen avatar May 18 '22 12:05 faassen

I tried using this test from your example and noticed you didn't wrap you application in a Router component. After adding that and updating the test to use fireEvent from solid-testing-library to click the link I ran into a few more problems that appear to be related to the testing environment missing some global objects.

This line in the router fails because SVGAElement is not defined.

This line in the router will later fail due jsdom throwing a not implemented error for window.scrollTo.

I'm not sure what the best way to resolve these issues. Maybe there is something needed for jsdom to expose all the globals.

Finally, using await Promise.resolve() doesn't seem to work to wait for the router to update the location. Location changes ion the router are run in a concurrent transition which ultimately executes them in a microtask. It appears the Promise.resolve() microtask in the tests actually gets queued first so any assertions after it will be run before the location has been updated. Using a task (eg setTimeout) to wait here does work.

rturnq avatar May 18 '22 18:05 rturnq

I am a bit confused what version of my test you used.

I did wrap stuff in Router but perhaps in the wrong way?

I moved to firing userevent as this was used in the react router example and seemed to avoid the SVGAElement issue, did you switch to fireEvent deliberately?

faassen avatar May 18 '22 20:05 faassen

I will see though whether setTimeout fixes the issues and will let you know!

faassen avatar May 18 '22 20:05 faassen

Ah, I was looking at this version from the initial commit and just added the link click, promise await and an assertion on location.pathname. Let me pull your latest.

rturnq avatar May 18 '22 21:05 rturnq

With the latest version that test will work, you just need to await the user.click(link). In fact you don't even need the Promise.resolve() after it since all microtasks (updating the location) will have run when the click promise returns back to the test.

rturnq avatar May 18 '22 21:05 rturnq

Did you see the test pass? I seem to be missing something. I've pushed a change that adds the await, but I still have a failure.

I tried to do what you suggested, and tried the following:

    await user.click(link);

    expect(
      screen.getByText("Item Collection Page", { exact: false })
    ).toBeInTheDocument();

But this fails on the expect, meaning the routing wasn't successful.

I've tried removing the expect altogether. In that case I get both ReferenceError: SVGAElement is not defined as well as Error: Not implemented: navigation (except hash changes). It appears this is triggered by the await for the user.click, as without await I don't get these errors.

This at first appeared to be working:

    user.click(link);

    // alternative: try to wait for the routing to complete
    // we should now on the page we routed to
    waitFor(() => {
      expect(
        screen.getByText("Item Collection Page", { exact: false })
      ).toBeInTheDocument();
    });

But that turned out to be incorrect: if I change the 'getByText' to expect something else entirely, the test still passes, so it appears the expect inside the waitFor never becomes true.

faassen avatar May 19 '22 09:05 faassen

I pulled your latest and did a fresh install and now it does indeed fail on the reference error. Without the await I suspect the error is swallowed.

Even though this is caused by the testing environment missing some functionality, I can make a minor changes to the router to work around it and make testing easier for everyone.

rturnq avatar May 19 '22 17:05 rturnq

That would be useful, thanks!

Would it make sense to add some component level tests to solid-app-router itself to verify this works? I can pare down my project to some minimal test case PR if that would help.

We also need to update the solid jest integration to ensure Jest passes the router files through babel.

faassen avatar May 19 '22 19:05 faassen

Any way I can help here? I could contribute a PR with failing tests, for instance.

faassen avatar Jun 07 '22 09:06 faassen

My apologies for not following up. 0.3.4 fixes the SVGAElement reference error issue so testing should work now. I would love a PR that adds component and/or integration tests even if that is just getting things setup.

rturnq avatar Jun 07 '22 17:06 rturnq

Are there any updates @faassen? Otherwise I am inclined to close this as fixed.

ryansolid avatar Nov 21 '22 20:11 ryansolid

I haven't had the chance to play with this again. I am fine with closing it; can always reopen if I get around to it.

faassen avatar Nov 21 '22 20:11 faassen