Added Option to Automatically Add Vitest to New Projects using svelte-create

[bertybot]

Issue

Getting started with unit testing in Sveltekit can be confusing, which was brought up in this issue here https://github.com/sveltejs/kit/issues/4423

Resolution

• Added the ability to automatically add Vitest with example tests to new projects using the svelte-create package. This should help people new to unit tests in Sveltekit get started with reasonable defaults by scaffolding the configuration for them, and giving them example tests to start out with.

Testing

• Manually tested this with every config to make sure it worked.

Possible problems

• I have not tested this Vitest config when emulating Sveltekit specific behavior such as $app/env and $app/stores I might need input from @benmccann on this one...

[changeset-bot[bot]]

🦋 Changeset detected

Latest commit: 01ab4ba765bdba9f08a8ac70593ddcb06aede61b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
create-svelte	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[dominikg]

Do we want to add c8 + coverage script by default? I think vitest asks you to install c8 if you run --coverage without it. Adding it by default means every user gets it, even if they are not interested in it.

[bertybot]

Do we want to add c8 + coverage script by default?

I think vitest asks you to install c8 if you run --coverage without it. Adding it by default means every user gets it, even if they are not interested in it.

Yea, this is true. Do you think I should just remove the coverage script as well?

[dominikg]

I'd skip it until we have an idea/documentation to actually build a full coverage test suite, which may need an additional util for mocking kit things as required.

Eg. hooks, page-endpoints and maybe even some parts of .svelte files are very difficult to test right now.

So a coverage report by c8 would be very inaccurate, either by giving a false sense of security when showing 100% even though it's not seeing everything, or by showing 50% and the user has no way of getting a higher coverage because tools are missing to implement tests.

[bertybot]

I'd skip it until we have an idea/documentation to actually build a full coverage test suite, which may need an additional util for mocking kit things as required.

Eg. hooks, page-endpoints and maybe even some parts of .svelte files are very difficult to test right now.

So a coverage report by c8 would be very inaccurate, either by giving a false sense of security when showing 100% even though it's not seeing everything, or by showing 50% and the user has no way of getting a higher coverage because tools are missing to implement tests.

Removed coverage tests

[dominikg]

there are rudimentary examples with and without testing-library-svelte in https://github.com/vitest-dev/vitest/tree/main/examples/svelte/test

I think adding testing-library for component tests is a better suggestion than everyone coming up with their own set of helpers.

[benmccann]

I don't have any objection to testing-library-svelte. But what I am worried about is adding a component test when it's not at all clear to me whether components are better tested with Vitest or Playwright. See the discussion in https://github.com/sveltejs/kit/discussions/5285 for more details

[bertybot]

I think adding testing-library for component tests is a better suggestion than everyone coming up with their own set of helpers.

Agreed, to give you my perspective. My team has been using testing-library for months now, and it is probably one of the best unit testing experiences I've had, at least on the frontend. I also think testing html content is a bad practice and should be avoided and that seems to be the only way to test Svelte without helpers. I don't think we should be encouraging a bad practice like that.

I do totally get the want to stick to one test helper though. Using, the playwright one and the testing-library one at the same time would probably be confusing and overcomplicated.

My suggestion is to embrace the controversy, that's at least what I've seen other frameworks do. Let them have the option of no helper (just js files), testing-library, or playwright. It allows people to pick their preference based on their use case and goals. I do get though how this would increase the maintenance cost of this relatively small feature a ton. So we might just want to keep it as is and allow people to install their own. Once this is merged I'll make sure to submit updates to the relevant documentation on testing library and playwright.

I really want to say we should make playwright the default but, the fact component testing is still experimental and locked in an experimental package makes it a deal breaker for me, at least for now....

[manuganji]

I'm using the newly recommended tests directory for keeping my tests and I have a few components inside. But I can't import them into my tests. I went looking through this template for guidance on how to write a component test.

Error: [vite-node] Failed to load /tests/components/TwoSubmitButtons.svelte

Should I make a separate issue for this or do you think it's solvable with the right vite.config::test settings? If you know which setting, could you please point me to it? This is the testing code in question:

import Form from '$lib/components/Form.svelte';
import TwoSubmitButtonsSvelte from './components/TwoSubmitButtons.svelte';
import { findByTestId, fireEvent, render, within } from '@testing-library/svelte';
import type { SpyInstance } from 'vitest';

describe('Test', () => {
  it('should render children buttons', () => {
    const props = { schema: {} };
    const { container } = render(TwoSubmitButtonsSvelte);
    expect(container.querySelectorAll('button[type=submit]')).toHaveLength(2);
  });
});

[bertybot]

I'm using the newly recommended tests directory for keeping my tests and I have a few components inside. But I can't import them into my tests. I went looking through this template for guidance on how to write a component test.

Error: [vite-node] Failed to load /tests/components/TwoSubmitButtons.svelte

Should I make a separate issue for this or do you think it's solvable with the right vite.config::test settings? If you know which setting, could you please point me to it? This is the testing code in question:

import Form from '$lib/components/Form.svelte';

import TwoSubmitButtonsSvelte from './components/TwoSubmitButtons.svelte';

import { findByTestId, fireEvent, render, within } from '@testing-library/svelte';

import type { SpyInstance } from 'vitest';



describe('Test', () => {

  it('should render children buttons', () => {

    const props = { schema: {} };

    const { container } = render(TwoSubmitButtonsSvelte);

    expect(container.querySelectorAll('button[type=submit]')).toHaveLength(2);

  });

});

I don't think Vite compiles svelte in the tests directory just the src. So, it won't be able to find svelte files in the tests folder. For individual component tests I would recommend putting them with the source code.

[Rich-Harris]

@manuganji please open a new issue for things like this — questions like that don't belong on a PR discussion. thanks

[benmccann]

I pushed some updates, so this should hopefully be close to being able to be merged. The one remaining item is @dominikg's comments about global types: https://github.com/sveltejs/kit/pull/5708#pullrequestreview-1051421077. Assuming we want to use globals, perhaps someone more familiar with TypeScript could add the types

[bertybot]

I pushed some updates, so this should hopefully be close to being able to be merged. The one remaining item is @dominikg's comments about global types: #5708 (review). Assuming we want to use globals, perhaps someone more familiar with TypeScript could add the types

THANKS @benmccann!!!

If its a huge problem then I think we should just leave off globals for now as @dominikg said it's cleaner anyway.

[bertybot]

removed the vitest globals in favor of named imports as suggested by @dominikg

while doing this I took the time to extensively test every configuration I found a bug adding two vite configs to every project which I fixed by removing the -vitest vite.config and renaming the +vitest vite.config.js to vite.config.ts

@benmccann I also fixed a few type errors that broke the Game test that you should check out. I just had to add a default property to the constructor and split the string out in to a char array in the test. These work but, they were kind of quick fixes so feel free to update to something more your style.

[dominikg]

I don't recommend using vite.config.ts . The way vite creates a temporary vite.config.hash.js isn't great imho.

[dominikg]

If we suggest colocating test files, there should be documentation about how to handle + prefixed files. See https://github.com/sveltejs/kit/issues/7121

[bertybot]

I don't recommend using vite.config.ts . The way vite creates a temporary vite.config.hash.js isn't great imho.

Updated and renamed vite.config.ts to vite.config.js and tested.

[dominikg]

unit test files are colocated in src, do we need to update tsconfig.json or show how that works at least so that users can have different ts settings for unit vs app code? or should the move it out of src in that case?

I'd really love to avoid going down the multiple tsconfig files road but i'm not sure you can have both playwright and vitest .spec.ts files with different gobal needs from a single tsconfig

[benmccann]

@dominikg why would you need different TypeScript settings for unit vs app code?

[dominikg]

@dominikg why would you need different TypeScript settings for unit vs app code?

eg node env vs not

[benmccann]

Isn't that just defined once per config? If you want multiple, I think you have to have multiple configs regardless of what directories things live in? I don't have a ton of experience writing tests in TypeScript projects, so I'm not understanding what alternative is being proposed and if there's some best practice around this that you think this PR might be violating?

[dominikg]

it depends, for basic unit tests sharing the same tsconfig works. but I've seen multiple projects splitting it into a separate tsconfig.test.json with include and exclude. due to the way kit generates the root tsconfig that could become difficult.

we can start with this setup but advanced usecases will require additional setup which isn't clear and there is no docs about it.

[dominikg]

is https://github.com/vitest-dev/vitest/issues/2298 a blocker here? not supporting these makes some unit tests harder/ impossible

related: https://github.com/sveltejs/kit/issues/6259

[bertybot]

is https://github.com/vitest-dev/vitest/issues/2298 a blocker here? not supporting these makes some unit tests harder/ impossible

related: https://github.com/sveltejs/kit/issues/6259

I think we agreed to basic unit testing to start giving the ability to configure to their projects needs as it grows.

This issue seems semi problematic but, could we not use mocks to get around this? I know in the issue the they use mockImport() which automatically makes a mock but couldn't for now they just use .mock() and do it by hand does that not work?

[Rich-Harris]

thanks all! very happy to get this in

[bertybot]

Thanks everyone!!!