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.

Sign up for GitHub

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

Jump to bottom

js: Add Jest testing #801

Merged
merged 5 commits into from
Mar 21, 2022
Merged

js: Add Jest testing #801

merged 5 commits into from
Mar 21, 2022

Conversation

ix5
Copy link
Member

@ix5 ix5 commented Feb 19, 2022 •
edited

As discussed in #754, add some basic Jest testing config and examples to facilitate finally writing some frontend code tests.

Note: End-to-end tests using puppeteer were split off and moved into #807

Terminology

  • Jest is a Javascript testing framework. It is mainly used here for unit tests.

Running

Install the needed packages:

npm install

Then run:

  • npm run test-unit for Jest unit tests

Highly WIP at the moment, but could be useful to someone else working on the same issues. Please communicate and collaborate!

Depends on #800. I have temporarily changed the base to that branch to make it easier to review this change.

@ix5
Copy link
Member Author

ix5 commented Feb 19, 2022 •
edited

Also pinging @stefangehn, @blatinier and @Lucas-C here. Let me know if you find this useful and please share tips/improvements/general ideas.

I'd love for someone more knowledgeable than me to write some proper tests and I hope this is a good starting-off point.

A way to simply have some docker containers (one for Isso, one for JS e2e puppeteer tests) interconnected and running them with a simple command would be much appreciated!

@ix5 ix5 added client (Javascript) client code and CSS feature labels Feb 19, 2022
@ix5 ix5 added this to the 0.13 milestone Feb 19, 2022
@ix5 ix5 changed the base branch from master to js-webpack February 19, 2022 16:59
Lucas-C
Lucas-C reviewed Mar 1, 2022
View reviewed changes
Copy link
Contributor

@Lucas-C Lucas-C left a comment

Choose a reason for hiding this comment

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

Nice bootstrapping! Good job @ix5 :)
Can I commit stuff on this branch if I find the time to write some tests?

isso/js/jest-puppeteer.config.js Outdated Show resolved Hide resolved
isso/js/tests/integration/puppet.test.js Outdated Show resolved Hide resolved
isso/js/tests/mocks/fileMock.js Show resolved Hide resolved
isso/js/tests/unit/utils.test.js Outdated Show resolved Hide resolved
package.json
"build-dev": "webpack --config isso/js/webpack.config.js --config-name dev",
"watch-dev": "webpack --config isso/js/webpack.config.js --config-name dev --watch",
"build-prod": "webpack --config isso/js/webpack.config.js --merge --config-name dev --config-name prod"
},
"devDependencies": {
"jest": "^27.5.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

"jest-puppeteer" is missing ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

Adding jest-puppeteer to devDependencies will automatically pull in a 400Mb+ download of headless chrome, as explained earlier. That's why I'm a bit hesitant to add it here.

Imo it should be up to the user whether they need to run the end-to-end tests.

Maybe there's an option to configure the package before installing to not pull in the chrome download automatically?

isso/js/jest-puppeteer.config.js Outdated
@@ -0,0 +1,6 @@
module.exports = {
Copy link
Contributor

Choose a reason for hiding this comment

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

This file should be at the root of the repository.
It is currently ignored by Jest when running npm test

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd like to keep all javascript-related stuff inside isso/js/, so I'll have to figure out a way for Jest to read this file.
See https://github.com/smooth-code/jest-puppeteer#configure-puppeteer

(remains TODO)

Copy link
Member Author

Choose a reason for hiding this comment

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

Point of reference: argos-ci/jest-puppeteer#160 (comment)

process.env.JEST_PUPPETEER_CONFIG = require.resolve('./jest-puppeteer.config.js');

isso/js/tests/unit/template.test.js Outdated Show resolved Hide resolved
@ix5 ix5 mentioned this pull request Mar 3, 2022
Merged
@ix5
Copy link
Member Author

ix5 commented Mar 3, 2022

Hey @Lucas-C, thank you so much for reviewing and verifying!

I probably should have split this PR into two PRs, one for Jest and one for the addition of puppeteer.

Re: virtualenv standard location and bringing up the Isso dev server, my wish for that would be to have some kind of standardization in testing.
Either docker containers with docker-compose for client and server (too heavyweight imo plus dependency on docker+compose) or an entry in the Makefile for creating the virtualenv and then starting the server from inside Node? Or maybe requiring the user to start the server?

Instrumentation is hard, and I'm open to suggestions here as I have no experience and no idea what accepted standard practices are.

@ix5
Copy link
Member Author

ix5 commented Mar 3, 2022 •
edited

Nice bootstrapping! Good job @ix5 :) Can I commit stuff on this branch if I find the time to write some tests?

Let me split up the puppeteer addition into another PR first, then commit away ;)

Edit: Done, new PR is at #807

@ix5 ix5 changed the title [WIP] js: Add Jest testing with puppeteer end-to-end integration test [WIP] js: Add Jest testing Mar 3, 2022
@ix5 ix5 mentioned this pull request Mar 3, 2022
Merged
@ix5 ix5 force-pushed the js-jest-puppeteer branch 2 times, most recently from fe23d2c to fbaac0b Compare March 7, 2022 15:23
ix5 added 5 commits March 21, 2022 21:39
@ix5
isso: js: Add Jest testing config for unit tests
42cec2c 
Provide a base config and npm commands to run unit tests
using the `Jest` tool.

Recommendations have been lifted from
https://mailchimp.com/developer/open-commerce/docs/testing-requirements/

`jsdom` "emulates" a browser environment in a light way so
that access to `window`, `document` etc. is possible.
@ix5
isso: tests/unit: Test utils.js
d40682b 
Just a simple test as a starting point for further ones.
@ix5
isso: tests/unit: Test config.js
825708e 
Test expected languages when none are set in the browser.
Could be expanded further.
@ix5
isso: tests/unit: Test isso.js Postbox
7c522eb
@ix5
isso: tests/unit: Check in isso.js snapshot
1ad1a05
@ix5 ix5 changed the title [WIP] js: Add Jest testing js: Add Jest testing Mar 21, 2022
@ix5 ix5 changed the base branch from js-webpack to master March 21, 2022 20:41
@ix5 ix5 merged commit 7a1ae20 into isso-comments:master Mar 21, 2022
@ix5 ix5 deleted the js-jest-puppeteer branch March 21, 2022 20:42
@ix5 ix5 mentioned this pull request Apr 30, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Lucas-C Lucas-C Lucas-C left review comments

Assignees
No one assigned
Labels
client (Javascript) client code and CSS feature
Projects
None yet
Milestone
0.13
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@ix5 @Lucas-C