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

[test] Jest snapshot testing "getNodeFromInstance: Invalid argument" #8739

Closed
1 task done
sakulstra opened this issue Oct 18, 2017 · 7 comments
Closed
1 task done
Assignees
Labels
bug 🐛 Something doesn't work test

Comments

@sakulstra
Copy link
Contributor

I currently use snapshot testing for a project, but it seems like as soon as i'm using a component which relies on material-ui/Button the tests fail with.

Invariant Violation: getNodeFromInstance: Invalid argument.
      
      at invariant (node_modules/fbjs/lib/invariant.js:42:15)
      at Object.getNodeFromInstance (node_modules/react-dom/lib/ReactDOMComponentTree.js:160:77)
      at findDOMNode (node_modules/react-dom/lib/findDOMNode.js:47:41)
      at ButtonBase.componentDidMount (node_modules/material-ui/ButtonBase/ButtonBase.js:222:47)
      at node_modules/react-test-renderer/lib/ReactCompositeComponent.js:262:25
      at measureLifeCyclePerf (node_modules/react-test-renderer/lib/ReactCompositeComponent.js:73:12)
      at node_modules/react-test-renderer/lib/ReactCompositeComponent.js:261:11
      at CallbackQueue.notifyAll (node_modules/react-test-renderer/lib/CallbackQueue.js:74:22)
      at ReactTestReconcileTransaction.close (node_modules/react-test-renderer/lib/ReactTestReconcileTransaction.js:34:26)
      at ReactTestReconcileTransaction.closeAll (node_modules/react-test-renderer/lib/Transaction.js:207:25)
      at ReactTestReconcileTransaction.perform (node_modules/react-test-renderer/lib/Transaction.js:154:16)
      at batchedMountComponentIntoNode (node_modules/react-test-renderer/lib/ReactTestMount.js:67:27)
      at ReactDefaultBatchingStrategyTransaction.perform (node_modules/react-test-renderer/lib/Transaction.js:141:20)
      at Object.batchedUpdates (node_modules/react-test-renderer/lib/ReactDefaultBatchingStrategy.js:60:26)
      at Object.batchedUpdates (node_modules/react-test-renderer/lib/ReactUpdates.js:95:27)
      at Object.render [as create] (node_modules/react-test-renderer/lib/ReactTestMount.js:126:18)
      at Object.test (node_modules/@storybook/addon-storyshots/dist/test-bodies.js:26:44)
      at Object.<anonymous> (node_modules/@storybook/addon-storyshots/dist/index.js:145:21)
          at Promise (<anonymous>)
          at <anonymous>

Expected Behavior

Snapshot testing is possible with all components.

Current Behavior

Snapshot testing isn't possible for some components.

Steps to Reproduce (for bugs)

Write a snapshot test for e.g. import Hidden from 'material-ui/Hidden'; and one for import Button from 'material-ui/Button'; (is there a pastebin which is capable of this?)

Context

I tried using storyshots for snapshot testing of components using material-ui components.

Tech Version
Material-UI beta.17
React 15.6
@oliviertassinari
Copy link
Member

@sakulstra Could you provide a reproduction project? I can't see why the snapshot testing wouldn't work. Could it be a jest issue?

@sakulstra
Copy link
Contributor Author

@oliviertassinari it probably is, yes. Upgrading react/react-dom and react-test-renderer to v16 solves the issue for me(sadly i can't just do this). I'll dig a bit deeper and setup a test repo tomorrow if i find some time.

@pelotom
Copy link
Member

pelotom commented Oct 19, 2017

@sakulstra this is a shot in the dark based on the error message and the fact that you're using storyshots: you might need to provide the createNodeMock option, as illustrated here.

@sakulstra
Copy link
Contributor Author

sakulstra commented Oct 19, 2017

@pelotom sadly no, but I think I located the problem: facebook/react#7371
Seems like findDOMNode like used in ButtonBase:

value: function componentDidMount() {
      this.button = (0, _reactDom.findDOMNode)(this);
      (0, _keyboardFocus.listenForFocusKeys)();
    }

isn't supported be react-test-renderer (awkward that it works with v16, can't find any docs stating a change here)

The fix for me was to mock FindDOMNode like stated in one of the comments:

jest.mock('react-dom', () => ({
  findDOMNode: () => {}
}));

Not sure if this is the best solution, but it should be sufficient till we switch over to v16.

@oliviertassinari I think this issue can be closed, but does it make sense to include some info in the docs? I've seen there is a poll about dropping v15 support, so the docs would probably be obsolete soon.

@oliviertassinari
Copy link
Member

Isn't this issue linked to #7710? The behavior changed with https://github.com/callemall/material-ui/pull/8218/files.

@sakulstra
Copy link
Contributor Author

sakulstra commented Oct 19, 2017

yep seems to be related :/ didn't find it in the issue list.
here's the reproduction repo: https://github.com/sakulstra/material-ui-snapshots

@oliviertassinari oliviertassinari self-assigned this Oct 20, 2017
@oliviertassinari oliviertassinari changed the title [Bug] Jest snapshot testing [test] Jest snapshot testing "getNodeFromInstance: Invalid argument" Oct 20, 2017
@oliviertassinari oliviertassinari added the bug 🐛 Something doesn't work label Oct 20, 2017
@oliviertassinari
Copy link
Member

@sakulstra Thanks a lot for the reproduction example! I confirm, it's broken with react@15 and it's working fine with react@16. We could have added some warnings internally, but given the issue is fixed upstream, I think that it's simpler to close the issue. People facing the issue can easily find the issue and the solution here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work test
Projects
None yet
Development

No branches or pull requests

3 participants