Move from AVA to Jest

[ericvera]

Removed all AVA references and migrated existing tests to Jest for both the generator and the generated template.

[skellock]

🥇

[skellock]

Ya, I'm not happy with our bootup time.

[jamonholmgren]

I'll be working on that soon.

[derekgreenberg]

Jest has its own mocks for some of the dependencies mocked in Tests/Setup.js

Perhaps Tests/Setup.js should be modified to use jest mocks instead. Example:

jest
.mock('react-native-device-info', () => {
  return { isTablet: jest.fn(() => { return false }) }
})
.mock('react-native-router-flux', () => {
  return {
    Actions: {'myScreen': () => {}},
    ActionConst: {RESET: 'reset'}
  }
})
.mock('react-native-keyboard-spacer', () => 'KeyboardSpacer')
.mock('react-native-i18n', () => {
  const english = require('../App/I18n/languages/english.json')
  const keys = require('ramda')
  const replace = require('ramda')
  const forEach = require('ramda')

  return {
    t: (key, replacements) => {
      let value = english[key]
      if (!value) return key
      if (!replacements) return value

      forEach((r) => {
        value = replace(`{{${r}}}`, replacements[r], value)
      }, keys(replacements))
      return value
    }
  }
})
.mock('StatusBar', () => 'StatusBar')

[jamonholmgren]

@ericvera What do you think of this suggestion?

[ericvera]

To be completely honest while I spent a lot of time troubleshooting issues with the mocks to get the transition I am by no means an expert. Is the suggestion to have those as examples that people can follow? or is it to mock all of RN? Unfortunately the documentation for RN+Jest was not the easiest to decode.

[derekgreenberg]

@ericvera The suggestion is to actually replace the Ava mocks with Jest mocks, so in other words, mock all of RN, since the goal is to transition from Ava to Jest.

When using Jest snapshot tests, for example, it's surprising how many RN modules are already mocked in Jest. The examples shown above include a mere 5 RN mocks. For a freshly ignited app, 'react-native-keyboard-spacer' is not necessary. 'react-native-i18n' is useful for testing components that use this library, but is harmless if it's never used in a test.

This PR uses enzyme's shallow to test the basic structure of a component, whereas the Jest mocks I've described above are critical to snapshot testing, which is well suited to the types of tests included in the ignite-ir-next templates. To determine if swapping out the Ava-style mocks for Jest-style mocks works with the changes in your PR - with the shallow type of tests rather than using snapshots - is a matter of trying it out. I'd be happy to do just that.

[derekgreenberg]

TOL: perhaps we can replace the enzyme-based 'shallow' tests with snapshot tests. It's not a difficult change to make, and provides an easy way for project team members to see how changes to components in a commit actually affect the render code - and is MUCH easier to maintain than having to rewrite the DOM traversing code with each change to the component.

[ericvera]

@derekgreenberg the PR to remove AVA was already approved and I believe is in master. It seems the question now is about:

• Whether to use enzyme 'shallow' tests vs. snapshots on the boilerplate

That should probably be a new issue.

I left enzyme as it was already in use and seemed powerful and flexible enough.

As for my opinion on enzyme vs. snapshot tests. My experience, with other snapshot based systems, is that as you develop the generated content will change resulting in having to re-generate and validate the snapshots. I have not used the jest snapshots so I am not sure how powerful/useful it is. You will also have to update DOM traversal based tests, but not as much. That may have to do with how one develops.

I think @skellock or someone from Infinite Red added enzyme so it would be good to get their feedback.

An option may be to provide one of each (snapshot and traversal tests) so that people know the options. Ideally with some doc on pros/cons of each approach.

[skellock]

I agree with @derekgreenberg. I think the snapshots replace almost everything we need from enzyme.

You can't get access to functions (because they don't serialize). But the majority of stateless functional component and PureComponents and (probably most of the Components) will be fine.

I haven't tested this, but I heard got it's face owned by react@16.0.0-beta.6 which is the required version in react-native@0.43 and react-native@0.44.

I say for now, we banish enzyme to the couch and make it earn its way back to bed.

[ericvera]

Unfortunately I don't have many spare cycles this week to work on this. @derekgreenberg you seem more familiar with snapshots so let me know if you want to take over from here.

[derekgreenberg]

@ericvera This PR has not yet been merged into master, but you are right about creating a new issue - dependent on this PR getting merged into master - to convert the existing component unit tests to use Jest snapshots.

I recently went through the process of converting a suite of enzyme-based tests in an Ignited app to use Jest snapshots and so far, it has proved itself to be easy to use. Whenever a developer modifies a component, the developer runs a script that I added to package.json:

"updateSnapshot": "jest --updateSnapshot"

The person who reviews a new PR looks at the diff of the snapshot and can see how the changes to the component affected the rendered output.

To @Steve's point, and as you pointed out in your suggestion ('An option may be to provide one of each (snapshot and traversal tests) so that people know the options. Ideally with some doc on pros/cons of each approach.'), as much as I'd like to kick Enzyme to the curb, snapshot tests will not cover functionality, especially when a component has state that is modified based on a user's interaction with the component. So the ideal would be to keep Enzyme around and do the following - after this PR is merged to master:

1. Add "updateSnapshot": "jest --updateSnapshot" as a script in package.json
2. If a snapshot test of an ignite-ir-next component covers 100% of the code, replace the unit test with a snapshot test
3. Otherwise, supplement the snapshot test with enzyme/shallow tests to exercise code that is not covered by the snapshot test. A simple example is a component with an onPress handler that invokes the handler that was passed in as a prop. The snapshot test does not cover this.

In summary, this PR is fine as is. After it is merged to master, I will add a new Issue to (a) add a script to package.json.ejs that makes it easy for users to update snapshots, (b) swap out the mocks to use Jest mocks (c) add snapshot tests and see if code coverage is 100% for each component (d) add enzyme/shallow unit tests to supplement the snapshot tests if necessary (e) if snapshot tests really cover 100% of the code of all components, add some enzyme/shallow tests anyway with code comments so users can learn how to use both snapshot and enzyme/shallow testing.

[skellock]

Thx all. Time to shine.