Adding Jest #40

gautamarora · 2018-01-22T23:03:58Z

A exploratory PR for using Jest for testing and coverage.

Description

This PR replaces chai, mocha, sinon, nyc with jest.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Chore, documentation, cleanup

Related Issue

#37

Motivation and Context

Some of the pros of using Jest would be:

speed of testing
interactive watch mode
snapshot testing

How Has This Been Tested?

npm test
npm run test:watch

Screenshots (if appropriate):

Checklist:

My code follows the code style of this project.
I have updated the documentation (if required).
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.

p.s. Force push closed my last PR #38 😞 , so creating a new one.

taveras

LGTM!
Jest is compelling, and I will add a few snapshot tests in a follow-up.

let's hold off on merging this until #19 is merged, in order to migrate its tests to Jest as well.

taveras · 2018-01-23T14:57:36Z

test/helpers.test.js

      };
      const data = { foo: 'bar' };
      const { firstChild } = parseXML('<a x="1" y="two">hello</a>');
      visitNode(firstChild, 0, converters, data);
-      sinon.assert.calledWith(converters.a, { x: '1', y: 'two' }, data);
+      expect(converters.a.mock.calls.length).toBe(1);
+      expect(converters.a.mock.calls[0][0]).toEqual({ x: '1', y: 'two' });


interesting, mocking in Jest will be different -- i like it!

I think we can also write:

expect(converters.a).toHaveBeenCalledTimes(1); expect(converters.a).toHaveBeenCalledWith({ x: '1', y: 'two' });

@pgoldrbx - yes, certainly. fixed.

pgoldrbx · 2018-01-23T17:19:23Z

package.json

@@ -15,7 +15,8 @@
    "lint": "eslint .",
    "prepublishOnly": "npm run clean && npm run build",
    "pretest": "eslint .",
-    "test": "nyc mocha --require babel-register --recursive"
+    "test": "jest  --silent=true",
+    "test-watch": "jest  --silent=true --watch"


v.minor, there's an extra space after jest

good catch :)

pgoldrbx · 2018-01-23T17:20:25Z

test/XMLToReact.test.js

