Transition from redux-saga to redux-logic and using Jest to test - Part One (#1655)

jwang1919 · outoftime · commit 7ed21f2ce964 · 2019-03-19T19:38:41.000-04:00
* CHECKPOINT jest

* Fix unnecessary addition of an eslint-disable directive

* Initial transition to react-logic. Moved over unlickGithubIdentity logic. Still need to get react-saga and react-logic to play nicely.

* Got redux-saga and redux-logic working at the same time.

* Attempt to get jest working on IntelliJ

* Make jest config more consistent; transform lodash-es

* Mocks for bugsnag and firebase libraries

These libraries are set up in code that runs at module load time, which
is probably not a great practice, but for now we just mock out the
libraries themselves so our modules will load without error.

* Revert unnecessary change to karma config

* Add some jest rules to eslint config

* Write test for unlinkGithubIdentityTest.js

* Small refactor

* Fix yarn.lock with deduplicate

* Fix yarn.lock again?

* Refactored test, installed jest-extended (but didn't use it)

* Fix package.json after rebase

* Fix yarn.lock again

* Fix incorrect merge with .eslintrc

* Add jest override to .eslintrc

* Lint fixing
diff --git a/.eslintrc b/.eslintrc
@@ -33,6 +33,39 @@
         "import/resolver": "node"
       }
     },
+    {
+      "env": {
+        "browser": false,
+        "es6": true,
+        "node": true,
+        "jest": true
+      },
+      "files": [
+        "**/__tests__/*.js"
+      ],
+      "rules": {
+        "jest/no-alias-methods": "warn",
+        "jest/no-disabled-tests": "warn",
+        "jest/no-focused-tests": "warn",
+        "jest/no-identical-title": "error",
+        "jest/no-jasmine-globals": "error",
+        "jest/no-jest-import": "error",
+        "jest/no-test-return-statement": "error",
+        "jest/prefer-to-contain": "warn",
+        "jest/prefer-to-have-length": "warn",
+        "jest/valid-describe": "error",
+        "jest/valid-expect-in-promise": "error",
+        "jest/valid-expect": "error",
+        "jest/prefer-called-with": "warn"
+      },
+      "plugins": [
+        "import",
+        "jest",
+        "react",
+        "promise",
+        "private-props"
+      ]
+    },
     {
       "files": "src/**/*",
       "rules": {
@@ -186,7 +219,10 @@
     "import/no-anonymous-default-export": [
       "warn",
       {
-        "allowArrowFunction": true
+        "allowArray": true,
+        "allowArrowFunction": true,
+        "allowLiteral": true,
+        "allowObject": true
       }
     ],
     "import/no-commonjs": "warn",
diff --git a/.travis.yml b/.travis.yml
@@ -9,6 +9,7 @@ install:
 - docker build --pull --cache-from popcodeorg/popcode:latest --tag popcode .
 script:
 - docker run --env NODE_ENV=test popcode yarn test
+- docker run --env NODE_ENV=test popcode yarn run test-deprecated
 - >-
   docker run
   --env NODE_ENV=production
diff --git a/__mocks__/@bugsnag/js.js b/__mocks__/@bugsnag/js.js
@@ -0,0 +1,26 @@
+import React, {Fragment} from 'react';
+import PropTypes from 'prop-types';
+import noop from 'lodash-es/noop';
+
+function ErrorBoundary({children}) {
+  return React.createElement(Fragment, null, children);
+}
+ErrorBoundary.propTypes = {children: PropTypes.node.isRequired};
+
+export default function bugsnag() {
+  return {
+    use: noop,
+
+    notify: noop,
+
+    getPlugin(plugin) {
+      if (plugin === 'react') {
+        return ErrorBoundary;
+      }
+
+      throw new Error(
+        `bugsnagClient.getPlugin mock called with unexpected plugin ${plugin}`,
+      );
+    },
+  };
+}
diff --git a/__mocks__/@bugsnag/plugin-react.js b/__mocks__/@bugsnag/plugin-react.js
@@ -0,0 +1,2 @@
+const bugsnagReact = {};
+export default bugsnagReact;
diff --git a/__mocks__/@firebase/app.js b/__mocks__/@firebase/app.js
@@ -0,0 +1,17 @@
+import constant from 'lodash-es/constant';
+
+class AuthProvider {
+  addScope() {}
+}
+
+export const firebase = {
+  auth: Object.assign(
+    () => ({}),
+    {
+      GithubAuthProvider: AuthProvider,
+      GoogleAuthProvider: AuthProvider,
+    },
+  ),
+
+  initializeApp: constant({}),
+};
diff --git a/__mocks__/@firebase/auth.js b/__mocks__/@firebase/auth.js
@@ -0,0 +1,2 @@
+/* eslint-disable import/no-anonymous-default-export */
+export default {};
diff --git a/__mocks__/@firebase/database.js b/__mocks__/@firebase/database.js
@@ -0,0 +1,2 @@
+const database = {};
+export default database;
diff --git a/babel.config.js b/babel.config.js
@@ -1,25 +1,38 @@
 const fs = require('fs');
 const path = require('path');
 
-let targets;
-if (process.env.DEBUG === 'true') {
-  targets = {browsers: 'last 1 Chrome version'};
-} else {
-  targets = JSON.parse(
-    fs.readFileSync(path.resolve(__dirname, 'config/browsers.json')),
-  );
-}
+module.exports = (api) => {
+  let targets;
 
-module.exports = {
-  presets: [
-    '@babel/preset-react',
-    ['@babel/preset-env', {targets, modules: false}],
-  ],
-  plugins: ['@babel/plugin-syntax-dynamic-import'],
-  compact: false,
-  overrides: [
-    {
-      include: './node_modules/parse5-sax-parser/lib/index.js',
-    },
-  ],
+  const isJest = api.caller(({name}) => name === 'babel-jest');
+  api.cache.using(() => `${isJest}:${process.env.NODE_ENV}`);
+
+  if (isJest) {
+    targets = {node: 'current'};
+  } else if (process.env.DEBUG === 'true') {
+    targets = {browsers: 'last 1 Chrome version'};
+  } else {
+    targets = JSON.parse(
+      fs.readFileSync(path.resolve(__dirname, 'config/browsers.json')),
+    );
+  }
+
+  const plugins = ['@babel/plugin-syntax-dynamic-import'];
+  if (isJest) {
+    plugins.push('babel-plugin-dynamic-import-node');
+  }
+
+  return {
+    presets: [
+      '@babel/preset-react',
+      ['@babel/preset-env', {targets, modules: isJest ? 'auto' : false}],
+    ],
+    plugins,
+    compact: false,
+    overrides: [
+      {
+        include: './node_modules/parse5-sax-parser/lib/index.js',
+      },
+    ],
+  };
 };
diff --git a/jest.config.js b/jest.config.js
@@ -0,0 +1,14 @@
+/* eslint-env node */
+/* eslint-disable import/no-commonjs */
+
+// For a detailed explanation regarding each configuration property, visit:
+// https://jestjs.io/docs/en/configuration.html
+
+module.exports = {
+  testPathIgnorePatterns: [
+    '/node_modules/',
+    '/bower_components/',
+  ],
+  transformIgnorePatterns: ['node_modules/(?!(lodash-es)/)'],
+  setupFilesAfterEnv: ['jest-extended'],
+};
diff --git a/package.json b/package.json
@@ -241,6 +241,7 @@
     "redux": "^4.0.1",
     "redux-actions": "^2.6.5",
     "redux-immutable": "^4.0.0",
+    "redux-logic": "^2.1.1",
     "redux-saga": "^1.0.2",
     "remark": "^10.0.1",
     "remark-external-links": "^4.0.0",
@@ -265,8 +266,10 @@
     "pretest": "script/check-configs && yarn run lint",
     "start": "gulp dev",
     "dev": "yarn install --frozen-lockfile && yarn start",
-    "test": "karma start --single-run --no-auto-watch",
-    "autotest": "yarn install --frozen-lockfile && karma start --no-single-run --auto-watch",
+    "test": "jest",
+    "autotest": "jest --watchAll",
+    "test-deprecated": "karma start --single-run --no-auto-watch",
+    "autotest-deprecated": "yarn install --frozen-lockfile && karma start --no-single-run --auto-watch",
     "lint-js": "eslint --max-warnings=0 --report-unused-disable-directives --ext .js,.jsx -- src test *.js",
     "lint-css": "stylelint src/**/*.css",
     "lint": "yarn run lint-js && yarn run lint-css",
@@ -287,6 +290,7 @@
     "almost-equal": "^1.1.0",
     "babel-eslint": "^10.0.1",
     "babel-loader": "^8.0.5",
+    "babel-plugin-dynamic-import-node": "^2.2.0",
     "bower": "^1.7.9",
     "brfs": "^2.0.0",
     "browser-sync": "^2.14.3",
@@ -301,6 +305,7 @@
     "eslint-import-resolver-webpack": "^0.10.1",
     "eslint-loader": "^2.1.1",
     "eslint-plugin-import": "^2.14.0",
+    "eslint-plugin-jest": "^22.3.0",
     "eslint-plugin-private-props": "^0.3.0",
     "eslint-plugin-promise": "^4.0.1",
     "eslint-plugin-react": "^7.12.4",
@@ -314,6 +319,8 @@
     "i18next-resource-store-loader": "^0.1.1",
     "imports-loader": "^0.8.0",
     "is-docker": "^1.1.0",
+    "jest": "^24.1.0",
+    "jest-extended": "^0.11.1",
     "jscodeshift": "^0.5.1",
     "json-loader": "^0.5.4",
     "karma": "^2.0.0",
diff --git a/src/createApplicationStore.js b/src/createApplicationStore.js
@@ -4,10 +4,12 @@ import {
   applyMiddleware,
 } from 'redux';
 import createSagaMiddleware from 'redux-saga';
+import {createLogicMiddleware} from 'redux-logic';
 import get from 'lodash-es/get';
 
 import reducers from './reducers';
 import rootSaga from './sagas';
+import rootLogic from './logic';
 import {bugsnagClient} from './util/bugsnag';
 
 const compose = get(
@@ -26,10 +28,16 @@ export default function createApplicationStore() {
       bugsnagClient.notify(error);
     },
   });
+  const sagaEnhancer = applyMiddleware(sagaMiddleware);
+
+  const logicMiddleware = createLogicMiddleware(rootLogic);
+  const logicEnhancer = applyMiddleware(logicMiddleware);
+
   const store = createStore(
     reducers,
-    compose(applyMiddleware(sagaMiddleware)),
+    compose(sagaEnhancer, logicEnhancer),
   );
   sagaMiddleware.run(rootSaga);
+
   return store;
 }
diff --git a/src/logic/__tests__/unlinkGithubIdentityTest.js b/src/logic/__tests__/unlinkGithubIdentityTest.js
@@ -0,0 +1,12 @@
+import unlinkGithubIdentity from '../unlinkGithubIdentity';
+
+import {unlinkGithub} from '../../clients/firebase';
+
+jest.mock('../../clients/firebase.js');
+
+test('should unlink Github Identity', async() => {
+  const {type, payload: {providerId}} = await unlinkGithubIdentity.process();
+  expect(unlinkGithub).toHaveBeenCalledWith();
+  expect(type).toBe('IDENTITY_UNLINKED');
+  expect(providerId).toBe('github.com');
+});
diff --git a/src/logic/index.js b/src/logic/index.js
@@ -0,0 +1,3 @@
+import unlinkGithubIdentity from './unlinkGithubIdentity';
+
+export default [unlinkGithubIdentity];
diff --git a/src/logic/unlinkGithubIdentity.js b/src/logic/unlinkGithubIdentity.js
@@ -0,0 +1,12 @@
+import {createLogic} from 'redux-logic';
+
+import {unlinkGithub} from '../clients/firebase';
+import {identityUnlinked} from '../actions/user';
+
+export default createLogic({
+  type: 'UNLINK_GITHUB_IDENTITY',
+  async process() {
+    await unlinkGithub();
+    return identityUnlinked('github.com');
+  },
+});
diff --git a/src/sagas/user.js b/src/sagas/user.js
@@ -16,15 +16,13 @@ import {
   identityLinked,
   linkIdentityFailed,
   accountMigrationError,
-  identityUnlinked,
 } from '../actions/user';
 import {getCurrentAccountMigration} from '../selectors';
 import {
   linkGithub,
   migrateAccount,
   signOut,
   saveCredentialForCurrentUser,
-  unlinkGithub,
 } from '../clients/firebase';
 import {getProfileForAuthenticatedUser} from '../clients/github';
 
@@ -51,11 +49,6 @@ export function* linkGithubIdentity() {
   }
 }
 
-export function* unlinkGithubIdentity() {
-  yield call(unlinkGithub);
-  yield put(identityUnlinked('github.com'));
-}
-
 export function* startAccountMigration() {
   const {shouldContinue} = yield race({
     shouldContinue: delay(5000, true),
@@ -90,7 +83,6 @@ export function* logOut() {
 export default function* user() {
   yield all([
     takeEvery('LINK_GITHUB_IDENTITY', linkGithubIdentity),
-    takeEvery('UNLINK_GITHUB_IDENTITY', unlinkGithubIdentity),
     takeEvery('LOG_OUT', logOut),
     takeEvery('START_ACCOUNT_MIGRATION', startAccountMigration),
   ]);
diff --git a/test/unit/sagas/user.js b/test/unit/sagas/user.js
@@ -12,8 +12,6 @@ import {
   linkIdentityFailed,
   logOut,
   startAccountMigration,
-  unlinkGithubIdentity,
-  identityUnlinked,
 } from '../../../src/actions/user';
 import {getCurrentAccountMigration} from '../../../src/selectors';
 import {
@@ -24,13 +22,11 @@ import {
   migrateAccount,
   signOut,
   saveCredentialForCurrentUser,
-  unlinkGithub,
 } from '../../../src/clients/firebase';
 import {getProfileForAuthenticatedUser} from '../../../src/clients/github';
 import {
   logOut as logOutSaga,
   linkGithubIdentity as linkGithubIdentitySaga,
-  unlinkGithubIdentity as unlinkGithubIdentitySaga,
   startAccountMigration as startAccountMigrationSaga,
 } from '../../../src/sagas/user';
 import {bugsnagClient} from '../../../src/util/bugsnag';
@@ -159,13 +155,6 @@ test('startAccountMigration', (t) => {
   });
 });
 
-test('unlinkGithubIdentity', (assert) => {
-  testSaga(unlinkGithubIdentitySaga, unlinkGithubIdentity).
-    next().call(unlinkGithub).
-    next().put(identityUnlinked('github.com'));
-  assert.end();
-});
-
 test('logOut', (assert) => {
   testSaga(logOutSaga, logOut()).
     next().call(signOut);
diff --git a/yarn.lock b/yarn.lock

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+const bugsnagReact = {};`
	`2`	`+export default bugsnagReact;`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+/* eslint-disable import/no-anonymous-default-export */`
	`2`	`+export default {};`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+const database = {};`
	`2`	`+export default database;`