Allow ES6 based tests using jest #2540

ohadlevy · 2017-10-29T11:36:59Z

jest testing support for miq

also add missing tests for component registry and enable eslint configuration for react.

ohadlevy · 2017-10-29T11:37:11Z

vojtechszocs · 2017-11-01T17:20:44Z

Jest can be used very similarly to jasmine, but allow us to use babel(ES6) and many of its pluging ecosystem (and ofcourse, its faster).

Jest is much more than that 😄 it's converging to become a test platform, beyond a simple test runner.

Jest does stuff like ES6 to ES5 transpilation using existing Babel configuration, which means you don't have to "webpack + run tests", you just "run tests". Simply put, Jest handles the tests and webpack handles the build.

vojtechszocs · 2017-11-01T17:22:44Z

.babelrc

+  "presets": [["env", { "modules": false }]],
+  "env": {
+    "test": {
+      "presets": []


This is just a preparation for the future, to allow test-specific Babel configuration, right?

vojtechszocs · 2017-11-01T17:32:04Z

app/javascript/packs/hello_angular.test.js

@@ -0,0 +1,5 @@
+test('it should log to the console hello angular', () => {
+  window.console.log = jest.fn();


This is a side effect which should be restored after the test finishes its execution (it's usually better to do this in beforeEach and afterEach though):

const restore = window.console.log; window.console.log = jest.fn(); require('./application-common'); expect(window.console.log).toBeCalled(); window.console.log = restore;

vojtechszocs · 2017-11-01T17:34:50Z

package.json

@@ -4,7 +4,9 @@
  "description": "ManageIQ Cloud Management Platform",
  "main": "index.js",
  "scripts": {
-    "test": "echo \"Error: no test specified\" && exit 1"
+    "test": "node node_modules/.bin/jest",


Why node node_modules/.bin/jest and not simply jest ?

AFAIK, when executing scripts defined in package.json via npm run or yarn run, executables in node_modules/.bin are added to the path automatically.

vojtechszocs · 2017-11-01T17:46:47Z

package.json

@@ -73,5 +78,28 @@
    "node": ">= 6.9.1",
    "npm": ">= 3.10.3",
    "yarn": ">= 0.20.1"
+  },
+  "jest": {


I'd strongly suggest not to pollute your package.json with dependency-specific configuration.

See this AVA issue where people asked for an alternative configuration method and were denied just because the maintainers weren't open minded enough. If you'd put all your Babel, Jest, etc. configuration into single package.json, you'd end up with a huge JSON file, mixing project definition (metadata, dependencies, etc.) with dependency configuration while also being limited to the static nature of JSON while also not being able to reuse/align those configurations across different projects.

A much better way is to create a separate file, like jest.config.js, that contains Jest related configuration. Plus, since it's .js it can contain logic as well.

vojtechszocs · 2017-11-01T17:57:17Z

package.json

@@ -73,5 +78,28 @@
    "node": ">= 6.9.1",
    "npm": ">= 3.10.3",
    "yarn": ">= 0.20.1"
+  },
+  "jest": {
+    "automock": true,


Why enable auto-mocking? IMHO explicit, selective mocking of modules is much better.

Note that auto-mocking was turned off by default since Jest 15.

vojtechszocs · 2017-11-01T18:01:10Z

package.json

+      "__testing__": true
+    },
+    "transform": {
+      "^.+\\.js$": "babel-jest"


Custom transform isn't necessary, unless you're using extra preprocessors, see here.

vojtechszocs · 2017-11-01T18:03:02Z

package.json

+    },
+    "roots": [
+      "app/javascript/packs"
+    ]


I'd explicitly specify which files should be picked up for testing, something like:

testMatch: ["**/*.test.js"]

vojtechszocs · 2017-11-01T18:07:28Z

package.json

+      "lcov"
+    ],
+    "moduleNameMapper": {
+      "^.+\\.(png|gif|css|scss)$": "identity-obj-proxy"


I'd usually mock binary assets, like images & fonts, like this:

"\.(png|jpg|gif|svg|eot|ttf|woff|woff2)$": "jest.mock.static-file.js"

where the file mentioned above would contain

'use strict'; module.exports = 'static-file-stub';

so that when someone imports a binary asset in JS code, he gets back the static-file-stub string.

TL;DR not sure if identity-obj-proxy is worth using on binary assets.

ohadlevy · 2017-11-07T14:14:16Z

@vojtechszocs in the meanwhile I'm taking a different approach at https://github.com/ohadlevy/miq_experimental_ui_plugin - please let me know what do you think about the defaults in that project.

current blocker I've seen is that travis doesn't know how to run multiple languages in the same project, e.g. you can either have a ruby matrix or a node matrix - not both.

miq-bot · 2017-11-13T15:02:00Z

This pull request is not mergeable. Please rebase and repush.

ohadlevy · 2018-01-29T07:17:52Z

@vojtechszocs can you please have another look?

ohadlevy · 2018-01-29T07:40:11Z

I've left a failing test so we can see it actually failing, once you approve I'll fix the test.
/cc @martinpovolny

also add missing tests for component registry and enable eslint configuration for react.

ohadlevy · 2018-01-29T10:37:04Z

.babelrc

@@ -1,5 +1,8 @@
 {
-  "presets": [
-    ["env", { "modules": false } ]


any idea why we had modules: false to begin with?

https://github.com/rails/webpacker/blob/master/lib/install/config/.babelrc#L4 is the only reason I think.

(Mostly our webpack config is just webpacker default.)

ohadlevy · 2018-01-29T10:40:13Z

.travis.yml

@@ -15,6 +15,7 @@ env:
  - TEST_SUITE=spec
  - TEST_SUITE=spec:javascript
  - TEST_SUITE=spec:compile
+  - TEST_SUITE=spec:jest


I was unable to find a way to have a different language (besides ruby) for travis to run while keeping the ruby one, this means we execute the tests as a rake task under ruby environment, however I believe this is exactly how jasmine tests run today.

It has a few disadvanteges:

setup installs the entire environment, when in fact we only need yarn (adds about 10 minutes to the test)

we run jest twice with two different rubies.

we can't run with different node version.

@skateman - you once mentioned you know of a way to run travis with different language matrix... any idea? :)

@ohadlevy yes, look here and here. Basically travis provides support for a generic container which is close to a clean debian/ubuntu base container. Here you can set up your stuff without interfering with the preinstalled environment.

@skateman thanks - the way I read it:

we can do it, but it means we have to revisit/update how we use travis today.

not in this PR :)

himdel · 2018-01-29T13:04:36Z

Any chance we can keep the tests nicely separated from the code as we have now?

Say, app/javascript for code and spec/javascript for specs?

Otherwise, LGTM, but the test failure is relevant ;).

ohadlevy · 2018-01-29T13:17:39Z

Any chance we can keep the tests nicely separated from the code as we have now?

Sure we can, but i have a few reason why i suggest this way:

this code is complied, in theory, we dont need the app/javascript folder content in our build (so less of a reason if you care about pushing test code to "production")
when using ES6, we often import, if the files are in different folders, i have to import x from '../../../app/javascripts/some/path/to/file' vs import x from 'file'.
when reviewing PR's, i see logically the test and the code at the same time (vs scroll down to the tests and up to the code over and over again.

Would you be willing to give it a try? We can always switch afterwards.

Otherwise, LGTM, but the test failure is relevant ;).

You mean the hello angular test? I left it on purpose so you can see its actually failing - happy to remove if you ack.

himdel · 2018-01-29T13:29:08Z

OK, I don't really have a strong opinion about the separation, I consider it useful because when searching, you usually only care about code, not specs and if those are together, it can get annoying to keep seeing specs in search results.

But I can set my search tool to ignore *.test.js files and the only negative effect will be that I won't ever notice stuff mentioned only in specs again :).

You mean the hello angular test? I left it on purpose so you can see its actually failing - happy to remove if you ack.

Aah, then yup, this looks promising :) It actually looks readable in the travis output.. (for posterity, this is how it looks)

Feel free to remove, I still want to give this a spin locally to see how it works in a browser, but IMO LGTM.

ohadlevy · 2018-01-29T13:49:18Z

@himdel I've removed the faling test - if someone wants to see it have a look at https://travis-ci.org/ManageIQ/manageiq-ui-classic/jobs/334595691

I also added a commit which fixes linting errors due to new the configuration.

himdel · 2018-01-29T14:12:07Z

Running the whole test suite:

yarn test

Running a specific test:

yarn run jest app/javascript/packs/hello_angular.test.js

Running with debugger enabled:

rake spec:jest:debug

(Which will do..

node --inspect-brk node_modules/.bin/jest --runInBand

and then there's a green button in chrome's inspector, or a list in chrome://inspect/#devices.

EDIT: Apparently there's no way to make chrome open the right thing from the script, but I guess $BROWSER chrome://inspect/#devices is slightly better than nothing...
)

.. we should add this info somewhere in guides.

miq-bot · 2018-01-29T14:51:02Z

Some comments on commits ohadlevy/manageiq-ui-classic@02394a2~...41db8b0

Rakefile

⚠️ - 118 - Detected puts. Remove all debugging statements.
⚠️ - 119 - Detected puts. Remove all debugging statements.
⚠️ - 120 - Detected puts. Remove all debugging statements.

ohadlevy · 2018-01-29T14:51:03Z

@himdel added task:

rake spec:jest:debug
                                                                                            
open your chrome://inspect/#devices on your chrome based browser (see https://facebook.github.io/jest/docs/en/troubleshooting.html for more details)

Debugger listening on ws://127.0.0.1:9229/1b95392b-d88c-449d-bf0b-dfd93a747940
For help see https://nodejs.org/en/docs/inspector
...

miq-bot · 2018-01-29T14:51:34Z

Checked commits ohadlevy/manageiq-ui-classic@02394a2~...41db8b0 with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 1 offense detected

Rakefile

❗ - Line 123, Col 1 - Layout/EmptyLinesAroundBlockBody - Extra empty line detected at block body end.

himdel · 2018-01-29T15:11:40Z

Thanks, waiting for travis.. :)

mzazrivec added fine/no test labels Oct 30, 2017

ohadlevy changed the title ~~jest testing support for miq~~ Allow ES6 based tests using jest Nov 1, 2017

ohadlevy force-pushed the jest branch from 9ed797f to 240d5ed Compare November 1, 2017 09:45

ohadlevy changed the title ~~Allow ES6 based tests using jest~~ [WIP] Allow ES6 based tests using jest Nov 1, 2017

miq-bot added the wip label Nov 1, 2017

ohadlevy force-pushed the jest branch 3 times, most recently from d9ba490 to 91a53a3 Compare November 1, 2017 11:47

vojtechszocs reviewed Nov 1, 2017

View reviewed changes

vojtechszocs mentioned this pull request Nov 3, 2017

Move to Jest for for testing ManageIQ/manageiq-ui-service#964

Closed

ohadlevy mentioned this pull request Nov 8, 2017

fixes #21592 - uses headless chrome instead of phantomjs theforeman/foreman#4987

Closed

miq-bot added the unmergeable label Nov 13, 2017

ohadlevy force-pushed the jest branch from 91a53a3 to 9478ba1 Compare January 28, 2018 12:24

miq-bot removed the unmergeable label Jan 28, 2018

ohadlevy force-pushed the jest branch 5 times, most recently from e8ecba2 to 52e10f7 Compare January 29, 2018 06:44

ohadlevy force-pushed the jest branch 2 times, most recently from 0de70ff to d0836c2 Compare January 29, 2018 07:39

ohadlevy force-pushed the jest branch from d0836c2 to e791610 Compare January 29, 2018 07:45

jest testing support for miq

02394a2

also add missing tests for component registry and enable eslint configuration for react.

ohadlevy force-pushed the jest branch from e791610 to 02394a2 Compare January 29, 2018 07:56

ohadlevy commented Jan 29, 2018

View reviewed changes

ohadlevy changed the title ~~[WIP] Allow ES6 based tests using jest~~ Allow ES6 based tests using jest Jan 29, 2018

miq-bot removed the wip label Jan 29, 2018

ohadlevy added 2 commits January 29, 2018 15:48

removed failing test.

698293a

fixed past linting issues.

306ccd2

added rake task to start node debugger

41db8b0

himdel merged commit 5efe52e into ManageIQ:master Jan 29, 2018

himdel self-assigned this Jan 29, 2018

himdel added the gaprindashvili/no label Jan 29, 2018

himdel added this to the Sprint 78 Ending Jan 29, 2018 milestone Jan 29, 2018

vojtechszocs mentioned this pull request Feb 9, 2018

Add new JS component API #3228

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow ES6 based tests using jest #2540

Allow ES6 based tests using jest #2540

ohadlevy commented Oct 29, 2017 •

edited

Loading

ohadlevy commented Oct 29, 2017

vojtechszocs commented Nov 1, 2017

vojtechszocs Nov 1, 2017

vojtechszocs Nov 1, 2017

vojtechszocs Nov 1, 2017

vojtechszocs Nov 1, 2017

vojtechszocs Nov 1, 2017

vojtechszocs Nov 1, 2017

vojtechszocs Nov 1, 2017

vojtechszocs Nov 1, 2017

ohadlevy commented Nov 7, 2017

miq-bot commented Nov 13, 2017

ohadlevy commented Jan 29, 2018

ohadlevy commented Jan 29, 2018

ohadlevy Jan 29, 2018

himdel Jan 29, 2018

ohadlevy Jan 29, 2018

skateman Jan 29, 2018

ohadlevy Jan 29, 2018

himdel commented Jan 29, 2018 •

edited

Loading

ohadlevy commented Jan 29, 2018 •

edited

Loading

himdel commented Jan 29, 2018 •

edited

Loading

ohadlevy commented Jan 29, 2018

himdel commented Jan 29, 2018 •

edited

Loading

miq-bot commented Jan 29, 2018

ohadlevy commented Jan 29, 2018

miq-bot commented Jan 29, 2018

himdel commented Jan 29, 2018

		@@ -0,0 +1,5 @@
		test('it should log to the console hello angular', () => {
		window.console.log = jest.fn();

Allow ES6 based tests using jest #2540

Allow ES6 based tests using jest #2540

Conversation

ohadlevy commented Oct 29, 2017 • edited Loading

ohadlevy commented Oct 29, 2017

vojtechszocs commented Nov 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ohadlevy commented Nov 7, 2017

miq-bot commented Nov 13, 2017

ohadlevy commented Jan 29, 2018

ohadlevy commented Jan 29, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

himdel commented Jan 29, 2018 • edited Loading

ohadlevy commented Jan 29, 2018 • edited Loading

himdel commented Jan 29, 2018 • edited Loading

ohadlevy commented Jan 29, 2018

himdel commented Jan 29, 2018 • edited Loading

Running the whole test suite:

Running a specific test:

Running with debugger enabled:

miq-bot commented Jan 29, 2018

ohadlevy commented Jan 29, 2018

miq-bot commented Jan 29, 2018

himdel commented Jan 29, 2018

ohadlevy commented Oct 29, 2017 •

edited

Loading

himdel commented Jan 29, 2018 •

edited

Loading

ohadlevy commented Jan 29, 2018 •

edited

Loading

himdel commented Jan 29, 2018 •

edited

Loading

himdel commented Jan 29, 2018 •

edited

Loading