Jump to content


Photo

General JavaScript


  • Please log in to reply
443 replies to this topic

#441 citypaul

citypaul

    Privileged

  • Privileged
  • PipPipPipPip
  • 843 posts
  • Gender:Male
  • Location:Manchester
  • Experience:Nothing
  • Area of Expertise:Web Developer

Posted 13 August 2017 - 09:38 AM

Look at using something like JsDom or mock out window references.

I went he mocking route.
Here I create some aliases to the window globals that I wish to mock
 

const addEListener = (e, cb) =>
  window.addEventListener(e, cb)

const removeEListener = (e, cb) =>
  window.removeEventListener(e, cb)

export {
  addEListener,
  removeEListener
}

I consume them in this component

 

import { Component, createElement as E } from 'react'
import T from 'prop-types'
import { classNames, addEListener, removeEListener } from '../../utils'

class NavbarDropdownToggle extends Component {
  constructor() {
    super()

    this.state = {
      isOpen: false
    }
  }

  componentDidMount() {
    addEListener('click', this.handleClickOutside)
  }

  componentWillUnmount() {
    removeEListener('click', this.handleClickOutside)
  }

  handleClick = () =>
    this.setState({
      isOpen: !this.state.isOpen
    })


  handleClickOutside = ({ target }) =>
    this.setState({
      isOpen: !this.button.contains(target)
        ? false
        : this.state.isOpen
    })

  render() {
    const { node, children, className, ...rest } = this.props
    return E(
      node || 'button',
      {
        onClick: this.handleClick,
        ref: (n) => { this.button = n },
        className: classNames(
          'c-navbar__dropdown-toggle', className,
          { 'is-open': this.state.isOpen }
        ),
        ...rest
      },
      children
    )
  }
}

NavbarDropdownToggle.propTypes = {
  className: T.string,
  node: T.string,
  children: T.node.isRequired
}

export default NavbarDropdownToggle

And tested here
 

import React from 'react'
import { shallow, mount } from 'enzyme'

import { Navbar } from '../'

import { addEListener, removeEListener } from '../../../utils/window'

jest.mock('../../../utils/window')

describe('<Navbar.Dropdown.Toggle />', () => {
  it('adds a click handler to the window when mounted and removes it when it unmounts', () => {
    const mockAddEventListener = jest.fn()
    const mockRemoveEventListener = jest.fn()
    addEListener.mockImplementation(mockAddEventListener)
    removeEListener.mockImplementation(mockRemoveEventListener)

    const $ = mount(<Navbar.Dropdown.Toggle>_</Navbar.Dropdown.Toggle>)
    const handleClickOutside = $.instance().handleClickOutside
    expect(mockAddEventListener).toHaveBeenCalledWith('click', handleClickOutside)
    $.unmount()
    expect(mockRemoveEventListener).toHaveBeenCalledWith('click', handleClickOutside)
  })
})


This is in Jest so the syntax will be different.  For Mocha you will need to bring in Sinon to mock.  These are reasons I use Jest because all this stuff just comes out of the box.

 

Are you testing that the DOM updates when the state of isOpen changes?

 

TBH I don't think I would explicitly check the event handler stuff at all there, at least not for setting the initial setup in componentDidMount. Isn't the point of that click handler to set the isOpen state when someone clicks? Can't you just test that entire thing by using Enzyme's simulate method with a click event, and prove that the `isOpen` state has the intended impact on the DOM? That way you are not tying your implementation detail to your test, and your test explicitly tests the thing you care about, which is that when someone clicks on an element, something happens in the DOM.

 

I much prefer the black box style of testing favoured by the people like Kent Beck and Martin Fowler who popularised TDD. 

 

The problem with the way the test currently is is that it's tightly coupled to the implementation, and it doesn't really test what appears to matter. Also, you've had to create an entirely new component under the hood just to allow the internals to be mocked, which again is a bit of a code/test smell to me. Sometimes these things may be necessary, but rarely in my experience.

 

If you focus your tests on behaviours that matter, you can refactor the internals on a whim with the confidence that comes from black box style testing. 



#442 citypaul

citypaul

    Privileged

  • Privileged
  • PipPipPipPip
  • 843 posts
  • Gender:Male
  • Location:Manchester
  • Experience:Nothing
  • Area of Expertise:Web Developer

Posted 13 August 2017 - 09:41 AM

There's an example in the Jest docs that does something similar here. This uses jQuery to change the DOM, but the principle is the same - don't test the explicit setting of the click handler, but instead test the resulting output of triggering the event in terms of the effect it has on the DOM. 

 

https://facebook.git...ial-jquery.html


Edited by citypaul, 13 August 2017 - 09:42 AM.


#443 rbrtsmith

rbrtsmith

    ReferenceError

  • Privileged
  • PipPipPipPipPip
  • 3,991 posts
  • Gender:Male
  • Location:Manchester, UK
  • Experience:Nothing
  • Area of Expertise:Web Developer

Posted 14 August 2017 - 07:41 AM

AFAIK Enzyme's simulate method only works on React nodes and not on real DOM nodes, as I am testing here for click events  on the actual document itself itself.  The expected behaviour is that if you click anywhere on the document that is not this component or a descendant of it, the state is updated to set isOpen to false.
I am not sure how else to test the behaviour.

Whilst mocking might not be the best solution it does enable me to test that the component behaves as expected.  I'd be happy to see a better way of doing this :)


Edited by rbrtsmith, 14 August 2017 - 07:42 AM.


#444 Jack

Jack

    NaN

  • Moderators
  • PipPipPipPipPip
  • 3,144 posts
  • Gender:Male
  • Location:Jersey Channel Islands
  • Experience:Advanced
  • Area of Expertise:Web Designer

Posted 14 August 2017 - 08:01 AM

I've not done much of this kinda stuff using Jest yet (started using Jest for some pure JS stuff, but not had to build anything new in React since using it, so not quite as familiar with JSDOM). However, I've never had to mock out the window object like this before and it doesn't sound like a great idea to me.

 

I've tended to use Karma and Mocha/Jasmine in the past, and that gives you a real DOM to work with if you're combining it with something like PhantomJS. 

 

I'd be surprised if you have to mock out the window object in this way. I'd prefer to use a real DOM, or a DOM like representation that's so close to reality it might as well be the real thing.

 

If you're using Karma with Phantom then you have a real window object because you have a real DOM, and the problem goes away.

 

Good point, I could probably use Chrome's new headless browser for this. Do you find any performance decreases testing with a headless browser rather than just Node?






0 user(s) are reading this topic

0 members, 0 guests, 0 anonymous users