Unit tests with Jest + Enzyme and updates to Tabs component #315

valentijnnieman · 2018-09-26T19:46:32Z

This is a work-in-progress PR, rebased of off #299 - that needs to be merged first.
I'm comparing this branch to the update_scripts branch, so the commits are easier to reason about.

This adds support for Jest, a JS test runner, and Enzyme, testing tools for React. It allows us to write unit tests in JS for the components, hopefully lightening the load/dependency on our Selenium integration tests, and allowing for a more test-driven approach by running Jest in watch mode or as a VS-Code/Atom plugin.

I've written some basic unit tests for the Tabs component, and caught a couple of bugs that I've also fixed.

@plotly/dash

valentijnnieman · 2018-09-26T19:48:04Z

.babelrc

+        },
+        "test": {
+            "plugins": ["transform-object-rest-spread", "styled-jsx/babel-test"]


Here we use (a brand new) plugin babel-test, so that Jest will use Babel and styled-jsx, but won't parse the classes to jsx-1324 or whatever, so that we can make assertions on them!

valentijnnieman · 2018-09-26T19:48:47Z

.circleci/config.yml

+                  command: |
+                      . venv/bin/activate
+                      python --version
+                      npm run test-unit


This is the only real difference apart from formatting - running the new unit tests in CircleCI.

valentijnnieman · 2018-09-26T19:49:35Z

src/components/Tabs.react.js

+        this.NoChildrenError = {
+            name: 'NoChildrenError',
+            message: 'Tabs did not have any children Tab components!',
+        };


Not really sure about this - if you have suggestions on how to do error handling here in a better way, please let me know!

chriddyp · 2018-09-26T19:52:04Z

I've written some basic unit tests for the Tabs component, and caught a couple of bugs that I've also fixed.

Just curious, what bugs did you find that weren't covered in the integration tests?

rmarren1 · 2018-09-26T19:55:15Z

I was getting an exception when the Tabs children is empty. Does this test that case?

valentijnnieman · 2018-09-26T20:10:08Z

I've written some basic unit tests for the Tabs component, and caught a couple of bugs that I've also fixed.

Just curious, what bugs did you find that weren't covered in the integration tests?

This happens when you only have one Tab component as children

I haven't tested if these are actual bugs that happen in Dash though. But at minimum these unit tests make the JS code more stable and less prone to breaking.

T4rk1n · 2018-09-26T20:12:03Z

I think I broke dash 0.28.0 😞

Traceback (most recent call last):
  File "/home/circleci/project/venv/lib/python2.7/site-packages/flask/app.py", line 2292, in wsgi_app
    response = self.full_dispatch_request()
  File "/home/circleci/project/venv/lib/python2.7/site-packages/flask/app.py", line 1815, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/home/circleci/project/venv/lib/python2.7/site-packages/flask/app.py", line 1718, in handle_user_exception
    reraise(exc_type, exc_value, tb)
  File "/home/circleci/project/venv/lib/python2.7/site-packages/flask/app.py", line 1813, in full_dispatch_request
    rv = self.dispatch_request()
  File "/home/circleci/project/venv/lib/python2.7/site-packages/flask/app.py", line 1799, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/home/circleci/project/venv/lib/python2.7/site-packages/dash/dash.py", line 995, in _serve_default_favicon
    return flask.Response(pkgutil.get_data('dash', 'favicon.ico'),
  File "/usr/local/lib/python2.7/pkgutil.py", line 589, in get_data
    return loader.get_data(resource_name)
  File "/usr/local/lib/python2.7/pkgutil.py", line 252, in get_data
    with open(pathname, "rb") as file:
IOError: [Errno 2] No such file or directory: '/home/circleci/project/venv/lib/python2.7/site-packages/dash/favicon.ico'

valentijnnieman · 2018-09-26T20:12:53Z

@rmarren1 Actually no it doesn't. I was thinking about it, and thought that the Tabs component should throw an error if there are no children. What would it render? Thinking about it now, perhaps the Tabs component should not throw an exception when there are no children, in the cases where you set the children later through a callback...

rmarren1 · 2018-09-26T20:18:25Z

I think either would be acceptable, but let's use .isRequired consistently with this choice so that users will see a nicer error via the validation PR.

valentijnnieman · 2018-09-26T20:19:23Z

@rmarren1 Ah you're right, thanks! I'll update that.

valentijnnieman · 2018-09-26T20:35:07Z

The Selenium tests are failing here because of this issue: plotly/dash-renderer#84

valentijnnieman · 2018-09-26T20:37:22Z

@rmarren1 So I can remove these errors I'm throwing in favour of .isRequired and the upcoming validations?

rmarren1 · 2018-09-26T21:09:51Z

If you have .isRequired won't prop-types throw a nice exception similar to NoChildException anyway?

valentijnnieman · 2018-10-15T14:54:55Z

@rmarren1 Yeah that's what I expected, but if I set it to .isRequired it will always throw an error, even if there are children. I think there might be an issue where Dash doesn't immediately pass the children on to the dash-renderer instance of components, or something else is going wrong in that pipeline. In the new unit tests here it's running as expected, by the way.

… different from this.props

valentijnnieman · 2018-10-17T17:19:19Z

requirements-dev.txt

@@ -8,6 +8,7 @@ tox
 tox-pyenv
 six
 plotly>=2.0.8
+urllib3<=1.22


This caused the Python 2.7 tests to fail, got this fix from #333

valentijnnieman · 2018-10-17T17:20:06Z

src/components/Tab.react.js

@@ -18,7 +18,7 @@ Tab.propTypes = {
    /**
     * The tab's label
     */
-    label: PropTypes.children,
+    label: PropTypes.string,


Support for components/something else than a string for label will have to come later, anything else than a string doesn't really work now anyways, so best to set it to string for now.

chriddyp

Let's also bump the versions and ship a release as soon as this is merged

… into unit_tests_with_jest

valentijnnieman commented Sep 26, 2018

View reviewed changes

valentijnnieman added 7 commits October 15, 2018 11:19

Setup jest

b98f36b

Setup babel-test plugin and Tabs unit tests

15c6813

Rebuild components with Tabs changes

72ad1c3

Formatted .circleci config and added Jest step

8ef4a64

Refactored Tabs

9d74498

Add child.props logic back in

4368653

Better children prop validation and fix integration tests

e7272a9

valentijnnieman force-pushed the unit_tests_with_jest branch from a9b3b40 to e7272a9 Compare October 15, 2018 15:25

valentijnnieman changed the base branch from update_scripts to master October 15, 2018 15:26

valentijnnieman added 7 commits October 15, 2018 11:30

Reformatted Tabs.react.js

0da5ff8

Update styled-jsx so it includes babel-test plugin

518b7c8

Rewrite child props logic again

e933923

Reformat Tabs.react.js part 2

6e9af8d

Remove debugging statements

2a72658

Children props logic update

e0d21c2

Remove debugging statements part 999

f4b4aeb

Don't break if no children are set on Tabs

962c96f

valentijnnieman changed the title ~~[WIP] Unit tests with Jest + Enzyme~~ Unit tests with Jest + Enzyme and updates to Tabs component Oct 15, 2018

valentijnnieman added 2 commits October 15, 2018 15:41

Remove children from Tabs in integration_test

5a567cd

Only use state if setProps isn't defined, check if imcoming props are…

15b40d6

… different from this.props

valentijnnieman mentioned this pull request Oct 17, 2018

some issue with tabs reverting their value? #331

Closed

pin urllib3 to <=1.22 a la #333

fa9f711

valentijnnieman requested review from chriddyp and Marc-Andre-Rivet October 17, 2018 16:57

valentijnnieman commented Oct 17, 2018

View reviewed changes

chriddyp approved these changes Oct 17, 2018

View reviewed changes

valentijnnieman added 2 commits October 17, 2018 15:46

Bump version and change some comments

7116f65

Merge branch 'master' of https://github.com/plotly/dash-core-components…

390a499

… into unit_tests_with_jest

valentijnnieman merged commit 0c7ffb4 into master Oct 17, 2018

valentijnnieman deleted the unit_tests_with_jest branch October 17, 2018 20:15

Uh oh!

Unit tests with Jest + Enzyme and updates to Tabs component #315

Unit tests with Jest + Enzyme and updates to Tabs component #315

Uh oh!

Conversation

valentijnnieman commented Sep 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

valentijnnieman Sep 26, 2018

Choose a reason for hiding this comment

Uh oh!

valentijnnieman Sep 26, 2018

Choose a reason for hiding this comment

Uh oh!

valentijnnieman Sep 26, 2018

Choose a reason for hiding this comment

Uh oh!

chriddyp commented Sep 26, 2018

Uh oh!

rmarren1 commented Sep 26, 2018

Uh oh!

valentijnnieman commented Sep 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

T4rk1n commented Sep 26, 2018

Uh oh!

valentijnnieman commented Sep 26, 2018

Uh oh!

rmarren1 commented Sep 26, 2018

Uh oh!

valentijnnieman commented Sep 26, 2018

Uh oh!

valentijnnieman commented Sep 26, 2018

Uh oh!

valentijnnieman commented Sep 26, 2018

Uh oh!

rmarren1 commented Sep 26, 2018

Uh oh!

valentijnnieman commented Oct 15, 2018

Uh oh!

valentijnnieman Oct 17, 2018

Choose a reason for hiding this comment

Uh oh!

valentijnnieman Oct 17, 2018

Choose a reason for hiding this comment

Uh oh!

chriddyp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

valentijnnieman commented Sep 26, 2018 •

edited

Loading

valentijnnieman commented Sep 26, 2018 •

edited

Loading