@@ -43,37 +41,37 @@ describe('XMLToReact class ', () => {
  };

  it('exports a module', () => {
-    expect(XMLToReact).to.be.a('function');
+    expect(XMLToReact).toEqual(expect.any(Function));


personally i'm fine with (typeof x).toEqual('function') as a pattern as well - but the mater works too :)

pgoldrbx · 2018-01-23T17:21:07Z

test/XMLToReact.test.js

-      expect(wrapper.exists()).to.equal(true);
-      expect(wrapper.find('.test > [fancy]')).to.have.length(1);
-      expect(visitNodeSpy.called).to.equal(true);
+      expect(wrapper.exists()).toBeTruthy();


how about .toEqual(true) for equivalent testing?

will resolve to use just .toEqual(true)

pgoldrbx · 2018-01-23T17:21:29Z

test/XMLToReact.test.js

@@ -82,8 +80,8 @@ describe('XMLToReact class ', () => {
          const xmltoreact = new XMLToReact(converters);
          const tree = xmltoreact.convert(badXML);

-          expect(tree).to.equal(null);
-          expect(visitNodeSpy.called).to.equal(false);
+          expect(tree).toEqual(null);


trivial - in some assertions below you use toBeNull - make it consistent? (i don't really care)

Fixed to .toEqual(null) everywhere

pgoldrbx · 2018-01-23T17:22:00Z

test/helpers.test.js

@@ -34,7 +32,7 @@ describe('helpers', () => {
        bar: () => {},
        baz: () => {},
      };
-      expect(validateConverters(converters)).to.equal(true);
+      expect(validateConverters(converters)).toBeTruthy();


my comments are still the same here - i'd like to see strict boolean assertion, .toEqual(true) for this and all below (everywhere)

pgoldrbx · 2018-01-23T20:47:10Z

test/helpers.test.js

-      expect(validateConverters(true)).to.equal(false);
-      expect(validateConverters(() => {})).to.equal(false);
-      expect(validateConverters(null)).to.equal(false);
+      expect(validateConverters()).not.toEqual(true);


shouldn't we just expect .toEqual(false) in this case?

pgoldrbx

🤡

* add environment and dependency prerequisites to README.md * change test command in readme * updated prerequisites * added testing and development environment details for CONTRIBUTING.md * change prerequisites to read as a sentence * Update CONTRIBUTING.md * update attribution heading for CONTRIBUTORING.md

pgoldrbx

lgtm! 🤡

pgoldrbx · 2018-01-26T18:11:51Z

@gautamarora ah, codeship is failing on lint issues!


/home/rof/src/github.com/CondeNast/xml-to-react/jest.config.js
11:22  error  Missing trailing comma  comma-dangle
12:6   error  Missing trailing comma  comma-dangle

pgoldrbx · 2018-02-04T21:44:05Z

@gautamarora looks like this one is good to merge?

gautamarora · 2018-02-04T21:48:59Z

@pgoldrbx - @taveras wanted to hold, his comment earlier...

let's hold off on merging this until #19 is merged, in order to migrate its tests to Jest as well.

* Move xmldom parsing to internal interface * fix lint errors * add changelog update

pgoldrbx · 2018-02-05T06:06:28Z

merged!

into feature/jest * 'feature/jest' of https://github.com/CondeNast/xml-to-react: linter strikes again Updating mock call checks Fix linting issues Making jest go faster + have cov thresholds Fix "not equal to true" tests to be "equal to false" Replace toBeTruthy()/toBeNull() with strict boolean checks Replace toBeTruthy()/toBeNull() with strict boolean checks Adding Jest

into feature/jest * 'feature/jest' of https://github.com/CondeNast/xml-to-react: Fix linting issues Making jest go faster + have cov thresholds Replace toBeTruthy()/toBeNull() with strict boolean checks Replace toBeTruthy()/toBeNull() with strict boolean checks Adding Jest linter strikes again Updating mock call checks Fix linting issues Making jest go faster + have cov thresholds Fix "not equal to true" tests to be "equal to false" Replace toBeTruthy()/toBeNull() with strict boolean checks Replace toBeTruthy()/toBeNull() with strict boolean checks Adding Jest

pgoldrbx · 2018-02-13T20:43:06Z

CHANGELOG.md

@@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.

 ## [Unreleased]
 - Initial Release
+- Changed XML parsing to use an internal interface.


🤷‍♂️ still part of our initial release

🤷‍♂️

pgoldrbx · 2018-02-13T20:43:43Z

package.json

@@ -15,7 +15,8 @@
    "lint": "eslint .",
    "prepublishOnly": "npm run clean && npm run build",
    "pretest": "eslint .",
-    "test": "nyc mocha --require babel-register --recursive"
+    "test": "jest  --silent=true",


trivial, extra space after jest

pgoldrbx · 2018-02-13T20:44:28Z

src/parser.js

@@ -0,0 +1,33 @@
+import { DOMParser } from 'xmldom';
+
+export const ERR_INVALID_XML = 'XMLToReact: Unable to parse invalid XML input. Please input valid XML.';


did this not get rebased properly? is that why it's showing up in this PR?

yeah, not sure what happened here, certainly was rebased.

i added a // eslint-disable-line no-console for the console.warn, but nothing else.

hmmm... because this was merged to master already
a08a366

it's not a big deal but i guess you have a merge commit in here or something

pgoldrbx

lgtm - a couple minor things but nothing blocking merge

pgoldrbx

i think this is good to go. let's do it!

Adding Jest

5de18d9

gautamarora requested review from pgoldrbx and taveras January 22, 2018 23:04

gautamarora added the enhancement label Jan 23, 2018

taveras approved these changes Jan 23, 2018

View reviewed changes

pgoldrbx reviewed Jan 23, 2018

View reviewed changes

gautamarora added 2 commits January 23, 2018 14:41

Replace toBeTruthy()/toBeNull() with strict boolean checks

d752527

Replace toBeTruthy()/toBeNull() with strict boolean checks

b52c775

pgoldrbx reviewed Jan 23, 2018

View reviewed changes

Fix "not equal to true" tests to be "equal to false"

e31fdd4

pgoldrbx approved these changes Jan 23, 2018

View reviewed changes

taveras and others added 2 commits January 25, 2018 18:15

Making jest go faster + have cov thresholds

7f16383

pgoldrbx approved these changes Jan 26, 2018

View reviewed changes

gautamarora added 3 commits January 26, 2018 13:14

Fix linting issues

0502cdf

Updating mock call checks

4a319f1

linter strikes again

d24b6fa

Move xmldom parsing to internal interface (#19)

a08a366

* Move xmldom parsing to internal interface * fix lint errors * add changelog update

gautamarora added 4 commits February 5, 2018 12:46

Adding Jest

1c87598

Replace toBeTruthy()/toBeNull() with strict boolean checks

02a70c8

Replace toBeTruthy()/toBeNull() with strict boolean checks

ed0457b

Making jest go faster + have cov thresholds

2929f28

gautamarora and others added 16 commits February 5, 2018 12:48

Fix linting issues

48dcc6a

Adding Jest

3fe218b

Replace toBeTruthy()/toBeNull() with strict boolean checks

f6f6658

Replace toBeTruthy()/toBeNull() with strict boolean checks

d0c6567

Fix "not equal to true" tests to be "equal to false"

a961c87

Making jest go faster + have cov thresholds

b6c9580

Fix linting issues

c10b932

Updating mock call checks

6d24198

linter strikes again

bc25bd4

Adding Jest

434822d

Replace toBeTruthy()/toBeNull() with strict boolean checks

f3fc48c

Making jest go faster + have cov thresholds

1b2d8b8

Fix linting issues

47c7dbe

Updating tests to jest for parser

7dfff4d

pgoldrbx reviewed Feb 13, 2018

View reviewed changes

pgoldrbx approved these changes Feb 13, 2018

View reviewed changes

Remove extra space

9cfb765

pgoldrbx approved these changes Feb 13, 2018

View reviewed changes

gautamarora merged commit 5ffff5b into master Feb 14, 2018

gautamarora deleted the feature/jest branch February 14, 2018 04:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding Jest #40

Adding Jest #40

gautamarora commented Jan 22, 2018 •

edited

Loading

taveras left a comment

taveras Jan 23, 2018

pgoldrbx Jan 26, 2018

gautamarora Jan 26, 2018

pgoldrbx Jan 23, 2018

gautamarora Jan 23, 2018

pgoldrbx Jan 23, 2018

pgoldrbx Jan 23, 2018

gautamarora Jan 23, 2018

pgoldrbx Jan 23, 2018

gautamarora Jan 23, 2018

pgoldrbx Jan 23, 2018 •

edited

Loading

pgoldrbx Jan 23, 2018

gautamarora Jan 23, 2018

pgoldrbx left a comment

pgoldrbx left a comment

pgoldrbx commented Jan 26, 2018

pgoldrbx commented Feb 4, 2018

gautamarora commented Feb 4, 2018

pgoldrbx commented Feb 5, 2018

pgoldrbx Feb 13, 2018

gautamarora Feb 13, 2018

pgoldrbx Feb 13, 2018

gautamarora Feb 13, 2018

pgoldrbx Feb 13, 2018 •

edited

Loading

gautamarora Feb 13, 2018

gautamarora Feb 13, 2018

pgoldrbx Feb 13, 2018

pgoldrbx left a comment

pgoldrbx left a comment

		@@ -0,0 +1,33 @@
		import { DOMParser } from 'xmldom';

		export const ERR_INVALID_XML = 'XMLToReact: Unable to parse invalid XML input. Please input valid XML.';

Adding Jest #40

Adding Jest #40

Conversation

gautamarora commented Jan 22, 2018 • edited Loading

Description

Types of changes

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Checklist:

taveras left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pgoldrbx Jan 23, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pgoldrbx left a comment

Choose a reason for hiding this comment

pgoldrbx left a comment

Choose a reason for hiding this comment

pgoldrbx commented Jan 26, 2018

pgoldrbx commented Feb 4, 2018

gautamarora commented Feb 4, 2018

pgoldrbx commented Feb 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pgoldrbx Feb 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pgoldrbx left a comment

Choose a reason for hiding this comment

pgoldrbx left a comment

Choose a reason for hiding this comment

gautamarora commented Jan 22, 2018 •

edited

Loading

pgoldrbx Jan 23, 2018 •

edited

Loading

pgoldrbx Feb 13, 2018 •

edited

Loading