Seamless usage when running within Jest #23

tdd · 2017-07-10T09:47:15Z

Hi all!

Thanks a ton for this module, it's super cool.

However, its existing behavior entirely focuses on people using snapshots outside of Jest, but a significant share of users still use Jest: they just favor Chai's assertion syntax and plugin ecosystem.

I do a lot of JS training, and this is what most of my trainees tell me. That's my situation, too.

This PR makes it so that, when running in Jest, we just delegate to Jest's built-in snapshot capability. No configuration, no setup cost, just registering the Chai plugin, like any other. As seamless as can be.

Tests, examples and docs are updated. I strived to follow the coding style I inferred from the source (even if I'm more of StandardJS fan myself 😉 ).

I hope you like it! Feel free to push back if you have any questions or want tweaks.

This addresses the one reason why I don't use this module in my trainings yet.

FWIW, I'm considering making it even nicer by automatically detecting Enzyme wrappers and then using enzyme-to-json to streamline the testing experience even more. This could be done for all modes, too, and be a nice one-up over Jest's built-in snapshotting. I'd love feedback on the idea.

Best,

tdd · 2017-07-10T09:50:09Z

Hey @suchipi! Would love your feedback on this.

Best,

suchipi · 2017-07-10T15:15:46Z

Hey! This looks interesting. My recommendation up until now has been for people to do something like this, but I can see how this would be useful for easy setup. I'll pull this down and give it a try later this week.

I don't want to automatically add enzyme-to-json because jest snapshots are used for more than just React and I want the scope of this module to be relatively small. However, snapshot serializers are a good way to automatically add functionality like that- a quick google search finds jest-serializer-enzyme. I'd have no problem adding a note to the README with instructions on how to add it, assuming it works- I haven't given it a try yet.

tdd · 2017-07-10T15:23:54Z

Hey there,

Re: Enzyme serialization: indeed, registering a serializer would make the most sense, that's a good point. Will adjust my own practice on this, and abandon the "too-smart built-in" approach 😉

tdd · 2017-07-10T15:25:19Z

Also re: Jest+Chai aliasing Jest's expect: yes, this is what I used to do, but having to switch between expectors and assertion syntaxes (jestExpect(x).toMatchSnapshot() vs. expect(x).to.matchSnapshot()) is just not a great developer experience. I like consistency 😬

suchipi

Thanks for submitting this! There are a few things I'd like to change; most of them are just in regard to the npm scripts.

suchipi · 2017-07-13T18:26:22Z

src/buildMatchSnapshot.js

@@ -33,4 +42,19 @@ const buildMatchSnapshot = (utils, parseArgs) => {
  };
 };

