Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unit test failures on React 15.2.1 #7231

Closed
jakepusateri opened this issue Jul 9, 2016 · 9 comments
Closed

Unit test failures on React 15.2.1 #7231

jakepusateri opened this issue Jul 9, 2016 · 9 comments

Comments

@jakepusateri
Copy link

jakepusateri commented Jul 9, 2016

Bug

Unit tests using shallow rendering fail on 'TypeError: Cannot read property 'onSetDisplayName' of null'.

I can't seem to get TestUtils into jsfiddle, but here is the build output and code snippet.

import expect from 'expect';
import TestUtils from 'react-addons-test-utils';
import React from 'react';
import BlurInput from '...';
...
    beforeEach(function () {
        props = {value: '1', onChange: function () {}};
        expect.spyOn(props, 'onChange');
        let renderer = TestUtils.createRenderer();
        renderer.render(<BlurInput {...props} />);
        instance = getInstance(renderer);
        componentTree = renderer.getRenderOutput();
    });
    it('reflects value to underlying input', function () {
        expect(componentTree.props.value).toBe('1');
        expect(instance.state.value).toBe('1');
    });


00:04:18.742   1) BlurInput "before each" hook for "reflects value to underlying input":
00:04:18.742      TypeError: Cannot read property 'onSetDisplayName' of null
00:04:18.742       at new ShallowComponentWrapper (node_modules/react/lib/ReactTestUtils.js:349:33)
00:04:18.742       at ReactShallowRenderer._render (node_modules/react/lib/ReactTestUtils.js:399:20)
00:04:18.742       at _batchedRender (node_modules/react/lib/ReactTestUtils.js:381:12)
00:04:18.742       at ReactDefaultBatchingStrategyTransaction.Mixin.perform (node_modules/react/lib/Transaction.js:138:20)
00:04:18.742       at Object.ReactDefaultBatchingStrategy.batchedUpdates (node_modules/react/lib/ReactDefaultBatchingStrategy.js:63:19)
00:04:18.742       at Object.batchedUpdates (node_modules/react/lib/ReactUpdates.js:98:20)
00:04:18.742       at ReactShallowRenderer.render (node_modules/react/lib/ReactTestUtils.js:374:16)
00:04:18.742       at Context.<anonymous> (BlurInput-test.js:19:18)

What is the expected behavior?
Unit test should run normally.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
15.2.1 has this issue.
15.2.0 does not.

@zpao
Copy link
Member

zpao commented Jul 9, 2016

I think this is because you are running a prod version of React in your tests. The testutils aren't entirely compatible with that (and generally expect that you're running in dev mode). This case in particular is going to blame to #7189 because we actually removed some dead code that wasn't used in production builds. The way did that removal broke your use case though.

cc @gaearon. The line at issue is ReactInstrumentation.debugTool.onSetDisplayName(this._debugID, displayName); which is going to fail because of https://github.com/facebook/react/pull/7189/files#diff-6bee1fd898dc3a6fef767ec0e6446e41R14

@gaearon
Copy link
Collaborator

gaearon commented Jul 9, 2016

Oops, I thought TestUtils are dev-only. Were we technically supporting running them in prod?

@jakepusateri
Copy link
Author

They had worked up until now, but it seems reasonable to not have them supported in production. It should be pretty simple to adjust my build to not set NODE_ENV=production during the test run.

@vvo
Copy link

vvo commented Jul 11, 2016

Was using NODE_ENV=production to avoid warnings in my npm test. I guess I will have to write better tests and code :)

@gaearon
Copy link
Collaborator

gaearon commented Jul 11, 2016

We’ll include a fix in the next version. Sorry I messed it up

@gaearon
Copy link
Collaborator

gaearon commented Jul 11, 2016

This will be fixed by #7246.

gaearon added a commit that referenced this issue Jul 14, 2016
I caused it with #7189.
We generally don’t recommend running TestUtils in production environment but this is technically a regression.

Fixes #7231.
kevgathuku added a commit to kevgathuku/docue that referenced this issue Jul 16, 2016
This should prevent React from throwing an error in cases where the
NODE_ENV is set to production
Ref: facebook/react#7231
kevgathuku added a commit to kevgathuku/docue that referenced this issue Jul 16, 2016
This is to avoid a regression in v15.2.1 where an error is being
thrown in cases where the production build is used during testing
Ref: facebook/react#7231
zpao pushed a commit that referenced this issue Jul 22, 2016
I caused it with #7189.
We generally don’t recommend running TestUtils in production environment but this is technically a regression.

Fixes #7231.
(cherry picked from commit 27d7592)
@sashafklein
Copy link

sashafklein commented Jul 29, 2016

@zpao @gaearon -- Is there a solution in mind going forward for CI testing? We run our tests on CircleCI, and want them to run in the production environment so they're as representative as possible of how the code will actually behave in production. Switching that test environment to dev feels like it would defeat one of the central purposes of a CI server.

@gaearon
Copy link
Collaborator

gaearon commented Jul 30, 2016

There should be no difference (except perf and warnings) between dev and prod versions of React. This is super important for us at Facebook and we are committed to keeping it this way. So I don't think running unit tests with prod env would be valuable to you.

@gaearon
Copy link
Collaborator

gaearon commented Jul 30, 2016

(End to end tests are a different matter but you wouldn't need TestUtils for them anyway.)

kevgathuku added a commit to kevgathuku/docue-frontend that referenced this issue Oct 26, 2016
This is to avoid a regression in v15.2.1 where an error is being
thrown in cases where the production build is used during testing
Ref: facebook/react#7231
kevgathuku added a commit to kevgathuku/docue that referenced this issue Oct 26, 2016
This is to avoid a regression in v15.2.1 where an error is being
thrown in cases where the production build is used during testing
Ref: facebook/react#7231
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants