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

Unit test failed when component has svelte-multiselect #48

Closed
wd-David opened this issue Feb 26, 2022 · 8 comments · Fixed by #54
Closed

Unit test failed when component has svelte-multiselect #48

wd-David opened this issue Feb 26, 2022 · 8 comments · Fixed by #54
Labels
discussion Gathering feedback on open questions docs Improvements or additions to documentation dx Developer experience

Comments

@wd-David
Copy link
Contributor

Hi @janosh, I like to thank you for creating such a great component! I love it!

Everything works great so far (v4.0.0) in my svelte app until I tried to add some unit tests.
Test failed when I tested component which has svelte-multiselect, Jest threw error (Vitest broke right away) like this:
Screen Shot 2022-02-26 at 12 45 38 PM

I'd already setup transformers so it's not a ESM problem in Jest, and other tests passed with different svelte libraries.

Here is a reproducible repo that you can check: https://github.com/davipon/test-failed
I simply import svelte-tiny-virtual-list & svelte-multiselect in the same test file and try to render them.
Test would fail before rendering Multiselect:

test('render multiselect', () => {
  render(Multiselect, { options: webFrameworks });
});

I'm not familiar with creating Svelte package, but I found a difference in these two libraries' index.js:
svelte-tiny-virtual-list:
Screen Shot 2022-02-26 at 1 02 41 PM

svelte-multiselect:
Screen Shot 2022-02-26 at 1 04 51 PM

Not sure if information above can help, and thanks again for your hard work!

@janosh janosh added bug Something isn't working question Further information is requested labels Feb 26, 2022
@janosh
Copy link
Owner

janosh commented Feb 26, 2022

@davipon Thanks for the detailed analysis! The error is a bit odd though. I have some local vitests that aren't quite ready to be pushed yet but those run fine whether I import from the component file or from index.(j|t)s.

// tests/multiselect.test.ts
import { tick } from 'svelte'
import { expect, test } from 'vitest'

// all of these work for me
import MultiSelect from '../src/lib/MultiSelect.svelte'
import MultiSelect from '../src/lib' // this imports from index.ts
import MultiSelect from '../package/index' // this imports from index.js
import MultiSelect from '../package/MultiSelect.svelte'

test(`mount component`, async () => {
  const options = [`Banana`, `Watermelon`, `Apple`, `Dates`, `Mango`]
  const instance = new MultiSelect({
    target: document.body,
    props: { options },
  })
  expect(instance).toBeTruthy()

  const lis = document.body.querySelectorAll(
    `div.multiselect > ul.options > li`
  )
  expect(lis.length).toEqual(5)
  lis.forEach((li, i) => {
    expect(li.textContent).toContain(options[i])
  })
})

This is my vite.config.ts:

import { svelte } from '@sveltejs/vite-plugin-svelte'

export default {
  plugins: [svelte({ hot: !process.env.VITEST })],
  test: {
    environment: `jsdom`,
  },
}

I'm happy to change

- export { default } from './MultiSelect.svelte'
+ export { default as default } from './MultiSelect.svelte'

if that's necessary. Could you do me a favor and actually go into node_modules/svelte-multiselect/index.js and make this change to see if it really solves your problem? I'd also like to find out why your and my vitest seem to behave differently.

@wd-David
Copy link
Contributor Author

I did try this but unfortunately the test still failed.

export { default as default } from './MultiSelect.svelte

Here are approaches I tried to find the root cause:

  1. Cloned the source code and directly tested your tests/multiselect.test.ts, test passed with different import.
  2. Back to my repo and copy package from node_modules to the project root like this:

Screen Shot 2022-02-26 at 10 15 15 PM

3. Use 3 different **import** for testing, all tests failed in Jest, last import passed in Vitest:
// import MultiSelect from 'svelte-multiselect'; // [FAILED]: import from node_modules
// import MultiSelect from 'svelte-multiselect/MultiSelect.svelte'; // [FAILED]: import from node_modules/svelte-multiselect/MultiSelect.svelte
import MultiSelect from '../svelte-multiselect'; // [PASSED]: Directly copy the package to the root of the project