+const safeRequireJestExpect = () => {
+  try {
+    return require("jest-matchers");


Jest is planning to rename jest-matchers to expect. Could we return the global expect here instead?

if (typeof expect !== "undefined") { return expect; } else { return null; }

suchipi · 2017-07-13T18:27:17Z

src/spec-jest/index.spec.js

@@ -0,0 +1,25 @@
+import chai, { expect } from "chai";


This file is redundant, because it does the same thing as examples/4_jestMode/spec.js. Could you remove it?

In fact you can remove the whole spec-jest directory

suchipi · 2017-07-13T18:29:10Z

package.json

@@ -6,12 +6,13 @@
  "repository": "https://github.com/suchipi/chai-jest-snapshot",
  "scripts": {
    "build": "rm -rf dist/* && babel src --out-dir dist",
-    "mocha": "mocha -r 'dist/spec/index' 'dist/**/*.spec.js'",
+    "jest": "jest --roots dist/spec-jest",


We don't need this jest script here because it's handled by the example, so let's remove it.

suchipi · 2017-07-13T18:29:31Z

package.json

    "examples": "cd examples && npm test",
    "example_1:watch": "npm run build && cd examples && npm run example_1:watch",
    "example_2:watch": "npm run build && cd examples && npm run example_2:watch",
    "example_3:watch": "npm run build && cd examples && npm run example_3:watch",
-    "test": "npm run build && npm run mocha && npm run examples"
+    "test": "npm run build && npm run mocha && npm run jest && npm run examples"


We don't need to run jest here at the top level because it gets run by the examples, so let's remove it from the test script.

suchipi · 2017-07-13T18:30:19Z

package.json

@@ -6,12 +6,13 @@
  "repository": "https://github.com/suchipi/chai-jest-snapshot",
  "scripts": {
    "build": "rm -rf dist/* && babel src --out-dir dist",
-    "mocha": "mocha -r 'dist/spec/index' 'dist/**/*.spec.js'",
+    "jest": "jest --roots dist/spec-jest",
+    "mocha": "mocha -r 'dist/spec/index' 'dist/spec/*.spec.js'",
    "examples": "cd examples && npm test",
    "example_1:watch": "npm run build && cd examples && npm run example_1:watch",
    "example_2:watch": "npm run build && cd examples && npm run example_2:watch",
    "example_3:watch": "npm run build && cd examples && npm run example_3:watch",


Could you add a script for example_4:watch here like the other examples have?

"example_4:watch": "npm run build && cd examples && npm run example_4:watch",

suchipi · 2017-07-13T18:31:02Z

package.json

@@ -34,6 +35,7 @@
    "babel-preset-es2015": "6.9.0",
    "babel-preset-react": "6.11.1",
    "chai": "3.5.0",
+    "jest": "20.0.4",


Since jest isn't used by the project's tests, just the examples, could you move this devDependency to examples/package.json?

suchipi · 2017-07-13T18:31:50Z

README.md

+
+chai.use(chaiJestSnapshot);
+```
+
 ### Framework-agnostic Configuration Mode (Recommended for Non-Mocha Users)


Let's update this to say "Recommended for Non-Mocha/Jest Users"

suchipi · 2017-07-13T18:32:05Z

README.md

+
+chai.use(chaiJestSnapshot);
+```
+
 ### Framework-agnostic Configuration Mode (Recommended for Non-Mocha Users)
 If you are not using mocha as your test runner, it is recommended to use chai-jest-snapshot in "framework-agnostic configuration mode".


Let's update this to say "If you are not using mocha or jest as your test runner"

suchipi · 2017-07-13T18:34:20Z

src/buildMatchSnapshot.js

+
+const thisRunsInJest = () => (
+  typeof jest === "object" &&
+  JEST_MARKERS.every((marker) => typeof jest[marker] === "function")


@cpojer is there a more canonical way to detect that we're running in jest? Or is this fine?

(JEST_MARKERS is:

const JEST_MARKERS = ["enableAutomock", "genMockFromModule", "clearAllMocks", "runAllTicks"];

)

tdd · 2017-07-14T06:47:51Z

Hey @suchipi I just pushed a commit addressing all your concerns.

As for canonical Jest detection, I couldn't find anything on Google or Jest's site to begin with, hence this home-baked heuristic. I intentionally focused on methods not likely to be found in other test runners (plus, the object is "jest", so…).

suchipi · 2017-07-14T07:02:50Z

Looks great, thank you!

suchipi · 2017-07-14T07:03:03Z

I will pull this and confirm it works on my machine, then merge and cut a version.

suchipi · 2017-07-14T07:18:42Z

This has been published in v1.3.0. Thank you for the contribution! 🎉

tdd · 2017-07-14T08:25:00Z

'most welcome!

…

On Fri, Jul 14, 2017, at 09:18, Stephen Scott wrote: This has been published in v1.3.0. Thank you for the contribution! 🎉> — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub[1], or mute the thread[2]. >

-- Christophe Porteneuve [email protected] Links: 1. #23 (comment) 2. https://github.com/notifications/unsubscribe-auth/AAAOqLbRTXNFm_z26krPi__z_hDl8iNUks5sNxZSgaJpZM4OSmpE

tdd added 3 commits July 10, 2017 11:37

Streamlined usage when within Jest

2eca93e

Updated README to describe "Jest mode"

b4e47b9

Strip an obsolete fixture from Jest-related spec

276b90d

suchipi requested changes Jul 13, 2017

View reviewed changes

Address PR review

04ea69f

suchipi approved these changes Jul 14, 2017

View reviewed changes

suchipi merged commit cc36edc into suchipi:master Jul 14, 2017

suchipi mentioned this pull request Jul 14, 2017

Jest Example #17

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Seamless usage when running within Jest #23

Seamless usage when running within Jest #23

tdd commented Jul 10, 2017

tdd commented Jul 10, 2017

suchipi commented Jul 10, 2017

tdd commented Jul 10, 2017

tdd commented Jul 10, 2017

suchipi left a comment

suchipi Jul 13, 2017

suchipi Jul 13, 2017

suchipi Jul 13, 2017

suchipi Jul 13, 2017

suchipi Jul 13, 2017

suchipi Jul 13, 2017

suchipi Jul 13, 2017

suchipi Jul 13, 2017

suchipi Jul 13, 2017

suchipi Jul 13, 2017

tdd commented Jul 14, 2017

suchipi commented Jul 14, 2017

suchipi commented Jul 14, 2017

suchipi commented Jul 14, 2017

tdd commented Jul 14, 2017 via email

Seamless usage when running within Jest #23

Seamless usage when running within Jest #23

Conversation

tdd commented Jul 10, 2017

tdd commented Jul 10, 2017

suchipi commented Jul 10, 2017

tdd commented Jul 10, 2017

tdd commented Jul 10, 2017

suchipi 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

tdd commented Jul 14, 2017

suchipi commented Jul 14, 2017

suchipi commented Jul 14, 2017

suchipi commented Jul 14, 2017

tdd commented Jul 14, 2017 via email