Skip to content

Conversation

@sophiebits
Copy link
Collaborator

This module-level mock() seems to have been interfering with other tests (26 specs failing). We don't have any other tests that do a module-level mock() so I'm going to assume it isn't supported in the open-source test runner right now. I changed it so only ReactEventEmitter.handleTopLevel is mocked; doing so made ReactEventEmitter complain that TopLevelCallbackCreator wasn't defined so I switched the ReactMount references to use React directly so that ReactDefaultInjection would kick in properly.

With this, all the tests pass.

This module-level mock() seems to have been interfering with other tests (26 specs failing). We don't have any other tests that do a module-level mock() so I'm going to assume it isn't supported in the open-source test runner right now. I changed it so only ReactEventEmitter.handleTopLevel is mocked; doing so made ReactEventEmitter complain that TopLevelCallbackCreator wasn't defined so I switched the ReactMount references to use React directly so that ReactDefaultInjection would kick in properly.

With this, all the tests pass.
@benjamn
Copy link
Contributor

benjamn commented Nov 21, 2013

Can we fix this instead by making require("mock-modules").dumpCache() reset all .mock and .dontMock settings between tests?

@Daniel15
Copy link
Member

Sorry about this, I'm the developer that wrote the original tests. I did the initial development in the version we use internally at Facebook and only ran them with the internal test runner. I didn't notice that the test was broken in the open-source version as I didn't realise the test runner behaviour was different. The change looks fine to me but I'm very new to React development.

@sophiebits
Copy link
Collaborator Author

@Daniel15 No problem, easy mistake to make. It's not even really a fault of the test at all.

@benjamn
Copy link
Contributor

benjamn commented Nov 21, 2013

@Daniel15 yeah, to be clear, you helped illuminate a problem in the open-source test runner. Thanks!

I'm going to merge this to get tests passing again, and then keep looking into the mocking problems. If I can fix those in another way, I'll reinstate the mocking of ReactEventEmitter in a follow-up PR.

benjamn added a commit that referenced this pull request Nov 21, 2013
Make ReactEventTopLevelCallback-test pass
@benjamn benjamn merged commit 566f8b2 into facebook:master Nov 21, 2013
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

Successfully merging this pull request may close these issues.

3 participants