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

Template tests #19

Closed
Rich-Harris opened this issue Oct 15, 2020 · 41 comments
Closed

Template tests #19

Rich-Harris opened this issue Oct 15, 2020 · 41 comments
Milestone

Comments

@Rich-Harris
Copy link
Member

If the app template doesn't include tests, then one of the first questions people will ask is 'how do we add tests'? We should probably have some way of answering this question

@Rich-Harris Rich-Harris added this to the public beta milestone Oct 29, 2020
@benmccann
Copy link
Member

@benmccann
Copy link
Member

A few questions we'll need to answer:

  • Do we want to have tests in the default template? It's somewhat opinionated, but will help beginners get started more easily
  • What should we use as a test runner? jest and mocha are maybe the most popular, but a lot of our tests use uvu
  • Do we want integration tests as well? Cypress was controversial in the sapper-template partially because of its large download size. We're now using Playwright for our own tests and it's smaller. Do we want to use it in the template as well?

@Rich-Harris
Copy link
Member Author

I won't touch jest with a barge pole, personally, and I don't see any advantage to mocha over uvu. uvu is lean enough that we could get away with including it by default. Playwright is a different matter though. I'm inclined to think it should be an optional add-on for that reason. (At any rate, Playwright by itself probably isn't sufficient for integration testing anyway, it's too low-level.)

But it's hard to know what testing setup to suggest when collectively we haven't really figured out what sort of things should live in a test suite

@pngwn
Copy link
Member

pngwn commented Dec 4, 2020

If we can figure a relatively bullet-proof way to get uvu + JSDOM to work without too much boilerplate we should for it. It isn't perfect but the trade-offs are reasonable for most projects. Luke would be the best person to comment here although I'll be trying to make that work soon myself.

@benmccann
Copy link
Member

benmccann commented Dec 4, 2020

Here's an example of Svelte + uvu + JSDOM: https://github.com/lukeed/uvu/tree/master/examples/svelte

There's also Svelte Testing Library: https://testing-library.com/docs/svelte-testing-library/example/

@lukeed
Copy link
Member

lukeed commented Dec 5, 2020

The jsdom + svelte example works well. I have clients running that today in production.

There's also a PR in uvu repo about adding preprocessor support to the example. It's on my todo list to fix that up a bit, but it'll be another good example to point at.

For the record, uvu browser testing story will be its own package, which auto invokes playwright for you. I'm playing with it still to make it as lean as possible, and set it up in a way such that you can auto-rerun tests when related source files change. That may be a bit too lofty of a goal, but was something I was shooting for regardless. Might also add a uvu.jsdom library that, when -rd, will setup the necessary globals so that the uvu example is even less boilerplate-y.

Also, not certain, but I've heard people adding svelte-testing-library to the example and having it just work. Haven't tried myself since the helpers included in example effectively supplant STL entirely.

@pngwn
Copy link
Member

pngwn commented Dec 5, 2020

The main value of testing library is the neat semantic helpers, it has a few benefits. But being able to do getByRole('button') has slightly better ergonomics than using query selectors, it also prevents you from relying on classes or IDs (although that fragility could be addressed by using data-test ids in any system).

However, it would be easy for users to modify the env file in the example have pretty much the same ergs as testing-library if that is what users wanted.

@ehrencrona
Copy link
Contributor

I spent a few hours trying the various testing approaches.

svelte-testing-library works, but to support preprocessing it needs to spin up a new node instance for each source file which seems like it should give terrible performance.

The uvu pull request that adds preprocessor support borrows the same code. I think this is because the transformation APIs they rely on are synchronous, whereas the preprocessor is async, and they use execSync to work around this. Something like deasync might be an alternative?

The uvu solution also relies on require.extensions, which has been deprecated since Node 0.10.6. But maybe this doesn't cause any actual problems.

I also tried the approach of https://dev.to/d_ir/introduction-4cep that uses Rollup to build the tests. He seems to have a lot of problems with ES6 vs CJS modules and indeed his repository no longer seems to work. I get an error related to ES6 modules that I could not diagnose.

