Use jest for unit tests by outofambit · Pull Request #5427 · desktop/desktop

outofambit · 2018-08-17T00:00:13Z

Goals

port our unit testing setup to something simpler and more featured
- specifically, support for test coverage metrics
explore alternatives to electron-mocha that play nicer with other libraries

Implementation

jest
refactored scripts/unit-test.ts into configuration/setup files used by jest (see jest.config.js)
manually mocked electron libraries for tests (see __mocks__/electron.ts)
mocked database with fake-indexeddb
run jest in electron as node to ensure similar build architecture

Discussion

~~had a couple disagreements with the linter that could be cleaned up better~~
~~one outstanding issue with VSTS regarding git config location (still looking into this)~~

j-f1 · 2018-08-17T11:13:16Z

Has the team read about snapshot testing in Jest yet?

outofambit · 2018-08-17T13:28:19Z

@j-f1 i am familiar with it! i haven't spent much time thinking how to apply them in this codebase tho, so if you have any thoughts, send 'em my way :)

Co-Authored-By: Brendan Forster <github@brendanforster.com>

This reverts commit 9216fbd.

app/test/__mocks__/electron.ts

@@ -0,0 +1,19 @@
+export const shell = {
+  moveItemToTrash: jest.fn(),


app/test/__mocks__/electron.ts

@@ -0,0 +1,19 @@
+export const shell = {


package.json

    "@types/keytar": "^4.0.0",
    "@types/legal-eagle": "^0.15.0",
    "@types/mini-css-extract-plugin": "^0.2.0",
-    "@types/mocha": "^2.2.48",


package.json

    "klaw-sync": "^3.0.0",
    "legal-eagle": "0.16.0",
    "mini-css-extract-plugin": "^0.4.0",
-    "mocha": "^5.0.4",


script/unit-tests.ts

-    repositoryRoot,
-    'node_modules',
-    '.bin',
-    'xvfb-maybe'


app/test/globals.ts


+const Dexie = require('dexie')
+Dexie.dependencies.indexedDB = require('fake-indexeddb')
+Dexie.dependencies.IDBKeyRange = require('fake-indexeddb/lib/FDBKeyRange')


app/test/setup-test-framework.ts

@@ -0,0 +1,3 @@
+/* eslint-disable strict */


app/test/unit-test-env.ts

@@ -0,0 +1,16 @@
+/* eslint-disable strict */


This reverts commit 80fa1dd.

This reverts commit e48a3eb.

outofambit · 2018-08-27T17:30:29Z

thanks for #5455 @shiftkey!! 💜

shiftkey

I had a few deeper Jest questions (mostly around how to organize the mocks we've already accrued) but for now I just focused on testing this and other bits that can be tidied up.

package.json

-    "electron-winstaller": "2.5.2"
+    "electron-winstaller": "2.5.2",
+    "jest": "^23.5.0",
+    "ts-jest": "^23.1.3"


package.json

    "electron-packager": "^12.0.0",
-    "electron-winstaller": "2.5.2"
+    "electron-winstaller": "2.5.2",
+    "jest": "^23.5.0",


package.json

+    "test:unit": "cross-env ELECTRON_RUN_AS_NODE=1 node_modules/.bin/electron ./node_modules/jest/bin/jest --config ./jest.config.js",
+    "test:unit:cov": "yarn test:unit --coverage",
+    "test:script": "mocha -P ./tsonfig.json -t 10000 --require ts-node/register script/changelog/test/*.ts",
+    "test": "yarn test:unit --runInBand && yarn test:script && yarn test:integration",


app/test/unit-test-env.ts

+  // ensuring Electron doesn't attach to the current console session (Windows only)
+  ELECTRON_NO_ATTACH_CONSOLE: '1',
+  // speed up ts-node usage by using the new transpile-only mode
+  TS_NODE_TRANSPILE_ONLY: 'true',


app/test/setup-test-framework.ts

@@ -0,0 +1,4 @@
+'use strict'


package.json

@@ -156,6 +158,8 @@
    "electron-builder": "20.27.1",
    "electron-mocha": "^6.0.1",


app/test/unit-test-env.ts

@@ -0,0 +1,17 @@
+'use strict'


outofambit · 2018-08-27T22:00:31Z

@shiftkey i tried removing the use stricts from those two .ts files, but keep getting these linter errors from yarn lint:

$ yarn lint
yarn run v1.9.4
$ yarn lint:src && yarn lint:prettier
$ yarn tslint && yarn eslint-check && yarn eslint
$ tslint -p .
$ eslint --print-config .eslintrc.* | eslint-config-prettier-check
No rules that are unnecessary or conflict with Prettier were found.
$ ts-node -P script/tsconfig.json script/eslint.ts

/Users/outofambit/GitHub/desktop/app/test/setup-test-framework.ts
  2:1  error  Use the global form of 'use strict'  strict

/Users/outofambit/GitHub/desktop/app/test/unit-test-env.ts
  1:1  error  Use the global form of 'use strict'  strict

any ideas? i've tried a lot of stuff but am getting nowhere.

shiftkey · 2018-08-28T11:53:06Z

any ideas? i've tried a lot of stuff but am getting nowhere.

I believe this is down to eslint considering these two files to be different to the rest of our codebase ("no import hints -> plain old script"?), and also ignoring that we've configured tsconfig.json to be strict: true which implies alwaysStrict when emitting TS ([docs]).

I can workaround this by adding an import * as Path from 'path' to the start of each file, in place of 'use strict' but that's unsatisfying because it's dead code - and I couldn't quite figure out the right eslint flag to set.

Three other workaround ideas I had:

disable each rule explicitly

diff --git a/app/test/setup-test-framework.ts b/app/test/setup-test-framework.ts
index e2baa26f0..36615fc18 100644
--- a/app/test/setup-test-framework.ts
+++ b/app/test/setup-test-framework.ts
@@ -1,4 +1,4 @@
-'use strict'
+/* eslint-disable strict */

 // set test timeout to 10s
 jest.setTimeout(10000)
diff --git a/app/test/unit-test-env.ts b/app/test/unit-test-env.ts
index 1088212de..a315028ef 100644
--- a/app/test/unit-test-env.ts
+++ b/app/test/unit-test-env.ts
@@ -1,4 +1,4 @@
-'use strict'
+/* eslint-disable strict */

 const environmentVariables = {
   // setting commit information so that tests don't need to rely on global config

disable the strict rule for all files under app/test (my preference):

diff --git a/app/test/.eslintrc.yml b/app/test/.eslintrc.yml
index 37d751d0d..38cf443ed 100644
--- a/app/test/.eslintrc.yml
+++ b/app/test/.eslintrc.yml
@@ -1,3 +1,4 @@
 rules:
   # throws with Chai
   no-unused-expressions: off
+  strict: off

leave it as-is and move on with our lives

aka "gawd, don't be so strict"

outofambit · 2018-08-28T16:30:57Z

🍏

shiftkey

Looks great @outofambit! I've opened #5496 to track writing some docs on this broad area, but that doesn't need to hold up this PR.

outofambit added 5 commits August 17, 2018 12:32

jest is awesome

ff3c5a8

clean coverage with clean-slate

a8912a2

add electron-mocha back for integration test

a817970

mock db

4b06e56

mocks working for app-test

ee2c716

outofambit force-pushed the jest branch from dadfaa9 to ee2c716 Compare August 17, 2018 17:46

outofambit and others added 3 commits August 17, 2018 14:16

electron

c742251

use .bin jest

9216fbd

Co-Authored-By: Brendan Forster <github@brendanforster.com>

reintegrate extra env for unit tests

2e6c437

outofambit changed the title ~~[WIP] Spike on using Jest for unit tests~~ Use jest for unit tests Aug 17, 2018

outofambit added 2 commits August 17, 2018 15:10

linting

7f3b367

Revert "use .bin jest"

eae0ca2

This reverts commit 9216fbd.

This comment has been minimized.

Sign in to view

outofambit added 4 commits August 19, 2018 15:13

add 10s timeout to jest tests

8c557b8

linting 😒

96059a6

run jest in band

73b7032

clean up unused deps

3038c8e

outofambit added the ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Aug 19, 2018

outofambit commented Aug 19, 2018

View reviewed changes

app/test/setup-test-framework.ts Outdated

@@ -0,0 +1,3 @@

/* eslint-disable strict */

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

outofambit commented Aug 19, 2018

View reviewed changes

app/test/unit-test-env.ts Outdated

@@ -0,0 +1,16 @@

/* eslint-disable strict */

This comment was marked as spam.

Sign in to view

outofambit added 6 commits August 19, 2018 16:53

use strict in unit test setup scripts

139f919

remove xvfb remnants

80fa1dd

remove one last xvfb remnant

e48a3eb

Revert "remove xvfb remnants"

807dbf9

This reverts commit 80fa1dd.

Revert "remove one last xvfb remnant"

a8be17c

This reverts commit e48a3eb.

make mocha a direct dep

e6d2697

shiftkey mentioned this pull request Aug 23, 2018

refactor config tests to control HOME environment variable #5455

Merged

Merge branch 'master' into jest

0c462b9

shiftkey self-assigned this Aug 27, 2018

shiftkey reviewed Aug 27, 2018

View reviewed changes

remove dead things, add babel-jest

12ecb11

fix linting for strict

7d3ca4e

aka "gawd, don't be so strict"

shiftkey approved these changes Aug 28, 2018

View reviewed changes

outofambit added this to the 1.3.5 milestone Aug 29, 2018

shiftkey merged commit 03741b0 into master Aug 29, 2018

shiftkey deleted the jest branch August 29, 2018 18:17

shiftkey mentioned this pull request Aug 29, 2018

some tweaks to jest's default behaviour #5508

Merged

streeter mentioned this pull request Aug 31, 2018

Appveyor builds are failing silently #5527

Closed

shiftkey mentioned this pull request Sep 21, 2018

app-test.tsx is not currently run as part of unit test suite #5704

Closed

mislav mentioned this pull request Sep 5, 2019

Add instructions on doing a focused test run #8219

Merged

		@@ -0,0 +1,19 @@
		export const shell = {
		moveItemToTrash: jest.fn(),

		@@ -156,6 +158,8 @@
		"electron-builder": "20.27.1",
		"electron-mocha": "^6.0.1",

Conversation

outofambit commented Aug 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Goals

Implementation

Discussion

Uh oh!

j-f1 commented Aug 17, 2018

Uh oh!

outofambit commented Aug 17, 2018

Uh oh!

This comment has been minimized.

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

outofambit commented Aug 27, 2018

Uh oh!

shiftkey left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

outofambit commented Aug 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shiftkey commented Aug 28, 2018

Uh oh!

outofambit commented Aug 28, 2018

Uh oh!

shiftkey left a comment

Choose a reason for hiding this comment

outofambit commented Aug 17, 2018 •

edited

Loading

outofambit commented Aug 27, 2018 •

edited

Loading