New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds a draft of article for review #27

Closed
wants to merge 8 commits into
from

Conversation

Projects
None yet
2 participants
@pksjce
Contributor

pksjce commented Dec 1, 2016

With reference to #14
Just a draft I've been working on. Thanks for this opportunity :)

@pksjce

This comment has been minimized.

Show comment
Hide comment
@pksjce

pksjce Dec 5, 2016

Contributor

@cpojer - Would be really grateful for a quick review! 🙏

Contributor

pksjce commented Dec 5, 2016

@cpojer - Would be really grateful for a quick review! 🙏

@cpojer

This comment has been minimized.

Show comment
Hide comment
@cpojer

cpojer Dec 5, 2016

I'll do it! :)

cpojer commented Dec 5, 2016

I'll do it! :)

Show outdated Hide outdated 2016/notes-on-testing-with-jest.md
During the past 4 months, I have been working on the Flipkart desktop website team. For me it meant finally working on code at scale. If there was any place where TDD would be appreciated, it was here, on user facing critical code. The tests were being written on a setup of Karma, Mocha, Sinon and Enzyme. It worked pretty great, but the major peeve for all of us, was that, it was so slow!
The team had already tried using Jest before, had faced many issues with the automocking and given up. But then Jest made a turnaround and with a great community effort released improved and [new versions](https://facebook.github.io/jest/blog/2016/10/03/jest-16.html). From all the blogposts, we found that Jest's main focus was testing performance and something new called ["Snapshot testing"](https://facebook.github.io/jest/blog/2016/07/27/jest-14.html). Also, mainly it promised to be "Painless JavaScript Testing" with a special focus on React.

This comment has been minimized.

@cpojer

cpojer Dec 7, 2016

How about "Special focus on React but it really works for anything"? Jest really has nothing to do with React and it's how I'm trying to message it :)

@cpojer

cpojer Dec 7, 2016

How about "Special focus on React but it really works for anything"? Jest really has nothing to do with React and it's how I'm trying to message it :)

This comment has been minimized.

@pksjce

pksjce Dec 8, 2016

Contributor

I had tried to use this with deku but had faced problems with the renderer. Though at that time, I was very new to Jest. Let me try it out once again :)

@pksjce

pksjce Dec 8, 2016

Contributor

I had tried to use this with deku but had faced problems with the renderer. Though at that time, I was very new to Jest. Let me try it out once again :)

This comment has been minimized.

@cpojer

cpojer Dec 8, 2016

I think one of the things that points to this is that Jest uses Jest itself for both unit and integration testing and it's worked really well for us. We aren't likely to switch to another test runner ;P

@cpojer

cpojer Dec 8, 2016

I think one of the things that points to this is that Jest uses Jest itself for both unit and integration testing and it's worked really well for us. We aren't likely to switch to another test runner ;P

Show outdated Hide outdated 2016/notes-on-testing-with-jest.md
- I really liked the interactive CLI that comes with it. In `watch` mode, Jest runs only the tests associated with your code changes. But it allows you to interact with the tests by providing functionality to update snapshots, search for a particular suite, run all tests if you want to.
- [Great documentation](https://facebook.github.io/jest/docs/getting-started.html)
- Snapshot testing for components. It is kind of tailor made for React apps.
- Easy configuration and great defaults. Running `jest` alone would be enough to run test files. Mocking is not a marriage of complexity and confusion.

This comment has been minimized.

@cpojer

cpojer Dec 7, 2016

Maybe mention built-in coverage support?

@cpojer

cpojer Dec 7, 2016

Maybe mention built-in coverage support?

This comment has been minimized.

@cpojer

cpojer Dec 7, 2016

I guess maybe it would be even worth calling it "zero configuration"?

@cpojer

cpojer Dec 7, 2016

I guess maybe it would be even worth calling it "zero configuration"?

This comment has been minimized.

@pksjce

pksjce Dec 8, 2016

Contributor

Yep, coverage is awesome. Will include a bit about that.
"Zero configuration" is true actually!

@pksjce

pksjce Dec 8, 2016

Contributor

Yep, coverage is awesome. Will include a bit about that.
"Zero configuration" is true actually!

Show outdated Hide outdated 2016/notes-on-testing-with-jest.md
- [Great documentation](https://facebook.github.io/jest/docs/getting-started.html)
- Snapshot testing for components. It is kind of tailor made for React apps.
- Easy configuration and great defaults. Running `jest` alone would be enough to run test files. Mocking is not a marriage of complexity and confusion.
- Existing tests could be [easily migrated](https://facebook.github.io/jest/docs/migration-guide.html#content)

This comment has been minimized.

@cpojer

cpojer Dec 7, 2016

"easily migrated through automated codemods"?

@cpojer

cpojer Dec 7, 2016

"easily migrated through automated codemods"?

Show outdated Hide outdated 2016/notes-on-testing-with-jest.md
Snapshot testing, is a procedure in which, Jest generates readable snapshots of your component's view. A snapshot is basically a text file with xml, which acts like the source of your view, at that point in your testcase. Different snapshots can be created with different props to your components.
Snapshots can be saved after simulating events like `click` and `hover`. The point would be to save the expected view of your component which is a function of some mock data. If in the future, a developer changes your component, Jest would create a new snapshot of the component and compare it with the saved snapshot. If there is a change in view, it will alert the developer and she will have to tell Jest to save this new snapshot. Since the snapshot is committed to version control, it will show up in a code review and the changes can be approved accordingly.
While we still have reservations of, if its actually feasible to review large changes in the snapshot, I definitely think this enriches the content of a code review.

This comment has been minimized.

@cpojer

cpojer Dec 7, 2016

It also may be worth calling out that the integration we did is pretty substantial. For example Jest keeps track of outdated snapshots, obsolete snapshots and obsolete snapshot files. If you delete a test file but not the associated snapshot file, Jest will fail the test run and vice versa. It covers all of the cases to make sure the project stays consistent.

@cpojer

cpojer Dec 7, 2016

It also may be worth calling out that the integration we did is pretty substantial. For example Jest keeps track of outdated snapshots, obsolete snapshots and obsolete snapshot files. If you delete a test file but not the associated snapshot file, Jest will fail the test run and vice versa. It covers all of the cases to make sure the project stays consistent.

This comment has been minimized.

@pksjce

pksjce Dec 8, 2016

Contributor

Oh! I didn't know this. Is it documented?

@pksjce

pksjce Dec 8, 2016

Contributor

Oh! I didn't know this. Is it documented?

This comment has been minimized.

@cpojer

cpojer Dec 8, 2016

No it's not. I should post about this.

@cpojer

cpojer Dec 8, 2016

No it's not. I should post about this.

Show outdated Hide outdated 2016/notes-on-testing-with-jest.md
// match snapshot because view changed.
expect(shallowToJson(component)).toMatchSnapshot();
});
});