@dummdidumm
Copy link
Member

Possible deasync alternative: https://github.com/sindresorhus/make-synchronous

@pngwn
Copy link
Member

pngwn commented Dec 7, 2020

That looks similar to an approach I’ve taken before, probably similar to what like is going to dig up: https://github.com/pngwn/svelte-test/blob/master/transform.js

@benmccann
Copy link
Member

svelte-preprocess only needs to be async for postcss and less. I wonder if for the tests we could disable css processing and just call the preprocessor syncronously? (also requested in svelteness/svelte-jester#20 (comment))

For jest, it looks like there's an issue jestjs/jest#9504 and PR jestjs/jest#9889 to support async transformers. I wonder if that's something that could be added to uvu as well?

@dummdidumm
Copy link
Member

svelte-preprocess also asynchronously loads the required dependencies (like await("typescript"))

@ehrencrona
Copy link
Contributor

deasync just hangs for me; not sure why.

make-synchronous doesn't seem like it would work for our use case:

no other code will execute (not even async code) until the given async function is done.
The given function is executed in a subprocess, so you cannot use any variables/imports from outside the scope of the function.

(I get some error about it trying to serialize the function and failing)

Anyway, all these async-to-sync solutions are kludges; I think @benmccann 's suggestion of looking into making preprocessing synchronous seems like the best way forward.

I wonder if that's something that could be added to uvu as well?

uvu uses the Node API require.extensions, which is not well documented, but I think it's synchronous. After all, require is normally synchronous; that's probably the reason why all these transformations need to be.

@dummdidumm
Copy link
Member

Making the preprocess synchronous will not work for all preprocessors I think. For the 90% case of typescript it will though. But the interface of preprocess is marked as being asynchronous, but then we require people to make it synchronous again, which feels wrong. I don't know, maybe the execSync is the "cleanest" for now. When using uvu, its speed hopefully makes up for the small performance hit of starting a new process everytime.

@ehrencrona
Copy link
Contributor

svelte-preprocess also asynchronously loads the required dependencies (like await("typescript"))

I tried replacing this with a synchronous require and it seems to work. Do we really need to load this dynamically in the first place?

@dummdidumm
Copy link
Member

It needs to be done dynamically (of sync or async is another question) because theoretically it can handle a lot of preprocessors, but if you statically require/import them, you would need to install all of them even if you only use one.

@benmccann
Copy link
Member

Why does the testing library need to invoke the preprocessor? Can we just compile the code before running the tests?

@dummdidumm
Copy link
Member

I thought about this, too in the sense of svelte.compile(svelte.preprocess(fs.loadFile(..))) but I think the problem is that this would only work as long as the component is completely indepent. As soon as it imports other components, we need to resolve and compile those, too, at which point we are in bundler-land.

@ehrencrona
Copy link
Contributor

That would certainly sidestep all these issues with preprocessing. https://dev.to/d_ir/setting-up-a-svelte-test-environment-2f3e sets up a new Rollup build just for the tests that he configures to have multiple entry points. He seems to have had some challenges with it (see under "Rollup configuration") and it all goes a bit over my head. I guess it might be a bit confusing to have two different builds.

@pngwn
Copy link
Member

pngwn commented Dec 8, 2020

Could snowpack do the heavy lifting for us here? If we started the 'compile all files in watch mode' process but without the dev server parts? Then we just need to map the test imports back to those transpiled modules.

We already have everything setup in terms of preprocessors etc. We just need to consider any node/ browser inconsistencies.

@lukeed
Copy link
Member

lukeed commented Dec 8, 2020

I found my Jest + Svelte runner. It's similar to what @pngwn linked but for whatever reason I remember it being more consistent? Not sure why – haven't used it in a year or so

// package.json
{
  "transform": {
      "^.+\\.svelte$": "<rootDir>/.jest/svelte.js"
    },
}

// .jest/svelte.js
const { join } = require('path');
const { execFileSync } = require('child_process');
const runner = join(__dirname, 'runner.js');

// jest doesn't support async transforms
exports.process = (code, file) => {
  const output = execFileSync('node', [runner, file]);
  return JSON.parse(output); //=> { code, map }
}

// .jest/runner.js
const { resolve } = require('path');
const { promisify } = require('util');
const svelte = require('svelte/compiler');
const { readFile, existsSync } = require('fs');
const config = require('../svelte.config');

const read = promisify(readFile);

const [input] = process.argv.slice(2);

if (!input) {
  console.error('Did not receive a filename!')
  process.exit(1);
}

const filename = resolve('.', input);
if (!existsSync(filename)) {
  console.error(`File does not exist!\n>${filename}`);
  process.exit(1);
}

// Begin
async function compile() {
  const text = await read(input, 'utf8');
  const plain = await svelte.preprocess(text, config.preprocess, { filename });
  const output = svelte.compile(plain.code, {
    filename,
    format: 'cjs',
    // ...config.compilerOptions
    accessors: true,
    css: false,
    dev: true,
  });

  const { code, map } = output.js;

  // send back string
  return JSON.stringify({ code, map });
}

compile().then(console.log).catch(err => {
  console.error('Compile Error:', err);
  process.exit(1);
});

This should presumably work with any require hook. Only the exports.process API from .jest/svelte.js was Jest-specific.
I'll try getting this to work within uvu & report back with any findings.

That said, the "official" uvu browser adapter will be using a Snowpack-like approach for compiling test files on the fly, which would allow any normal setup w/ async transformers.

@pngwn
Copy link
Member

pngwn commented Dec 8, 2020

I've always found mine consistent too, but apparently it isn't. 'Works on my machine' probably.

@lukeed
Copy link
Member

lukeed commented Dec 8, 2020

It probably is! High chance I was doing something stupid.
Again, this is old & I can't speak to details/reasons why anymore. Just sharing so we have more to potentially work with.

@pngwn
Copy link
Member

pngwn commented Dec 8, 2020

Probably isn't! Wasn't a defence, more just pointing out that the approach could well be inconsistent.

@ehrencrona
Copy link
Contributor

svelte-preprocess only needs to be async for postcss and less. I wonder if for the tests we could disable css processing and just call the preprocessor syncronously?

I tried this out in sveltejs/svelte#5770 and it seems to work. Supporting both async and sync makes the code a bit messy, though. What do you think, is this a way forward?

@Rich-Harris
Copy link
Member Author

This feels like the wrong solution honestly. We should find a way to test the result of the snowpack pipeline (since a component might very well rely on modules that also need to be processed by snowpack) rather than incomplete scenario-solving

@Rich-Harris Rich-Harris modified the milestones: public beta, 1.0 Dec 13, 2020
@benmccann
Copy link
Member

benmccann commented Mar 14, 2021

Jest just merged support for async transforms, which means pre-processing without creating new processes will now be supported with the next release of Jest

@dominikg
Copy link
Member

dominikg commented Mar 14, 2021

I'm not entirely sure i understand what this issue is about but some thoughts crossed my mind that i'd like to share in hopes they will be helpful

  1. playwright-core without bundled browser
    to reduce download size for integration tests with playwright, you can use playwright-core and an existing chrome installation passed to it via CHROME_BIN env or by guessing the path https://github.com/svitejs/svite/blob/3a9e1a069450cd3fa009716192dc645802e5370e/scripts/jestGlobalSetup.js#L35

  2. use a browser server
    chromium.launchServer creates a service that can be reused across tests so they don't have to start/stop chrome all over again https://github.com/svitejs/svite/blob/3a9e1a069450cd3fa009716192dc645802e5370e/scripts/jestGlobalSetup.js#L59

  3. compile / transform with the tools we already have available
    Would it be possible to levarage a vite dev server instance for transforms via https://github.com/vitejs/vite/blob/26c46b92b2b5b21e21380459d1d1a76cb929a69a/packages/vite/src/node/server/index.ts#L192
    Or maybe a special vite-plugin-svelte instance that works standalone?

ps. that jest setup is mostly not my work. i adpoted it from vite and added some things like the guessing part

@Rich-Harris
Copy link
Member Author

This could be useful: https://github.com/microsoft/playwright-test

@kenkunz
Copy link

kenkunz commented Jun 7, 2021

(Sorry… I just realized I was auto-commenting on this ticket from my forked-repo commits. Ignore the first two above – I overwrote my history thinking it wouldn't be viewed by anyone else.)

I've had good experience using Modern Web's @web/test-runner with Svelte projects using both the Vite and Snowpack plugins. I created a POC SvelteKit plugin to see if a similar approach would work – this is still rough around the edges, but it demonstrates the basic idea: kenkunz/sveltekit 1d99ee1. To try it out, install the kenkunz/sveltekit, use create-svelte with the default template, run npm it from the new app directory.

I included sample tests for both a page component (that includes nested components) and a $lib component to show that this works for either case (tested with _Private components as well). I intentionally kept the dependencies to a minimum, but I've also tested this with Svelte Testing Library and it layers on top of this approach nicely.

There are several benefits to this approach that align with the concerns I saw raised in this thread:

  • relatively lightweight
  • un-opinionated about test framework (mocha is the default, but supports others)
  • un-opinionated about assertion library (I used npm assert in my examples, but you could use chai, jest or whatever)
  • can launch tests in a variety of browsers, including headless (uses locally installed Chrome by default, but supports Puppeteer, Playwright, Selenium and others off-the-shelf)
  • can directly test page components, private components, etc.

If you think this approach has merit, I would gladly work on turning it into a releasable version. Per my code comments, I think we'd want to extract & export a method from SvelteKit to start a dev server (from JS vs. cli), then create a real WTR SvelteKit plugin as a separate package.

If you don't think Web Test Runner is the right fit, please share some feedback as to why – I'm open to exploring alternatives. I think providing a working and flexible off-the-shelf testing solution would help with SvelteKit adoption. Personally, this was a potential barrier – having a proven/working testing solution has given me confidence to move forward with SvelteKit on my current project.

@rob-balfre
Copy link

I've been converting a component over to SvelteKit and as Rich's original post states, I had a question. 'how do we add tests?'

As Rich said last year:

... it's hard to know what testing setup to suggest when collectively we haven't really figured out what sort of things should live in a test suite

It might be as simple as being clear in the docs/FAQ that kit doesn't have opinions on tests (yet). Next.js just has a section about how to add tests using popular solutions, we could have something similar.

Otherwise we could find the new most asked question is about tests and no longer about routers 😄

@benmccann
Copy link
Member

I think Svelte Adders are a pretty good solution for this. There's one for Jest already and folks could create new ones for other testing solutions

@rob-balfre
Copy link

@benmccann thanks svelte-add looks like a good resource, I hadn't heard of it before.

I'm still leaning towards SvelteKit having an 'official' way of doing things though. It is after all the official framework for Svelte. Maybe just some basic examples in the docs and then let power users discover tools like svelte-add.

@benmccann
Copy link
Member

I think the problem with having "an official way" to do testing is that even among the maintainers we all have our favorite libraries and it will be hard to get consensus.

We do have an FAQ which mentions Svelte Adders. Perhaps we could make one specifically for testing if that'd make it more discoverable

There's also an FAQ on the main Svelte site about testing: https://svelte.dev/faq#how-do-i-test-svelte-apps

@rob-balfre
Copy link

Yes that's why I'm suggesting adding some examples on the official docs and not choose an official approach, sorry I wasn't very clear in my previous response.

So when people look for a testing solution for SvelteKit the docs can show some examples, including a section on using svelte-add. I personally never noticed the FAQ section, it's second nature for me to just read the docs.

@rmunn
Copy link
Contributor

rmunn commented Sep 7, 2021

One thing that Svelte-Kit probably needs to provide, or at least the FAQ should give recipes for, is some way to mock fetch in load functions. There are plenty of libraries that can let you replace methods on your components with mock versions of the methods, but it's not obvious to me how the fetch parameter of load functions can be mocked. (It's possible I'm missing something, of course).

I wonder if Svelte-Kit could provide a way to tell the compiler "Here's a mock fetch function that I want you to provide to my component instead of the real one." That would be very useful in integration tests. For unit testing, you could just compile the component to JS and call its load() function with parameters you provide, but for integration tests, it would be very nice to be able to hand the component a mock fetch function so that when it tries to fetch realapi.example.org/some/path it gets handed { "ok": true, "data": "some-test-data" } without ever hitting realapi.example.org at all.

Another subtlety around mocking fetch is its API: if your load function does const res = await fetch(url) and then const result = await res.json(), as most probably do, it's not always trivial to mock that. It would be nice if Svelte-Kit could provide some kind of scaffolding for mocking fetch, where you pass in a function that takes a URL string and returns a Javascript object, and the scaffolding would turn that into a function that returned the right kinds of promises for res.json() and res.text() and so on.

Just off-the-top-of-my-head ideas at this point.

@brev
Copy link
Contributor

brev commented Feb 16, 2022

Hi All, thanks for the great code and community.

I've come up with a new, simple, fast, more reliable way to test my SvelteKit site, using only:

Ideally, this will make for a new svelte-adder, somewhat like svelte-add-jest. I don’t have time to do this right now, but hope to later, and wouldn’t mind if someone else beats me to it.

Demo

Install a fresh SvelteKit demo app:

npm init svelte@next my-app  # demo app, +typescript, +eslint, +prettier
cd my-app
npm install

Install our additional packages:

npm install --save-dev \
  @testing-library/svelte \
  c8 \
  dotenv \
  esm-loader-css \
  esm-loader-import-alias \
  esm-loader-import-meta-custom \
  esm-loader-import-relative-add-extension \
  esm-loader-json \
  esm-loader-mock-exports \
  esm-loader-svelte \
  esm-loader-typescript \
  global-jsdom \
  jsdom \
  node-esm-loader \
  node-fetch \
  uvu

Create a .loaderrc.js config file for the awesome node-esm-loader library by @sebamarynissen. You can find code and documentation for each of these loaders in my esm-loaders monorepo:

// .loaderrc.js
import dotenv from "dotenv";
import { resolve } from "path";

dotenv.config({ path: "./.env.development" });

export default {
  loaders: [
    "esm-loader-svelte",
    "esm-loader-typescript",
    "esm-loader-css",
    "esm-loader-json",
    {
      loader: "esm-loader-import-alias",
      options: {
        aliases: {
          "$app/env": resolve(".svelte-kit/runtime/app/env.js"),
          "$app/navigation": resolve(".svelte-kit/runtime/app/navigation.js"),
          "$app/paths": resolve(".svelte-kit/runtime/app/paths.js"),
          "$app/stores": resolve(".svelte-kit/runtime/app/stores.js"),
          "$lib/": `${resolve("src/lib/")}/`,
          "$service-worker": resolve("src/service-worker.js"),
        },
      },
    },
    {
      loader: "esm-loader-import-meta-custom",
      options: {
        meta: {
          env: process.env,
        },
      },
    },
    {
      loader: "esm-loader-import-relative-add-extension",
      options: {
        extensions: {
          ".js": ".js",
          ".svelte": ".ts",
          ".ts": ".ts",
        },
      },
    },
    {
      loader: "esm-loader-mock-exports",
      options: {
        includes: [/svelte\/motion/],
      },
    },
  ],
};

We'll add a new test for the src/lib/Counter.svelte component:

// src/lib/Counter.test.ts
import * as assert from "uvu/assert";
import { cleanup, fireEvent, render } from "@testing-library/svelte";
import Counter from "./Counter.svelte";
import { _MOCK } from "svelte/motion";
import { suite } from "uvu";
import { writable } from "svelte/store";

import "global-jsdom/register";

const test = suite("Counter");

test("render", async () => {
  _MOCK("spring", writable); // mock spring motion for jsdom

  const { findByText, getByLabelText, queryByText } = render(Counter);

  const buttonAdd = getByLabelText("Increase the counter by one");
  const buttonSub = getByLabelText("Decrease the counter by one");
  assert.ok(buttonAdd);
  assert.ok(buttonSub);
  assert.ok(queryByText("0"));
  assert.not(queryByText("2"));

  await fireEvent.click(buttonAdd); // 1
  await fireEvent.click(buttonAdd); // 2
  await fireEvent.click(buttonAdd); // 3
  await fireEvent.click(buttonSub); // 2
  assert.ok(await findByText("2"));

  _MOCK.CLEAR();
  cleanup();
});

test.run();

And, we'll add a new test for the src/routes/todos/index.ts data endpoint, where we'll do a custom mock of the fetch() method (cc @rmunn):

// src/routes/todos/index.test.ts
import * as assert from "uvu/assert";
import fetch from "node-fetch";
import { get } from "./index";
import { suite } from "uvu";

globalThis.fetch = fetch;

const test = suite("TodosEndpoint");

test("get", async () => {
  globalThis.fetch = async () => ({
    ok: true,
    status: 200,
    json: async () => [
      {
        uid: "f3fafa29-31ec-41e5-8258-4af5ce938ed0",
        text: "hello",
        done: true,
      },
      {
        uid: "ad794a97-4f91-4861-a5ab-a52c657822b3",
        text: "hi",
        done: false,
      },
    ],
  });

  const result = await get({
    request: {},
    locals: {
      userid: 1234,
    },
  });

  assert.ok(result);
  assert.ok(result.body);
  assert.ok(result.body.todos);
  assert.is(result.body.todos.length, 2);

  globalThis.fetch = fetch;
});

test.run();

Finally, we run our new demo tests:

NODE_OPTIONS='--experimental-loader node-esm-loader' npx uvu src .test.ts

Setup C8 code coverage report config file .c8rc.json:

// .c8rc.json
{
  "all": true,
  "exclude": [".svelte-kit/**", "**/*.d.ts", "**/*.test.ts"],
  "extension": [".js", ".cjs", ".ts", ".svelte"],
  "reporter": ["text", "html"],
  "src": "src/"
}

Generate code coverage reports into coverage/ subdirectory:

NODE_OPTIONS='--experimental-loader node-esm-loader' npx c8 uvu src .test.ts

Notes

Thanks for any feedback!

cc @jerrythomas

@Rich-Harris
Copy link
Member Author

closed via #4056

@fredguth
Copy link

fredguth commented Jul 4, 2022

I found this thread by looking for a playwright usage with sveltekit example.

As of today, if one npm create svelte my-app and choose ...demo app and Add Playwright..., my-app is created with playwright config files and a tests/tests.ts file. Unfortunately, though, the tests.ts does not fully test the created app. svelte/realworld also does not test the app.

As Rich said:

If the app template doesn't include tests, then one of the first questions people will ask is 'how do we add tests'? We should probably have some way of answering this question

Although teaching how to use Playwright is out of scope of what the svelte community should provide, wouldn't it be helpful to point to a good working test example of a svelte-kit app? If someone can point me some good example I would happily contribute by adding tests to the demo app.

@benmccann
Copy link
Member

@fredguth it's not clear to me what you were hoping would be tested. We demonstrate how to setup and configure things, which is the hard part. What exactly is missing?

@fredguth
Copy link

fredguth commented Jul 6, 2022

As someone who has never seen Playwright, I was expecting tests/test.js would be a complete test of the demo app. For example, testing not only the about page, but the todo app in the /todos route. Maybe it is out of scope, but I think this kind of detail helps newcomers to SvelteKit.

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

No branches or pull requests