test(`mount component`, async () => {
	const options = [`Banana`, `Watermelon`, `Apple`, `Dates`, `Mango`];
	const instance = new MultiSelect({
		target: document.body,
		props: { options }
	});
	expect(instance).toBeTruthy();

	const lis = document.body.querySelectorAll(`div.multiselect > ul.options > li`);
	expect(lis.length).toEqual(5);
	lis.forEach((li, i) => {
		expect(li.textContent).toContain(options[i]);
	});
});

I've updated my reproducible repo, please kindly try: https://github.com/davipon/test-failed


I was using Vitest 0.5.5 yesterday and failed the test without any informative error, but it started to yell after upgrade to 0.5.7 today:
Screen Shot 2022-02-26 at 10 32 49 PM

It's weird because svelte ext should already been configured in vitest.config.js, and test did pass when I imported it from the one copied from node_modules.
So I'm wondering if it's related to packaging in SvelteKit, I tried to test different Svelte libraries but hard to find a pattern.
Some libraries are still using rollup and some are just Svelte action libraries without actual svelte file.

@janosh
Copy link
Owner

janosh commented Feb 26, 2022

The problem seems to be that imports from node_modules in tests are not handled by Svelte preprocessor.

Maybe this SO answer helps? Does it work when you exclude svelte-multiselect from transformIgnorePatterns?

@wd-David
Copy link
Contributor Author

wd-David commented Feb 27, 2022

Thanks @janosh, it works after adding transformIgnorePatterns in jest.config.json:

{
  "testEnvironment": "jsdom",
  "transformIgnorePatterns": ["node_modules/?!(svelte-multiselect)"],
  "transform": {
    "^.+\\.[t|j]s?$": "esbuild-jest",
    "^.+\\.svelte$": ["svelte-jester", { "preprocess": true }]
  }
}

While test still failed in Vitest, I couldn't find the corresponding config in Vitest so far, will raise a ticket there.


Since SvelteKit wouldn't build code when packaging, also transformers like babel-jest, ts-jest, svelte-jester wouldn't transpile files within node_modules (like this and this), this seems to be a general problem for future Svelte ecosystem when it comes to unit testing.

I think it would be great if we can add a section in README for people who will encounter the same problem, like recommended Jest config for unit testing, can also add Vitest config if there is a solution in the future.
I can create a PR for that if you think it's OK. Thanks again for your time!

@janosh
Copy link
Owner

janosh commented Feb 27, 2022

While test still failed in Vitest, I couldn't find the corresponding config in Vitest so far, will raise a ticket there.

Thanks. Could you link this issue over there? I'd like to subscribe to your vitest issue.

this seems to be a general problem for future Svelte ecosystem when it comes to unit testing.

Indeed which is why I'm very curious to hear if there's a recommended solution.

I can create a PR for that if you think it's OK.

Sure that would be great!

@janosh janosh added discussion Gathering feedback on open questions docs Improvements or additions to documentation dx Developer experience and removed bug Something isn't working question Further information is requested labels Feb 27, 2022
@janosh
Copy link
Owner

janosh commented Feb 27, 2022

Based on vitest-dev/vitest#809, https://vitest.dev/config/#transformmode might be something to play around with.

@wd-David
Copy link
Contributor Author

wd-David commented Feb 28, 2022

I also tried transformmode but it failed as well.

Anyway, I asked in Vitest Discord channel and got the answer!
We need to use dep.inline in Vitest:

test: {
  globals: true,
  environment: 'happy-dom',
  deps: {
    inline: [/svelte-multiselect/]
  }
}

Now we know how to deal with transpiling errors when unit testing most of the Svelte component packages (I believe) in both Jest and Vitest, I'll create a PR to add a Unit Test section in README with examples.

Thanks again @janosh, your comments really helped me to dig in and find the root cause, please kindly close this issue, have a good one!

@janosh
Copy link
Owner

janosh commented Mar 1, 2022

Cool! Let's close this issue once your PR to update the readme is merged.

@janosh janosh closed this as completed in #54 Mar 1, 2022
janosh added a commit that referenced this issue Mar 1, 2022
…nstream testing (#54)

* Add Unit Test & stackblitz link in README (closes #48)

* bit more concise wording

Co-authored-by: Janosh Riebesell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Gathering feedback on open questions docs Improvements or additions to documentation dx Developer experience
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants