From 651f1057409ba870e54ade53996375192369072a Mon Sep 17 00:00:00 2001 From: Ben Gotow Date: Wed, 18 Feb 2015 14:09:46 -0800 Subject: [PATCH] fix(tests): Clean up after ReactTestUtils, wipe ComponentRegistry between specs Summary: Why does message-list have default participants? No other packages do Component registry warns if mixin component name not found Clear the component registry between tests and wipe React elements inserted into DOM Everything should have a displayName, even you ComposerView Stub all ComponentRegistry dependencies, always Test Plan: Run all the tests at the same time Reviewers: evan Reviewed By: evan Differential Revision: https://review.inboxapp.com/D1201 --- .../composer/lib/composer-view.cjsx | 1 + .../spec/inbox-composer-view-spec.cjsx | 2 +- .../message-list/lib/message-list.cjsx | 12 ++----- .../message-list/lib/thread-participants.cjsx | 15 -------- .../message-list/spec/message-list-spec.cjsx | 36 ++++++++++++------- .../thread-list/spec/thread-list-spec.cjsx | 14 +++++++- spec-inbox/models/model-spec.coffee | 2 +- spec/spec-helper.coffee | 30 ++++++++++++++++ src/component-registry.coffee | 9 +++-- 9 files changed, 79 insertions(+), 42 deletions(-) delete mode 100644 internal_packages/message-list/lib/thread-participants.cjsx diff --git a/internal_packages/composer/lib/composer-view.cjsx b/internal_packages/composer/lib/composer-view.cjsx index 378222bdd..7af4aaf58 100644 --- a/internal_packages/composer/lib/composer-view.cjsx +++ b/internal_packages/composer/lib/composer-view.cjsx @@ -18,6 +18,7 @@ ParticipantsTextField = require './participants-text-field.cjsx' # simulate the effect of the parent re-rendering us module.exports = ComposerView = React.createClass + displayName: 'ComposerView' getInitialState: -> state = @getComponentRegistryState() diff --git a/internal_packages/composer/spec/inbox-composer-view-spec.cjsx b/internal_packages/composer/spec/inbox-composer-view-spec.cjsx index 9d688f3d7..7474622e0 100644 --- a/internal_packages/composer/spec/inbox-composer-view-spec.cjsx +++ b/internal_packages/composer/spec/inbox-composer-view-spec.cjsx @@ -29,7 +29,7 @@ textFieldStub = (className) -> focus: -> draftStoreProxyStub = (localId) -> - listen: -> # noop + listen: -> -> draft: -> new Message() changes: add: -> diff --git a/internal_packages/message-list/lib/message-list.cjsx b/internal_packages/message-list/lib/message-list.cjsx index c5825495a..a720800f5 100755 --- a/internal_packages/message-list/lib/message-list.cjsx +++ b/internal_packages/message-list/lib/message-list.cjsx @@ -2,8 +2,6 @@ _ = require 'underscore-plus' React = require 'react' MessageItem = require "./message-item.cjsx" -ThreadParticipants = require "./thread-participants.cjsx" - {Actions, ThreadStore, MessageStore, ComponentRegistry} = require("inbox-exports") module.exports = @@ -80,13 +78,9 @@ MessageList = React.createClass

{@state.current_thread.subject}

- {if Participants? - - else - - } + {for MessageListHeader in MessageListHeaders diff --git a/internal_packages/message-list/lib/thread-participants.cjsx b/internal_packages/message-list/lib/thread-participants.cjsx deleted file mode 100644 index d20abfc1b..000000000 --- a/internal_packages/message-list/lib/thread-participants.cjsx +++ /dev/null @@ -1,15 +0,0 @@ -_ = require 'underscore-plus' -React = require "react" - -module.exports = -ThreadParticipants = React.createClass - displayName: 'ThreadParticipants' - - render: -> -
- {@_formattedParticipants()} -
- - _formattedParticipants: -> - contacts = @props.thread_participants ? [] - _.map(contacts, (c) -> c.displayName()).join(", ") diff --git a/internal_packages/message-list/spec/message-list-spec.cjsx b/internal_packages/message-list/spec/message-list-spec.cjsx index 1e141dd5e..1d98c0e2b 100644 --- a/internal_packages/message-list/spec/message-list-spec.cjsx +++ b/internal_packages/message-list/spec/message-list-spec.cjsx @@ -16,6 +16,18 @@ TestUtils = React.addons.TestUtils InboxTestUtils, ComponentRegistry} = require "inbox-exports" +ComposerItem = React.createClass + render: ->
+ focus: -> + +AttachmentItem = React.createClass + render: ->
+ focus: -> + +ParticipantsItem = React.createClass + render: ->
+ focus: -> + MessageItem = proxyquire("../lib/message-item.cjsx", { "./email-frame": React.createClass({render: ->
}) }) @@ -24,15 +36,7 @@ MessageList = proxyquire("../lib/message-list.cjsx", { "./message-item.cjsx": MessageItem }) -ComposerItem = React.createClass - render: ->
- focus: -> -ComponentRegistry.register - name: 'Composer' - view: ComposerItem - MessageParticipants = require "../lib/message-participants.cjsx" -ThreadParticipants = require "../lib/thread-participants.cjsx" me = new Namespace( "name": "User One", @@ -175,14 +179,22 @@ test_thread = (new Thread).fromJSON({ }) describe "MessageList", -> - _resetMessageStore = -> + beforeEach -> + ComponentRegistry.register + name: 'Composer' + view: ComposerItem + ComponentRegistry.register + name: 'Participants' + view: ParticipantsItem + ComponentRegistry.register + name: 'AttachmentComponent' + view: AttachmentItem + MessageStore._items = [] MessageStore._threadId = null spyOn(MessageStore, "itemLocalIds").andCallFake -> {"666": "666"} - beforeEach -> - _resetMessageStore() @message_list = TestUtils.renderIntoDocument() @message_list_node = @message_list.getDOMNode() @@ -218,7 +230,7 @@ describe "MessageList", -> it "displays the thread participants on the page", -> items = TestUtils.scryRenderedComponentsWithType(@message_list, - ThreadParticipants) + ParticipantsItem) expect(items.length).toBe 1 it "focuses new composers when a draft is added", -> diff --git a/internal_packages/thread-list/spec/thread-list-spec.cjsx b/internal_packages/thread-list/spec/thread-list-spec.cjsx index 3177296ed..9114c9191 100644 --- a/internal_packages/thread-list/spec/thread-list-spec.cjsx +++ b/internal_packages/thread-list/spec/thread-list-spec.cjsx @@ -11,7 +11,8 @@ ReactTestUtils = _.extend ReactTestUtils, require("jasmine-react-helpers") ThreadStore, DatabaseStore, InboxTestUtils, - NamespaceStore} = require "inbox-exports" + NamespaceStore, + ComponentRegistry} = require "inbox-exports" ThreadListColumn = require("../lib/thread-list-column") @@ -21,6 +22,9 @@ ThreadListNarrowItem = require("../lib/thread-list-narrow-item.cjsx") ThreadListTabular = require("../lib/thread-list-tabular.cjsx") ThreadListTabularItem = require("../lib/thread-list-tabular-item.cjsx") +ParticipantsItem = React.createClass + render: ->
+ me = new Namespace( "name": "User One", "email": "user1@inboxapp.com" @@ -218,6 +222,10 @@ describe "ThreadListTabular", -> ThreadStore._resetInstanceVars() + ComponentRegistry.register + name: 'Participants' + view: ParticipantsItem + @thread_list = ReactTestUtils.renderIntoDocument( ) @@ -300,6 +308,10 @@ describe "ThreadListNarrow", -> new Promise (resolve, reject) -> resolve(test_threads()) ThreadStore._resetInstanceVars() + ComponentRegistry.register + name: 'Participants' + view: ParticipantsItem + @thread_list = ReactTestUtils.renderIntoDocument( ) diff --git a/spec-inbox/models/model-spec.coffee b/spec-inbox/models/model-spec.coffee index 07b00adbe..1453507e1 100644 --- a/spec-inbox/models/model-spec.coffee +++ b/spec-inbox/models/model-spec.coffee @@ -3,7 +3,7 @@ Attributes = require '../../src/flux/attributes' {isTempId} = require '../../src/flux/models/utils' _ = require 'underscore-plus' -fdescribe "Model", -> +describe "Model", -> describe "constructor", -> it "should accept a hash of attributes and assign them to the new Model", -> attrs = diff --git a/spec/spec-helper.coffee b/spec/spec-helper.coffee index 867de523c..084b6c1fb 100644 --- a/spec/spec-helper.coffee +++ b/spec/spec-helper.coffee @@ -21,6 +21,7 @@ clipboard = require 'clipboard' NamespaceStore = require "../src/flux/stores/namespace-store" Contact = require '../src/flux/models/contact' +{ComponentRegistry} = require "inbox-exports" atom.themes.loadBaseStylesheets() atom.themes.requireStylesheet '../static/jasmine' @@ -65,8 +66,35 @@ if specDirectory isCoreSpec = specDirectory == fs.realpathSync(__dirname) +# Override React.addons.TestUtils.renderIntoDocument so that +# we can remove all the created elements after the test completes. +React = require "react/addons" +ReactTestUtils = React.addons.TestUtils +ReactTestUtils.scryRenderedDOMComponentsWithAttr = (root, attrName, attrValue) -> + ReactTestUtils.findAllInRenderedTree root, (inst) -> + inst.props[attrName] and (!attrValue or inst.props[attrName] is attrValue) + +ReactTestUtils.findRenderedDOMComponentWithAttr = (root, attrName, attrValue) -> + all = ReactTestUtils.scryRenderedDOMComponentsWithAttr(root, attrName, attrValue) + if all.length is not 1 + throw new Error("Did not find exactly one match for data attribute: #{attrName} with value: #{attrValue}") + all[0] + +ReactElementContainers = [] +ReactTestUtils.renderIntoDocument = (element) -> + container = document.createElement('div') + ReactElementContainers.push(container) + React.render(element, container) + +ReactTestUtils.unmountAll = -> + for container in ReactElementContainers + React.unmountComponentAtNode(container) + ReactElementContainers = [] + beforeEach -> Grim.clearDeprecations() if isCoreSpec + ComponentRegistry._clear() + $.fx.off = true documentTitle = null atom.workspace = new Workspace() @@ -146,6 +174,8 @@ afterEach -> $('#jasmine-content').empty() unless window.debugContent + ReactTestUtils.unmountAll() + jasmine.unspy(atom, 'saveSync') ensureNoPathSubscriptions() waits(0) # yield to ui thread to make screen update more frequently diff --git a/src/component-registry.coffee b/src/component-registry.coffee index 5f3536595..e3e042659 100644 --- a/src/component-registry.coffee +++ b/src/component-registry.coffee @@ -16,17 +16,20 @@ getViewsByName = (components) -> [registered, requested] = component.split " as " requested ?= registered state[requested] = ComponentRegistry.findViewByName registered + if not state[requested]? + console.log("""Warning: No component found for #{requested} in + #{@constructor.displayName}. Loaded: #{Object.keys(registry)}""") state Mixin = getInitialState: -> - getViewsByName(@components) + getViewsByName.apply(@, [@components]) componentWillMount: -> - @_componentUnlistener = ComponentRegistry.listen (event) => + @_componentUnlistener = ComponentRegistry.listen => @setState getViewsByName(@components) - componentDidUnmount: -> + componentWillUnmount: -> @_componentUnlistener() # Internal representation of components