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

Fix/jest esm babel #30

Merged
merged 5 commits into from
Sep 7, 2021
Merged

Fix/jest esm babel #30

merged 5 commits into from
Sep 7, 2021

Conversation

patrickleet
Copy link
Member

No description provided.

@benmccann
Copy link

Hmm. I don't know why Jest is always trying to load the .cjs version. It also looks like it's hardcoded to use transformSource and never uses transformSourceAsync. I think I'd probably report this as a bug to Jest and see if there's something that can be fixed there

https://github.com/facebook/jest/blob/bc50e7f360ab1845abbaa0b3ad788caead0d3174/packages/jest-reporters/src/generateEmptyCoverage.ts#L72

@patrickleet
Copy link
Member Author

I think the require error is from jest.mock jestjs/jest#10025

@benmccann
Copy link

The thing about CJS / processAsync is coming from the coverage stuff. If you remove the coverage config from jest.json then those errors disappear. We should file a bug with Jest about that

Even with the coverage stuff removed the tests still fail though since the SvelteKit stuff hasn't been mocked due to lack of mocking support in ESM

@benmccann
Copy link

Hmm. I created a new project using coverage and svelte-jester 2.0 and everything's passing (https://github.com/benmccann/svelte-jest-add-example). I'm not sure what's different to be causing the failure here

@patrickleet
Copy link
Member Author

Yea I played around with it more and got past that error but still unable to get mocks working correctly for anything beyond a basic example. which seems more like a jest/esm issue.

@benmccann
Copy link

Yeah, we'd need Jest to fix that. Maybe you could try this workaround in the meantime? jestjs/jest#10025 (comment)

@patrickleet
Copy link
Member Author

@benmccann

I got a version of this working - will share pending some clean up, but used strategy from sveltejs/kit#1485 (comment)

collectCoverageFrom is what's causing those coverage errors - without it, there are not errors, however, it doesn't know which files to collect coverage from which throws it off a lot.

Signed-off-by: Patrick Lee Scott <pat@patscott.io>
…m esbuild-jest

Signed-off-by: Patrick Lee Scott <pat@patscott.io>
Signed-off-by: Patrick Lee Scott <pat@patscott.io>
Signed-off-by: Patrick Lee Scott <pat@patscott.io>
Signed-off-by: Patrick Lee Scott <pat@patscott.io>
@codecov
Copy link

codecov bot commented Sep 7, 2021

Codecov Report

Merging #30 (38dae5d) into master (715e4e3) will decrease coverage by 1.67%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #30      +/-   ##
==========================================
- Coverage   88.08%   86.40%   -1.68%     
==========================================
  Files          10       10              
  Lines         235      206      -29     
  Branches       17       17              
==========================================
- Hits          207      178      -29     
  Misses         28       28              
Impacted Files Coverage Δ
src/routes/oto.svelte 66.66% <0.00%> (-7.25%) ⬇️
src/routes/upsell.svelte 66.66% <0.00%> (-7.25%) ⬇️
src/routes/downsell.svelte 66.66% <0.00%> (-7.25%) ⬇️
src/routes/index.svelte 81.25% <0.00%> (-4.47%) ⬇️
src/routes/checkout.svelte 83.33% <0.00%> (-3.63%) ⬇️
src/routes/thank-you.svelte 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 715e4e3...38dae5d. Read the comment docs.

@patrickleet
Copy link
Member Author

@benmccann pushed

I guess the combination of collectCoverageFrom and actual importing and testing things is enough for the coverage errors to go away - I now only see it for __layout, the only file I'm not explicitly testing

Running coverage on untested files...Failed to collect coverage from /Users/patrickleet/dev/cloudnativeentrepreneur/funnel/src/routes/__layout.svelte
ERROR: Jest: synchronous transformer /Users/patrickleet/dev/cloudnativeentrepreneur/funnel/node_modules/svelte-jester/dist/transformer.cjs must export a "process" function.
STACK: Error: Jest: synchronous transformer /Users/patrickleet/dev/cloudnativeentrepreneur/funnel/node_modules/svelte-jester/dist/transformer.cjs must export a "process" function.
    at invariant (/Users/patrickleet/dev/cloudnativeentrepreneur/funnel/node_modules/@jest/transform/build/ScriptTransformer.js:1092:11)
    at assertSyncTransformer (/Users/patrickleet/dev/cloudnativeentrepreneur/funnel/node_modules/@jest/transform/build/ScriptTransformer.js:1098:3)
    at ScriptTransformer.transformSource (/Users/patrickleet/dev/cloudnativeentrepreneur/funnel/node_modules/@jest/transform/build/ScriptTransformer.js:611:7)
    at _default (/Users/patrickleet/dev/cloudnativeentrepreneur/funnel/node_modules/@jest/reporters/build/generateEmptyCoverage.js:143:38)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)

This error is just for show though - doesn't actually break anything

@patrickleet patrickleet merged commit 1b38af0 into master Sep 7, 2021
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

Successfully merging this pull request may close these issues.

None yet

2 participants