feat(test): jest 28 support #4979

rwaskiewicz · 2023-10-24T21:07:05Z

What is the current behavior?

Stencil lacks support for Jest 28
GitHub Issue Number: Fixes: #3348

Note to Reviewers

This PR is meant to follow along with the steps for adding a new version of Jest, currently ready for review (#4952). I suggest reading that in tandem with this PR, each commit in this PR can be directly tied back to a step (there may be >1 commit per step) in that document.

What is the new behavior?

Add support for Jest 28 to Stencil

Does this introduce a breaking change?

Yes
No

Testing

To test, I generated a dev build of the branch - @stencil/core@4.6.0-dev.1698235669.413d292

Component Starter Testing

I tested this on a Stencil Component Starter (manually).

I started by generating a new project with the dev build:

cd /tmp \
&& npm init stencil@latest component jest-test \
&& cd $_ \
&& npm i @stencil/core@4.6.0-dev.1698235669.413d292

Then installed each version of jest and tested them:

npm i jest@24 jest-cli@24 @types/jest@24 && npm t -- --no-cache && \
npm i jest@25 jest-cli@25 @types/jest@25 && npm t -- --no-cache && \
npm i jest@26 jest-cli@26 @types/jest@26 && npm t -- --no-cache && \
npm i jest@27 jest-cli@27 @types/jest@27 && npm t -- --no-cache && \
npm i jest@28 jest-cli@28 @types/jest@28 && npm t -- --no-cache

Ionic Framework

I tested this branch in the Ionic Framework as well.

Note: Node 18 is required to run tests here.

Pull the latest Ionic Framework code, check out main
cd core/ && npm i @stencil/core@4.6.0-dev.1698235669.413d292
npm i jest@28 @types/jest@28 jest-cli@28 @jest/core@28
npm run test.spec -- --no-cache
At this point, an error should appear
Apply the following patch:

diff --git a/core/src/components/datetime/test/data.spec.ts b/core/src/components/datetime/test/data.spec.ts
index 4484224402..0306d09f9c 100644
--- a/core/src/components/datetime/test/data.spec.ts
+++ b/core/src/components/datetime/test/data.spec.ts
@@ -482,7 +482,7 @@ describe('generateTime()', () => {
 
 describe('getToday', () => {
   beforeAll(() => {
-    jest.useFakeTimers('modern');
+    jest.useFakeTimers();

Rerun the tests, everything ought to pass now 👍

Other information

Docs PR: ionic-team/stencil-site#1257

Note to self When it's time to merge, please squash this, add the STENCIL- ticket number and the 'fixes' contents to the squashed commit

github-actions · 2023-10-24T21:08:57Z

`--strictNullChecks` error report

Typechecking with --strictNullChecks resulted in 1393 errors on this branch.

That's the same number of errors on main, so at least we're not creating new ones!

reports and statistics

Our most error-prone files

Path	Error Count
src/dev-server/index.ts	37
src/mock-doc/serialize-node.ts	36
src/dev-server/server-process.ts	32
src/compiler/build/build-stats.ts	27
src/compiler/output-targets/dist-lazy/generate-lazy-module.ts	25
src/compiler/style/test/optimize-css.spec.ts	23
src/testing/puppeteer/puppeteer-element.ts	23
src/compiler/prerender/prerender-main.ts	22
src/runtime/vdom/vdom-render.ts	20
src/runtime/client-hydrate.ts	19
src/screenshot/connector-base.ts	19
src/compiler/config/test/validate-paths.spec.ts	16
src/dev-server/request-handler.ts	15
src/compiler/prerender/prerender-optimize.ts	14
src/compiler/sys/stencil-sys.ts	14
src/compiler/transpile/transpile-module.ts	14
src/runtime/vdom/vdom-annotations.ts	14
src/sys/node/node-sys.ts	14
src/compiler/build/build-finish.ts	13
src/compiler/prerender/prerender-queue.ts	13

Our most common errors

Typescript Error Code	Count
TS2345	418
TS2322	398
TS18048	310
TS18047	100
TS2722	38
TS2532	34
TS2531	23
TS2454	14
TS2352	13
TS2769	10
TS2790	10
TS2538	8
TS2344	5
TS2416	4
TS2493	3
TS18046	2
TS2684	1
TS2488	1
TS2430	1

Unused exports report

There are 12 unused exports on this PR. That's the same number of errors on main, so at least we're not creating new ones!

Unused exports

File	Line	Identifier
src/runtime/bootstrap-lazy.ts	21	setNonce
src/screenshot/screenshot-fs.ts	18	readScreenshotData
src/testing/testing-utils.ts	198	withSilentWarn
src/utils/index.ts	145	CUSTOM
src/compiler/app-core/app-data.ts	25	BUILD
src/compiler/app-core/app-data.ts	115	Env
src/compiler/app-core/app-data.ts	117	NAMESPACE
src/compiler/fs-watch/fs-watch-rebuild.ts	123	updateCacheFromRebuild
src/compiler/types/validate-primary-package-output-target.ts	62	satisfies
src/compiler/types/validate-primary-package-output-target.ts	62	Record
src/testing/puppeteer/puppeteer-declarations.ts	485	WaitForEventOptions
src/compiler/sys/fetch/write-fetch-success.ts	7	writeFetchSuccessSync

tanner-reits · 2023-10-25T17:28:53Z

src/screenshot/screenshot-compare.ts

-    const timeout = jasmine.DEFAULT_TIMEOUT_INTERVAL * 0.5;
+    /**
+     * When using screenshot functionality in a runner that is not Jasmine (e.g. Jest Circus), we need to set a default
+     * value for timeouts. There are issues runtime errors that occur if we attempt to use optional chaining + nullish


Small grammatical error, occurs in a few other spots as well where this comment was copy-pasta'd

Suggested change

* value for timeouts. There are issues runtime errors that occur if we attempt to use optional chaining + nullish

* value for timeouts. There are runtime errors that occur if we attempt to use optional chaining + nullish

Bah - the worst part of copy/paste!

tanner-reits · 2023-10-25T17:38:10Z

src/testing/jest/jest-apis.ts

+type BasePreprocessor = {
+  process(sourceText: string, sourcePath: string, ...args: any[]): string | TransformedSource;
+  getCacheKey(sourceText: string, sourcePath: string, ...args: any[]): string;
+};


Can we just use this as a the type for JestPreprocessor to avoid having to make changes to this file with each version added/removed?

Yeah, I'm open to that! 879fca5

tanner-reits

LGTM!

rwaskiewicz · 2023-10-25T20:25:43Z

Note, Steps 3-5 are a little different in the README PR for adding new Jest support after this commit - 09129f4

add suport for jest 28 in stencil. this commit is the first pass at support a new version of jest in stencil in well over a year, and the first to use the new package-based architecture. this commit was generated using the steps that are outline in the README file of the jest directory, [available here](https://github.com/ionic-team/stencil/blob/d725ac9ef73f9700feb773d4a65e3ea988db6d31/src/testing/jest/README.md?plain=1). note: some commits of the original pr may be out of order with the readme above. although the readme is permalinked, it did undergo some changes between the time this pr's commits were created and the time this pr was created. some items of note: - the contents of `jest-27-and-under/` were copied into `jest-28/` using the latest available contents at the time - 6893954 (#4904). in order to support jest 28, the following changes were made: 1. projects are now typed as `string` in v28: the typings for jest are causing the project to fail compilation. update the return value here, as `Config.Path` is now just a string 1. resolve TS4053 error in v28 facade: we had to resolve a typescript compilation error where we would run into typescript error TS4053. this was caused by this facade's implementation of `getJestPreset` returning a Jest preset that referenced Jest v28's `Config` interface. the TypeScript compiler did not know how to make sense of this import within the module, requiring us to manually import it 1. update typings of the preprocessor: in jest 28, preprocessors cannot return a string. rather, they must at minimum return `{ code : string }`. this commit resolves those compilation errors by using Jest's `TransformedSource` type and updating the return values of the function accordingly 1. migrate to jest-circus for v28: switch users over to jest-circus for v28, rather than jest-jasmine2. this makes it such that we are not required to force users to add jest-jasmine2 to their dependencies, since it was removed from jest in v28 (and published as a separate package). 1. handle updated environment export: the test environments export has changed in v28. update the require statement to handle this change. 1. remove jasmine globals: remove direct references to jasmine globals that were used in jest 28 code. in some instnaces, puppeteer code that is not versioned (per version of jest) had to be modified to ensure that we did not attempt to reference a global `jasmine` that did not exist finally, some backwards compatability code was removed from the new `jest-28/` directory. it did not need to be executed in this new architecture (and can be siloed to `jest-27-and-under/`) closes: 3348 STENCIL-955

add support for jest 29 in stencil. this commit is a fast-follow to #4979 (d3aa539), which adds support for jest 28. as such, there will be many similarlities between the two pieces of code. STENCIL-956

rwaskiewicz marked this pull request as ready for review October 25, 2023 12:34

rwaskiewicz requested a review from a team as a code owner October 25, 2023 12:34

rwaskiewicz requested review from alicewriteswrongs and tanner-reits October 25, 2023 12:34

rwaskiewicz mentioned this pull request Oct 25, 2023

feat(test): jest 29 support #4981

Merged

2 tasks

tanner-reits requested changes Oct 25, 2023

View reviewed changes

rwaskiewicz requested a review from tanner-reits October 25, 2023 18:47

tanner-reits approved these changes Oct 25, 2023

View reviewed changes

alicewriteswrongs approved these changes Oct 26, 2023

View reviewed changes

rwaskiewicz force-pushed the rwaskiewicz/jest/v28-support branch from 6243210 to fbe17dd Compare October 26, 2023 18:12

rwaskiewicz enabled auto-merge October 26, 2023 18:16

rwaskiewicz added this pull request to the merge queue Oct 26, 2023

Merged via the queue into main with commit d3aa539 Oct 26, 2023
108 checks passed

rwaskiewicz deleted the rwaskiewicz/jest/v28-support branch October 26, 2023 18:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(test): jest 28 support #4979

feat(test): jest 28 support #4979

rwaskiewicz commented Oct 24, 2023 •

edited

Loading

github-actions bot commented Oct 24, 2023 •

edited

Loading

tanner-reits Oct 25, 2023

rwaskiewicz Oct 25, 2023

tanner-reits Oct 25, 2023

rwaskiewicz Oct 25, 2023

tanner-reits left a comment

rwaskiewicz commented Oct 25, 2023

	* value for timeouts. There are issues runtime errors that occur if we attempt to use optional chaining + nullish
	* value for timeouts. There are runtime errors that occur if we attempt to use optional chaining + nullish

feat(test): jest 28 support #4979

feat(test): jest 28 support #4979

Conversation

rwaskiewicz commented Oct 24, 2023 • edited Loading

What is the current behavior?

Note to Reviewers

What is the new behavior?

Does this introduce a breaking change?

Testing

Component Starter Testing

Ionic Framework

Other information

github-actions bot commented Oct 24, 2023 • edited Loading

--strictNullChecks error report

reports and statistics

Unused exports report

tanner-reits Oct 25, 2023

Choose a reason for hiding this comment

rwaskiewicz Oct 25, 2023

Choose a reason for hiding this comment

tanner-reits Oct 25, 2023

Choose a reason for hiding this comment

rwaskiewicz Oct 25, 2023

Choose a reason for hiding this comment

tanner-reits left a comment

Choose a reason for hiding this comment

rwaskiewicz commented Oct 25, 2023

rwaskiewicz commented Oct 24, 2023 •

edited

Loading

github-actions bot commented Oct 24, 2023 •

edited

Loading

`--strictNullChecks` error report