This comment has been minimized.

@cpojer

cpojer Dec 7, 2016

the indentation in this example seems off. I'd recommend to actually just drop the "describe" block too :D It's just overhead.

@cpojer

cpojer Dec 7, 2016

the indentation in this example seems off. I'd recommend to actually just drop the "describe" block too :D It's just overhead.

});
```
__I can't get PostCSS to work!__

This comment has been minimized.

@cpojer

cpojer Dec 7, 2016

This sucks :( If PostCSS had a sync API, this would just work. The problem here is that Jest transforms files just-in-time when they are required; that's why they need to be synchronous.

One way this could work is to have a "preprocess-step" before you run your tests, like build all the CSS files into "css-out/" and then use moduleNameMapper to map from my.css to css-out/my.css.

@cpojer

cpojer Dec 7, 2016

This sucks :( If PostCSS had a sync API, this would just work. The problem here is that Jest transforms files just-in-time when they are required; that's why they need to be synchronous.

One way this could work is to have a "preprocess-step" before you run your tests, like build all the CSS files into "css-out/" and then use moduleNameMapper to map from my.css to css-out/my.css.

This comment has been minimized.

@pksjce

pksjce Dec 8, 2016

Contributor

Wow. This is a good idea.

@pksjce

pksjce Dec 8, 2016

Contributor

Wow. This is a good idea.

Show outdated Hide outdated 2016/notes-on-testing-with-jest.md
## Conclusion
Its been a couple of weeks since we started using `Jest`. Code Coverage is slowly growing within the team. However we have configured our PR builds to fail if existing tests fail, that include snapshots. In the future, we plan to have coverage thresholds too!

This comment has been minimized.

@cpojer

cpojer Dec 7, 2016

"that includes snapshots"

@cpojer

cpojer Dec 7, 2016

"that includes snapshots"

@cpojer

This comment has been minimized.

Show comment
Hide comment
@cpojer

cpojer Dec 7, 2016

Thank you! I left a few comments but this is looking really great. Thank you so much for doing this.

cpojer commented Dec 7, 2016

Thank you! I left a few comments but this is looking really great. Thank you so much for doing this.

@pksjce

This comment has been minimized.

Show comment
Hide comment
@pksjce

pksjce Dec 8, 2016

Contributor

Thanks for the review! Lots of TIL moments.

Contributor

pksjce commented Dec 8, 2016

Thanks for the review! Lots of TIL moments.

@cpojer

This comment has been minimized.

Show comment
Hide comment
@cpojer

cpojer Dec 8, 2016

Anytime ❤️

cpojer commented Dec 8, 2016

Anytime ❤️

@pksjce

This comment has been minimized.

Show comment
Hide comment
@pksjce

pksjce Dec 9, 2016

Contributor

Closing this.

Contributor

pksjce commented Dec 9, 2016

Closing this.

@pksjce pksjce closed this Dec 9, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment