perf(diff-snapshot): perform work outside jest process (#95)

__tests__/diff-snapshot.spec.js

@@ -15,19 +15,54 @@
 /* eslint-disable global-require */
 const fs = require('fs');
 const path = require('path');
-const mockSpawn = require('mock-spawn')();
 
 describe('diff-snapshot', () => {
   beforeEach(() => {
     jest.resetModules();
     jest.resetAllMocks();
   });
 
+  describe('runDiffImageToSnapshot', () => {
+    const mockSpawnSync = jest.fn();
+    const fakeRequest = {
+      receivedImageBuffer: Buffer.from('abcdefg'),
+      snapshotIdentifier: 'foo',
+      snapshotsDir: 'bar',
+      updateSnapshot: false,
+      failureThreshold: 0,
+      failureThresholdType: 'pixel',
+    };
+
+    function setupTest(spawnReturn) {
+      mockSpawnSync.mockReturnValue(spawnReturn);
+      jest.mock('child_process', () => ({ spawnSync: mockSpawnSync }));
+      const { runDiffImageToSnapshot } = require('../src/diff-snapshot');
+      return runDiffImageToSnapshot;
+    }
+
+    it('runs external process and returns result', () => {
+      const runDiffImageToSnapshot = setupTest({
+        status: 0, output: [null, null, null, JSON.stringify({ add: true, updated: false })],
+      });
+
+      expect(runDiffImageToSnapshot(fakeRequest)).toEqual({ add: true, updated: false });
+
+      expect(mockSpawnSync).toBeCalled();
+    });
+
+    it('throws when process returns a non-zero status', () => {
+      const runDiffImageToSnapshot = setupTest({ status: 1 });
+      expect(() => runDiffImageToSnapshot(fakeRequest)).toThrow();
+    });
+  });
+
   describe('diffImageToSnapshot', () => {
     const mockSnapshotsDir = path.normalize('/path/to/snapshots');
     const mockSnapshotIdentifier = 'id1';
     const mockImagePath = './__tests__/stubs/TestImage.png';
     const mockImageBuffer = fs.readFileSync(mockImagePath);
+    const mockBigImagePath = './__tests__/stubs/TestImage150x150.png';
+    const mockBigImageBuffer = fs.readFileSync(mockBigImagePath);
     const mockFailImagePath = './__tests__/stubs/TestImageFailure.png';
     const mockFailImageBuffer = fs.readFileSync(mockFailImagePath);
     const mockMkdirSync = jest.fn();
@@ -49,8 +84,6 @@ describe('diff-snapshot', () => {
         readFileSync: jest.fn(),
       });
 
-      mockSpawn.setDefault(mockSpawn.simple(0));
-      jest.mock('child_process', () => ({ spawnSync: mockSpawn }));
       jest.mock('fs', () => mockFs);
       jest.mock('mkdirp', () => ({ sync: mockMkdirpSync }));
       const { diffImageToSnapshot } = require('../src/diff-snapshot');
@@ -131,7 +164,7 @@ describe('diff-snapshot', () => {
       // Check that pixelmatch was called
       expect(mockPixelMatch).toHaveBeenCalledTimes(1);
       // Check that that it did not attempt to write a diff
-      expect(mockSpawn.calls).toEqual([]);
+      expect(mockWriteFileSync.mock.calls).toEqual([]);
     });
 
     it('should write a diff image if the test fails', () => {
@@ -161,9 +194,35 @@ describe('diff-snapshot', () => {
         { threshold: 0.01 }
       );
 
-      expect(mockSpawn.calls[0].args[0]).toBe(path.resolve('./src/write-result-diff-image.js'));
-      expect(mockSpawn.calls[0].command).toBe('node');
-      expect(mockSpawn.calls[0].opts.input).toEqual(expect.any(Buffer));
+      expect(mockWriteFileSync).toHaveBeenCalledTimes(1);
+    });
+
+    it('should fail if image passed is a different size', () => {
+      const diffImageToSnapshot = setupTest({ snapshotExists: true, pixelmatchResult: 5000 });
+      const result = diffImageToSnapshot({
+        receivedImageBuffer: mockBigImageBuffer,
+        snapshotIdentifier: mockSnapshotIdentifier,
+        snapshotsDir: mockSnapshotsDir,
+        updateSnapshot: false,
+        failureThreshold: 0,
+        failureThresholdType: 'pixel',
+      });
+
+      expect(result).toMatchObject({
+        diffOutputPath: path.join(mockSnapshotsDir, '__diff_output__', 'id1-diff.png'),
+        pass: false,
+      });
+      expect(mockPixelMatch).toHaveBeenCalledTimes(1);
+      expect(mockPixelMatch).toHaveBeenCalledWith(
+        expect.any(Buffer),
+        expect.any(Buffer),
+        expect.any(Buffer),
+        150,
+        150,
+        { threshold: 0.01 }
+      );
+
+      expect(mockWriteFileSync).toHaveBeenCalledTimes(1);
     });
 
     it('should pass <= failureThreshold pixel', () => {

__tests__/index.spec.js

@@ -19,7 +19,7 @@ const path = require('path');
 describe('toMatchImageSnapshot', () => {
   function setupMock(diffImageToSnapshotResult) {
     jest.doMock('../src/diff-snapshot', () => ({
-      diffImageToSnapshot: jest.fn(() => diffImageToSnapshotResult),
+      runDiffImageToSnapshot: jest.fn(() => diffImageToSnapshotResult),
     }));
 
     const mockFs = Object.assign({}, fs, {
@@ -135,8 +135,8 @@ describe('toMatchImageSnapshot', () => {
 
     const customDiffConfig = { threshold: 0.3 };
     matcherAtTest('pretendthisisanimagebuffer', { customDiffConfig });
-    const { diffImageToSnapshot } = require('../src/diff-snapshot');
-    expect(diffImageToSnapshot.mock.calls[0][0].customDiffConfig).toEqual(customDiffConfig);
+    const { runDiffImageToSnapshot } = require('../src/diff-snapshot');
+    expect(runDiffImageToSnapshot.mock.calls[0][0].customDiffConfig).toEqual(customDiffConfig);
   });
 
   it('passes diffImageToSnapshot everything it needs to create a snapshot and compare if needed', () => {
@@ -164,9 +164,9 @@ describe('toMatchImageSnapshot', () => {
     const matcherAtTest = toMatchImageSnapshot.bind(mockTestContext);
 
     matcherAtTest('pretendthisisanimagebuffer');
-    const { diffImageToSnapshot } = require('../src/diff-snapshot');
+    const { runDiffImageToSnapshot } = require('../src/diff-snapshot');
 
-    const dataArg = diffImageToSnapshot.mock.calls[0][0];
+    const dataArg = runDiffImageToSnapshot.mock.calls[0][0];
     // This is to make the test work on windows
     dataArg.snapshotsDir = dataArg.snapshotsDir.replace(/\\/g, '/');
 
@@ -198,9 +198,9 @@ describe('toMatchImageSnapshot', () => {
     const matcherAtTest = toMatchImageSnapshot.bind(mockTestContext);
 
     matcherAtTest('pretendthisisanimagebuffer', { customSnapshotIdentifier: 'custom-name' });
-    const { diffImageToSnapshot } = require('../src/diff-snapshot');
+    const { runDiffImageToSnapshot } = require('../src/diff-snapshot');
 
-    expect(diffImageToSnapshot.mock.calls[0][0].snapshotIdentifier).toBe('custom-name');
+    expect(runDiffImageToSnapshot.mock.calls[0][0].snapshotIdentifier).toBe('custom-name');
   });
 
   it('attempts to update snapshots if snapshotState has updateSnapshot flag set', () => {
@@ -222,9 +222,9 @@ describe('toMatchImageSnapshot', () => {
     const matcherAtTest = toMatchImageSnapshot.bind(mockTestContext);
 
     matcherAtTest('pretendthisisanimagebuffer');
-    const { diffImageToSnapshot } = require('../src/diff-snapshot');
+    const { runDiffImageToSnapshot } = require('../src/diff-snapshot');
 
-    expect(diffImageToSnapshot.mock.calls[0][0].updateSnapshot).toBe(true);
+    expect(runDiffImageToSnapshot.mock.calls[0][0].updateSnapshot).toBe(true);
   });
 
   it('should work when a new snapshot is added', () => {
@@ -242,7 +242,7 @@ describe('toMatchImageSnapshot', () => {
     };
     const mockDiff = jest.fn();
     jest.doMock('../src/diff-snapshot', () => ({
-      diffImageToSnapshot: mockDiff,
+      runDiffImageToSnapshot: mockDiff,
     }));
 
     const mockFs = Object.assign({}, fs, {
@@ -329,9 +329,9 @@ describe('toMatchImageSnapshot', () => {
     };
     setupMock({ updated: true });
 
-    const diffImageToSnapshot = jest.fn(() => ({}));
+    const runDiffImageToSnapshot = jest.fn(() => ({}));
     jest.doMock('../src/diff-snapshot', () => ({
-      diffImageToSnapshot,
+      runDiffImageToSnapshot,
     }));
 
     const Chalk = jest.fn();
@@ -342,15 +342,15 @@ describe('toMatchImageSnapshot', () => {
     const customConfig = { perceptual: true };
     const toMatchImageSnapshot = configureToMatchImageSnapshot({
       customDiffConfig: customConfig,
-      customSnapshotsDir: 'path/to/my-custom-snapshots-dir',
+      customSnapshotsDir: path.join('path', 'to', 'my-custom-snapshots-dir'),
       noColors: true,
     });
     expect.extend({ toMatchImageSnapshot });
     const matcherAtTest = toMatchImageSnapshot.bind(mockTestContext);
 
     matcherAtTest();
 
-    expect(diffImageToSnapshot).toHaveBeenCalledWith({
+    expect(runDiffImageToSnapshot).toHaveBeenCalledWith({
       customDiffConfig: {
         perceptual: true,
       },

package.json

@@ -23,7 +23,7 @@
     "preset": "amex-jest-preset",
     "collectCoverageFrom": [
       "src/*.js",
-      "!src/write-result-diff-image.js",
+      "!src/diff-process.js",
       "!**/node_modules/**",
       "!test-results/**"
     ],
@@ -42,7 +42,7 @@
   "author": "Andres Escobar <[email protected]> (https://github.com/anescobar1991)",
   "license": "Apache-2.0",
   "devDependencies": {
-    "amex-jest-preset": "^4.0.2",
+    "amex-jest-preset": "^5.0.0",
     "eslint": "^4.15.0",
     "eslint-config-amex": "^7.0.0",
     "is-png": "^1.1.0",

src/write-result-diff-image.js → src/diff-process.js

@@ -12,19 +12,22 @@
  * the License.
  */
 
-const { PNG } = require('pngjs');
 const fs = require('fs');
+
 const getStdin = require('get-stdin');
 
+const { diffImageToSnapshot } = require('./diff-snapshot');
+
 getStdin.buffer().then((buffer) => {
   try {
-    const input = JSON.parse(buffer);
-    const { imagePath, image } = input;
+    const options = JSON.parse(buffer);
+
+    options.receivedImageBuffer = Buffer.from(options.receivedImageBuffer, 'base64');
+
+    const result = diffImageToSnapshot(options);
 
-    image.data = Buffer.from(image.data);
+    fs.writeSync(3, Buffer.from(JSON.stringify(result)));
 
-    const pngBuffer = PNG.sync.write(image);
-    fs.writeFileSync(imagePath, pngBuffer);
     process.exit(0);
   } catch (error) {
     console.error(error); // eslint-disable-line no-console

src/diff-snapshot.js

@@ -12,13 +12,13 @@
  * the License.
  */
 
+const childProcess = require('child_process');
 const fs = require('fs');
 const path = require('path');
-const childProcess = require('child_process');
-const rimraf = require('rimraf');
-const pixelmatch = require('pixelmatch');
 const mkdirp = require('mkdirp');
+const pixelmatch = require('pixelmatch');
 const { PNG } = require('pngjs');
+const rimraf = require('rimraf');
 
 /**
  * Helper function to create reusable image resizer
@@ -164,16 +164,10 @@ function diffImageToSnapshot(options) {
       PNG.bitblt(
         receivedImage, compositeResultImage, 0, 0, imageWidth, imageHeight, imageWidth * 2, 0
       );
-
-      const input = { imagePath: diffOutputPath, image: compositeResultImage };
-      // image._packer property contains a circular reference since node9, causing JSON.stringify to
-      // fail. Might as well discard all the hidden properties.
-      const serializedInput = JSON.stringify(input, (name, val) => (name[0] === '_' ? undefined : val));
-
-      // writing diff in separate process to avoid perf issues associated with Math in Jest VM (https://github.com/facebook/jest/issues/5163)
-      const writeDiffProcess = childProcess.spawnSync('node', [`${__dirname}/write-result-diff-image.js`], { input: Buffer.from(serializedInput) });
-      // in case of error print to console
-      if (writeDiffProcess.stderr.toString()) { console.log(writeDiffProcess.stderr.toString()); } // eslint-disable-line no-console, max-len
+      // Set filter type to Paeth to avoid expensive auto scanline filter detection
+      // For more information see https://www.w3.org/TR/PNG-Filters.html
+      const pngBuffer = PNG.sync.write(compositeResultImage, { filterType: 4 });
+      fs.writeFileSync(diffOutputPath, pngBuffer);
 
       result = {
         pass: false,
@@ -197,6 +191,30 @@ function diffImageToSnapshot(options) {
   return result;
 }
 
+
+function runDiffImageToSnapshot(options) {
+  options.receivedImageBuffer = options.receivedImageBuffer.toString('base64');
+
+  const serializedInput = JSON.stringify(options);
+
+  let result = {};
+
+  const writeDiffProcess = childProcess.spawnSync(
+    'node', [`${__dirname}/diff-process.js`],
+    { input: Buffer.from(serializedInput), stdio: ['pipe', 'inherit', 'inherit', 'pipe'] }
+  );
+
+  if (writeDiffProcess.status === 0) {
+    const output = writeDiffProcess.output[3].toString();
+    result = JSON.parse(output);
+  } else {
+    throw new Error('Error running image diff.');
+  }
+
+  return result;
+}
+
 module.exports = {
   diffImageToSnapshot,
+  runDiffImageToSnapshot,
 };

src/index.js

@@ -16,7 +16,7 @@ const kebabCase = require('lodash/kebabCase');
 const merge = require('lodash/merge');
 const path = require('path');
 const Chalk = require('chalk').constructor;
-const { diffImageToSnapshot } = require('./diff-snapshot');
+const { runDiffImageToSnapshot } = require('./diff-snapshot');
 const fs = require('fs');
 
 const SNAPSHOTS_DIR = '__image_snapshots__';
@@ -68,7 +68,7 @@ function configureToMatchImageSnapshot({
     }
 
     const result =
-      diffImageToSnapshot({
+      runDiffImageToSnapshot({
         receivedImageBuffer: received,
         snapshotsDir,
         snapshotIdentifier,