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

🪟 🔧 Use jest directly as test runner #21174

Merged
merged 1 commit into from
Jan 10, 2023
Merged

🪟 🔧 Use jest directly as test runner #21174

merged 1 commit into from
Jan 10, 2023

Conversation

timroes
Copy link
Collaborator

@timroes timroes commented Jan 9, 2023

What

This is part of moving our build system from CRA to Vite. Since Vite doesn't have any test runner build in we need a solution for running our tests without Create React App. We use jest directly for doing so (elaboration see below).

This PR already switches the tests to run via Jest directly, since we can extract that into a separate PR, since it does not require Vite in any way.

How

This changes the npm scripts to run tests from using CRA (via craco) to use jest as a command directly. It also adds the required configuration files we'll need to run the tests.

We continue to use babel as a transpiler for jest (instead of using esbuild that we plan to use with vite for actual bundling), since the jest babel integration works better and supports jest mocks hoisting mechanism properly (which esbuild doesn't).

For more details view my inline comments.

Why jest?

We also looked at vitest as an alternative to run tests. We decided to stay with jest/babel for tests for some reasons:

  • Vitest is still very new, while jest is already established.
  • We haven't seen issues using jest in the past - never change a running system
  • Vitest requires a different syntax for mocking, so we'd need to change way more code to get it running.

We also looked at integrating jest with vite via vite-jest. Given the rare updates of the library, low download rate and felt immaturity, we decided not to go that route.

@octavia-squidington-iv octavia-squidington-iv added the area/frontend Related to the Airbyte webapp label Jan 9, 2023
@@ -0,0 +1,12 @@
module.exports = {
env: {
test: {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ Add the require babel configuration (that babel-jest will read) that is required to transform our code. We only add this when running in the test environment, to not potentially collide with any configuration of Create React App, while we're still using it to build the rest of the app.

@@ -0,0 +1,21 @@
// eslint-disable-next-line jest/no-jest-import
import type { Config } from "jest";
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ Actual jest config, compiled from our previous package.json plus some additional config we need now, since CRA doesn't take care of it anymore.

"\\.module\\.scss$": "test-utils/mock-data/mockIdentity.js",
"\\.(css|png|svg|scss)$": "test-utils/mock-data/mockEmpty.js",
},
setupFilesAfterEnv: ["./src/test-utils/setup-tests.ts"],
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ Moved this file (and the classname-serializer) into src/test-utils, since we no longer require it to be at src/setupTests.ts, which CRA forced us to.

Comment on lines +15 to +16
"\\.module\\.scss$": "test-utils/mock-data/mockIdentity.js",
"\\.(css|png|svg|scss)$": "test-utils/mock-data/mockEmpty.js",
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ Make sure we mock asset imports in a way that jest test run properly.

import "../globals";

// jsdom doesn't mock `matchMedia`, which is required by react-slick
Object.defineProperty(window, "matchMedia", {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ Rewrote this mock slightly from what we had beforehand here, to align with what jest actually has in their documentation: https://jestjs.io/docs/26.x/manual-mocks#mocking-methods-which-are-not-implemented-in-jsdom

Copy link
Contributor

@josephkmh josephkmh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, tests pass ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/frontend Related to the Airbyte webapp
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants