Use jest for unittesting - standardize unit testing epic #904

ndelangen · 2017-04-16T15:51:05Z

Issue:

What I did

UPDATED a lot of packages
ADDED jest tested to the root
REMOVED unit-testing dependencies from packages
CHANGED tests to run in jest
MOVED examples out of packages

So far, we've set up the tools and have one failing test inside storyshots.

The mocks here a bit complicated, so I'm leaving them for later

index

# Conflicts: # .eslintrc # .gitignore # package.json # packages/addon-knobs/package.json # packages/addon-knobs/yarn.lock # packages/react-storybook/package.json # packages/storybook-ui/package.json # packages/storybook-ui/src/modules/api/actions/__tests__/api.js # packages/storybook-ui/src/modules/shortcuts/actions/__tests__/shortcuts.js # packages/storybook-ui/src/modules/ui/actions/__tests__/ui.js # packages/storybook-ui/src/modules/ui/configs/__tests__/handle_keyevents.js # packages/storybook-ui/src/modules/ui/configs/__tests__/handle_routing.js # packages/storybook-ui/src/modules/ui/configs/__tests__/init_panels.js # packages/storyshots/package.json # packages/storyshots/stories/__test__/__snapshots__/storyshots.test.js.snap

This reverts commit a2c7a4a.

This reverts commit 1cbf37b.

shilman

Approved but with some caviats. See comments.

shilman · 2017-04-16T19:01:38Z

.travis.yml

 notifications:
  email: false
 node_js:
  - "node"
 before_install: ./scripts/travis/before_install.sh
 after_success: ./scripts/travis/after_success.sh
-script: npm run lint && npm run test
+script:
+  - "npm run lint"


@ndelangen remove " marks? see https://docs.travis-ci.com/user/customizing-the-build

shilman · 2017-04-16T21:48:46Z

packages/react-storybook/src/client/preview/client_api.test.js

-import { expect } from 'chai';
-import ClientAPI from '../client_api';
-
-const { describe, it } = global;


@ndelangen don't we want this across the board?

What do you mean here @shilman?

shilman · 2017-04-16T21:49:58Z

packages/storybook-ui/src/modules/shortcuts/actions/shortcuts.test.js

@@ -1,6 +1,4 @@
-import actions from '../shortcuts';
-import { expect } from 'chai';
-const { describe, it } = global;


shilman · 2017-04-16T21:50:34Z

packages/storybook-ui/src/modules/ui/containers/layout.test.js


 describe('manager.ui.containers.layout', () => {
  describe('mapper', () => {
-    it('should give correct data', () => {
+    test('should give correct data', () => {


why test? should be it?

test is the default and mentioned all over the documentation.

shilman · 2017-04-16T21:51:23Z

packages/storyshots/stories/__test__/storyshots.test.js

@@ -1,2 +0,0 @@
-import initStoryshots from '../../src';


are we not testing storyshots? is this temporary?

The way they were tested was just a smoke test, which breaks now because the main index isn't executed within the context of that package's package.json

yes temporary indeed, the implementation was a bad smoke-test. Someone needs to do a thorough analysis on how to test it.

shilman

If I change .eslintrc.js to extend airbnb-base, it shows me lots of linting errors.

  extends: [
    'airbnb-base',
  ],

I believe the current configuration is incorrectly swallowing a lot of errors.

aaronmcadam · 2017-04-17T12:19:22Z

@ndelangen I see you're trying out yerna, what was the motivation there? The author himself says it's quite a hacky solution.

ndelangen · 2017-04-17T14:41:54Z

Ok, @shilman I agree, linting can be made much more strict then it is now. I will setup a new branch and PR for that. I think we can merge this and then start working on improving linting?

shilman · 2017-04-17T18:02:37Z

OK @ndelangen I agree with that solution, because we are doing heavy lifting on the monorepo transition and it's better to have these big changes merged. But FYI, I added a line in some source file that said only foo and the linter didn't throw any error. So I think it's a high priority change. I'm glad to see you're already working on it.

aaronmcadam and others added 30 commits April 9, 2017 18:39

chore(tests): migrate to Jest

9986d94

So far, we've set up the tools and have one failing test inside storyshots.

fix(addon-knobs): remove mocha type defs

cde14c7

refactor(react-storybook): use Jest matchers instead of chai

146f5a8

refactor(storybook-ui): use Jest matchers in index tests

30c9ba8

refactor(storybook-ui): use Jest matchers in API tests

e53965c

remove storyshots "test"

a2a1817

fix(package.json): move Jest config to .jestrc

27a8c68

fix: remove all Yarn lock files

cc9f29b

let NPM resolve the path to the snapshot serializer

7a1c030

fix(storybook-ui): use Jest mocks in ui test

fb65019

fix(storybook-ui): use Jest matchers, leave Sinon mocks

3178d70

The mocks here a bit complicated, so I'm leaving them for later

fix(storybook-ui): use Jest mocks in gen_podda_loader

bea3ce0

fix(storybook-ui): use Jest matchers in filters

ac57c94

fix(storybook-ui): use Jest matchers in shortcuts

5ff3b08

fix(storybook-ui): use Jest mocks in handle_keyevents

f5c2978

fix(storybook-ui): use Jest mocks in init_panels

41badd3

fix(storybook-ui): use Jest matchers in down_panel

6ec6bc5

fix(storybook-ui): use Jest matchers in shortcuts_help

56aeb49

update handle_keyevents

94c9035

fix(storybook-ui): use Jest matchers in left_panel

c0d4c54

fix(storybook-ui): use Jest matchers and mocks in handle_routing

ab26aed

fix(storybook-ui): use Jest mocks and enzyme matchers

226cba2

fix(storybook-ui): use Jest matchers in layout

9f6d1e7

fix(storybook-ui): use Jest mocks and enzyme matchers in left_panel

751de4c

index

fix(storybook-ui): use Jest mocks in left_panel header

86680cb

fix(storybook-ui): use jest matchers in layout index

7fdaf84

fix(storybook-ui): use Jest matchers in stories

52a1052

fix(storybook-ui): use Jest and enzyme matchers in text_filter

26b96ab

CHANGE packages/channel to jest

95d696e

ndelangen added 3 commits April 16, 2017 22:49

CHANGE to use yerna over lerna

1cbf37b

CHANGE code coverage to blacklist generators code

b4cbbbd

MOVE documentation site outside packages

50c61f1

ndelangen force-pushed the jest-updated branch 3 times, most recently from 6f6a652 to 13b0938 Compare April 16, 2017 21:08

FIX wrong yerna package

a2c7a4a

ndelangen force-pushed the jest-updated branch from 13b0938 to a2c7a4a Compare April 16, 2017 21:19

ndelangen added 3 commits April 16, 2017 23:33

Revert "FIX wrong yerna package"

783f342

This reverts commit a2c7a4a.

Revert "CHANGE to use yerna over lerna"

f87d377

This reverts commit 1cbf37b.

Merge branch 'master' into jest-updated

bfd9d26

shilman approved these changes Apr 16, 2017

View reviewed changes

shilman requested changes Apr 16, 2017

View reviewed changes

ndelangen mentioned this pull request Apr 17, 2017

Set ESlint rules more strict 🚑 #911

Merged

14 tasks

shilman approved these changes Apr 17, 2017

View reviewed changes

shilman merged commit 0792854 into master Apr 17, 2017

shilman removed the in progress label Apr 17, 2017

ndelangen deleted the jest-updated branch April 28, 2017 16:03

theinterned mentioned this pull request May 8, 2017

Set ESlint rules more strict 🚑 #1000

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use jest for unittesting - standardize unit testing epic #904

Use jest for unittesting - standardize unit testing epic #904

ndelangen commented Apr 16, 2017 •

edited

Loading

shilman left a comment

shilman Apr 16, 2017

shilman Apr 16, 2017

aaronmcadam Apr 17, 2017

shilman Apr 16, 2017

shilman Apr 16, 2017

ndelangen Apr 17, 2017

aaronmcadam Apr 17, 2017

shilman Apr 16, 2017

aaronmcadam Apr 17, 2017

ndelangen Apr 17, 2017

shilman left a comment

aaronmcadam commented Apr 17, 2017

ndelangen commented Apr 17, 2017

shilman commented Apr 17, 2017

Use jest for unittesting - standardize unit testing epic #904

Use jest for unittesting - standardize unit testing epic #904

Conversation

ndelangen commented Apr 16, 2017 • edited Loading

What I did

shilman left a 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

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

shilman left a comment

Choose a reason for hiding this comment

aaronmcadam commented Apr 17, 2017

ndelangen commented Apr 17, 2017

shilman commented Apr 17, 2017

ndelangen commented Apr 16, 2017 •

edited

Loading