Tests and lint issues about skeleton HOT 13 CLOSED

Okay, this one was bugging me and I needed to get to the bottom of it.

It's because I use pnpm... BUT
https://www.kochan.io/nodejs/pnpms-strictness-helps-to-avoid-silly-bugs.html

Substitute @testing-library/dom for the debug@1 in the article to understand the issue. Because it is not listed in skeletons dependencies, even though it directly uses it, it is at the 'mercy' of testing library who can change/remove it. In this case it is very trivial and small and not likely to have major flow on effects.

Also, npm for some reason is also happy to install the deprecated package without even warning you of such, which will cause an issue at some point.

pnpm i on the other hand lets you know that something is up and you should be looking into it.

I also don't like wasting 200mb per project by having exact copies of files in each projects node_modules folder.

So I'm going to propose, properly adding the @testing-library/dom to package.json to resolve the pnpm DX and we can sort out the deprecated testing package in another issue.

That work for you guys ?

from skeleton.

Tests were working fine last I checked (typically run at least theme daily), but I'll review. And yes, we've been doing a bad job keeping up with linting. Definitely something to be addressed soon. I'd assume most are minor issues.

from skeleton.

FYI we'll keep this ticket in regards to the testing issue indicated above. I've created another ticket here to address linting: #70

from skeleton.

@niktek If you won't mind, could you share more details about the issues you encountered about the tests? Screenshots or copy/paste would be fine.

from skeleton.

Hey @thomasbjespersen,
Okay, so on a fresh fork, cd'd to the root of the project folder:

pnpm i
vitest-svelte-kit 0.0.6 (0.0.7 is available) deprecated
pnpm dev
The request url "/Users/nik/code/github/skeleton/.svelte-kit/runtime/client/start.js" is outside of Vite serving allow list.
pnpm test
Error: [vite-node] Failed to load @testing-library/dom

There is only @testing-library/svelte in package.json and no @testing-library/dom in the devDependncies so that one is quick to fix. I imagine vitest-svelte-kit can be removed due to it all being native to Vite now (adding vitest on a new project didn't pull in the vitest-svelte-kit either)?

I need to look into the outside of Vite serving allow list a bit more

from skeleton.

Looks like they are all resolved with this: #74

from skeleton.

@niktek - Hey Nik! I have been unable to replicate this from my side. Please comment on this ticket if further issues should arise.. We can always reopen if needed.

from skeleton.

This cannot be reproduced under regular circumstances..

from skeleton.

Tests were working fine.. Closed..

from skeleton.

I'm still able to repro this

If you look at:
https://github.com/Brain-Bones/skeleton/blob/master/package.json
There is no @testing-library/dom, so it is impossible for it to work without doing a manual install of that library. Can you check that you don't have @testing-library/dom installed as a global package (npm list -g) from multiple projects maybe ?

And - https://www.npmjs.com/package/vitest-svelte-kit it's not meant to be used any more.

If you just delete your node_modules folder and do npm i then you will at least see the deprecation warning and removing the package will show the need for the fixes in vitest.config.js

from skeleton.

@thomasbjespersen @niktek Given Nik is the first contributor to pull down the project and run the test suite, I'd like to troubleshoot what's going on. Especially since he's able to reproduce. I'll investigate on my end and respond with my findings asap.

Just to confirm, I don't have the testing-library DOM package installed globally. To my knowledge we're only using JSDOM, so that could be part of the problem

I had a suspicion the vitest-svelte-kit package is no longer needed, but we really need to devote some time the Vitest setup to ensure everything is up to date and properly configured.

Again, I'll follow up soon after a more thorough investigation.

from skeleton.

@niktek FYI I've had a chance to run through and test this again. Steps to reproduce:

1. Pull on my local master branch to ensure it's current
2. Run npm run test to run the test runner

No errors are present. Everything passes as expected:

Perhaps reach out in the Contributions channel on Discord to see if others can replicate on their end? Help eliminate it being something special me and Thomas have done.

Otherwise we'll have to chalk it up as as an isolated issue for yourself and close the ticket. I'll wait until you follow up before I do that though.

from skeleton.

I'm all for this. I think this will tie in well with the Vitest config updates we've been brainstorming, but I'm fine with it happening sooner.

from skeleton.