[chore] first steps for getting a test setup

MathiasWP commented 2 years ago

Setup

Unit/Svelte tests are done with Playwright: npm run playwright
Code tests are done with Vitest: npm run vitest
Testing everything: npm run test

We need to differentiate what testing library should be used based on what type of test is written. I propose the following naming convention:

*.spec.ts == test with Playwright *.test.ts == test with Vitest

Please let me know if you have any thoughts here!

Tests written

I've written the following tests as a MVP check:

clickoutside action
random meta utility method
resettable store

How we test Svelte stuff

We need the ability to have test-components when testing things like actions. There is a lot of annoying pitfalls regarding the import of a Svelte component into a TypeScript file (which playwright uses). Playwright also looks for some stuff in .svelte-kit/output when running the tests (not exactly yet sure why, not really that interesting).

To solve these issues I've created a setup script which does the following:

Build the project if .svelte-kit/output is empty
Compile all *.svelte files in /test to their JS output. Compiling /test/clickoutside/Component.svelte will create a /test/clickoutside/Component.svelte.js file. This is somewhat elegant because the import Component from ./Component.svelte statement in the test file will then point to the JS file instead of the Svelte file. These autogenerated *.svelte.js are currently gitignored.

This setup script runs once before every npm run playwright call.

A challenge we need to fix

I have not yet found a way to support Svelte-kit in the tests. Imports like import { page } from "$app/stores" doesn't make any sense in a normal TypeScript context, and the stores also won't work unless we are in a Svelte-kit context. I'm not sure what the best way is to solve this issue. In the Vitest tests i think the easiest solution is to mock the imports (haven't tried, but Vitest has a great API for mocking imports). It's a little but tricker in the Playwright tests, but there should be a way to solve this.

vercel[bot] commented 2 years ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
svu	✅ Ready (Inspect)	Visit Preview	Aug 10, 2022 at 3:04PM (UTC)

nikolai-cc commented 2 years ago

Really cool stuff. I will have the time to dive into this and will do so right away on Saturday (I'll have to familiarize myself a bit with Playwright before I can say anything useful about it.) I'll keep you updated!

MathiasWP commented 2 years ago

Really cool stuff. I will have the time to dive into this and will do so right away on Saturday (I'll have to familiarize myself a bit with Playwright before I can say anything useful about it.) I'll keep you updated!

Of course! This is just a proposal for a test setup, so any thoughts and ideas are much appreciated!

nikolai-cc commented 2 years ago

Hey! I got a bit lost in the Playwright rabbit hole, so it took a bit longer than expected to get some results. But I have some good news.

First of all, I made some changes to the mvp and now it feels stable enough to use, this is what I had to change:

The manual svelte compiling felt a bit hacky, but luckily this is not necessary. By adding the /test folder to tsconfig we can have the svelte components compile correctly when running the tests.
Playwright uses Vite under the hood; we can extend it's built-in vite config with the $lib alias so that it will resolve correctly.
Vitest expects .svelte-kit/tsconfig.json to be there, so I extended the setup script to include that check.
Various changes which were probably a result of a bit rushed mvp setup. (broken imports, unresolved merge conflicts)

This removes step 2 of 'how we test svelte stuff' and solves a part of the 'challenge we need to fix'. Now that this is out of the way, and I've familiarised myself with this for a bit - some conceptual thoughts.

Pros

I like that with Playwright, we're testing the actions in the context where they're used: svelte components.
I like that Playwright allows us to easily and automatically do cross-browser testing. This is something that the main alternative (using vitest with testing-library) does not have.
I like Playwrights syntax, it seems easy enough.

Cons

I slightly dislike the complexity of maintaining two test suites.
I dislike that currently Playwright is designed for e2e testing, and we have to use the 'experimental' and partly undocumented ct feature. I've spent considerably more time than I wanted running into edge cases.
I still haven't found a way to use $app/store imports, which means we can't test everything we want.

Alternatives

Use Vitest + testing-library instead of Playwright. This way we don't have the complexity of maintaining two different test suites, and two distinct syntaxes for writing tests. It does not have the multi-engine tests of Playwright, but I'm not sure how much we need that anyway
~~don't bother~~

Other thoughts

I know this is is a bit of a nitpick, but I dislike cluttering of the root folder with test-related stuff. I could change the top-level playwright folder to test/template, but moving the config seems to break everything. I'll experiment with this for a bit more.

Conclusion

I think this is a great start and is getting to a nice spot. There's some complexity involved which I dislike, but as I stated in the issue, I'm aware that this is a necessary evil. I think that once we finalise this setup and resolve the $app imports it's a decent enough foundation to build all tests on. See below for some related SvelteKit issues on that topic.

nikolai-cc commented 2 years ago

Here some related things I've come across (mostly for my own reference) related to testing / vitest / playwright:

Playwright Components Experimental docs: https://playwright.dev/docs/test-components
An interesting (WIP) post about using vitest and playwright with svelteKit: https://github.com/davipon/svelte-component-test-demo
shimming $app/stores issue in sveltekit: https://github.com/sveltejs/kit/issues/1485
related issue: https://github.com/sveltejs/kit/issues/5525#issuecomment-1186390654

MathiasWP commented 2 years ago

Awesome work! I am sorry for leaving some unpolished code in the PR, it was a bit rushed.

I believe all the changes you've made are the correct changes, so I'll just give some thoughts about the state of testing:

We will most likely have to use experimental technologies, or maybe pave some of the paths ourselves when it comes to testing SvelteKit. SvelteKit is young and in beta, plus there aren't many other libraries like this for SvelteKit. I agree that an ideal setup would be Vitest + an extra library that supports component/e2e testing for Svelte. I have no personal preference here, but i feel that being able to write as good tests as possible > having 2 test suites. If using testing-library allows us to write the tests we need, then I'm all for using that instead 👍

The $app/* import issue is bad, and this is something that I am positive we could solve with mocking in Vitest. If i'm wrong, then could we write a transformer in Vite that that replaces the $app/* imports with a mocked version of the code?

Turning this...

import { goto } from "$app/navigation"

...into this?

const goto = () => {};

Or can we resolve $app in the test config as an alias?

I'm impressed by the work you've done here Nikolai, I feel that we're getting somewhere!

nikolai-cc commented 2 years ago

It's been a while!

I've explored the alternative vitest route, you can see some of that in this branch. In the end I concluded that testing actions would probably take more hassle compared to getting the $app imports to work.

So I've spent the rest of the time performing black magic, pulling my hairs out and shouting at my computer, but I think I've come to a reasonable solution. 😅

For the component tests we can tell Playwright to resolve the $lib alias as we're used to, and the $app alias to a mocked version of the $app stores. Don't ask me how long I tried to spent getting them to correctly resolve to the .svelte-kit/runtime folders before realising that the Svelte context is not available since playwright-ct runs Svelte instead of Kit 🤦🙈

I've also solved the problem for Vitest. You can see that in action in the other branch, and will port that to this one this weekend.

Anyways, try it out and let me know what you think!

As you'll see all test-related stuff (including config) is now in the test folder. The tests themselves are in test/tests.

To test everything: npm run test To test vitest: npm run test:vitest To test playwright: npm run test:playwright

If you encounter problems, this might be resolved by cleaning the playwright cache: npm run test:clean

To me this looks like a decent foundation. If you don't have anything to add I'll write up a test README this weekend and then we can start adding tests.

P.S. While working on this problem I realised that in the near future I'll want to rewrite as many actions as possible to not depend on the svelte stores 🙈 This way it's easier to use them outside of the SvelteKit environment such as in the REPL or pure Svelte projects. But that's for another day.

nikolai-cc commented 2 years ago

I've fixed some more things and this is ready to merge now. Further discussion can be done in #7

nikolai-cc / svu