From 00f5b320b67566c1e4cd257a7df29e8dba7758ac Mon Sep 17 00:00:00 2001 From: kentaromiura Date: Fri, 20 May 2016 20:15:52 +0100 Subject: [PATCH] All of @cpojer comments. also adds some more tests in the integration tests part --- integration_tests/__tests__/snapshot-test.js | 39 ++++++++++++ .../snapshot}/__tests__/snapshot.js | 9 +++ integration_tests/snapshot/package.json | 1 + packages/jest-jasmine2/package.json | 4 +- packages/jest-snapshot/src/SnapshotFile.js | 22 ++----- .../src/__tests__/SnapShotFile.js | 24 ++++---- .../src/__tests__/getMatchers.js | 8 ++- packages/jest-snapshot/src/getMatchers.js | 59 ++++++++----------- packages/jest-snapshot/src/index.js | 16 ++--- 9 files changed, 106 insertions(+), 76 deletions(-) create mode 100644 integration_tests/__tests__/snapshot-test.js rename {packages/jest-snapshot/src => integration_tests/snapshot}/__tests__/snapshot.js (76%) create mode 100644 integration_tests/snapshot/package.json diff --git a/integration_tests/__tests__/snapshot-test.js b/integration_tests/__tests__/snapshot-test.js new file mode 100644 index 000000000000..f3bd9d48e052 --- /dev/null +++ b/integration_tests/__tests__/snapshot-test.js @@ -0,0 +1,39 @@ +/** + * Copyright (c) 2014-present, Facebook, Inc. All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + * @emails oncall+jsinfra + */ +'use strict'; +const fs = require('fs'); +const path = require('path'); +const runJest = require('../runJest'); + +describe('Snapshot', () => { + it('works as expected', () => { + const result = runJest.json('snapshot', []); + const json = result.json; + + expect(json.numTotalTests).toBe(2); + expect(json.numPassedTests).toBe(2); + expect(json.numFailedTests).toBe(0); + expect(json.numPendingTests).toBe(0); + expect(result.status).toBe(0); + + const content = fs.readFileSync( + path.resolve( + __dirname, + '../snapshot/__tests__/__snapshots__/snapshot.js.snap' + ) + ); + + const output = JSON.parse(content); + expect( + output['snapshot is not influenced by previous counter 0'] + ).not.toBe(undefined); + + }); +}); diff --git a/packages/jest-snapshot/src/__tests__/snapshot.js b/integration_tests/snapshot/__tests__/snapshot.js similarity index 76% rename from packages/jest-snapshot/src/__tests__/snapshot.js rename to integration_tests/snapshot/__tests__/snapshot.js index 6ff63fea173d..20a7a5a25571 100644 --- a/packages/jest-snapshot/src/__tests__/snapshot.js +++ b/integration_tests/snapshot/__tests__/snapshot.js @@ -22,4 +22,13 @@ describe('snapshot', () => { expect(JSON.stringify(test)).toMatchSnapshot(); }); + it('is not influenced by previous counter', () => { + const test = { + a:43, + b:'43', + c:'fourtythree', + }; + expect(JSON.stringify(test)).toMatchSnapshot(); + }); + }); diff --git a/integration_tests/snapshot/package.json b/integration_tests/snapshot/package.json new file mode 100644 index 000000000000..0967ef424bce --- /dev/null +++ b/integration_tests/snapshot/package.json @@ -0,0 +1 @@ +{} diff --git a/packages/jest-jasmine2/package.json b/packages/jest-jasmine2/package.json index 4dfc21425634..6379223dfdf4 100644 --- a/packages/jest-jasmine2/package.json +++ b/packages/jest-jasmine2/package.json @@ -9,8 +9,8 @@ "main": "src/index.js", "dependencies": { "graceful-fs": "^4.1.3", - "jest-util": "^12.0.2", - "jest-snapshot": "^12.0.2" + "jest-snapshot": "^12.0.2", + "jest-util": "^12.0.2" }, "jest": { "testEnvironment": "node" diff --git a/packages/jest-snapshot/src/SnapshotFile.js b/packages/jest-snapshot/src/SnapshotFile.js index 89bf2bff486e..9aa7ae1a24ce 100644 --- a/packages/jest-snapshot/src/SnapshotFile.js +++ b/packages/jest-snapshot/src/SnapshotFile.js @@ -17,14 +17,12 @@ const ensureDirectoryExists = filePath => { createDirectory(path.join(path.dirname(filePath))); } catch (e) {} }; -const fileExists = path => { - let exists = true; +const fileExists = filePath => { try { - fs.accessSync(path, fs.F_OK); - } catch (e) { - exists = false; - } - return exists; + fs.accessSync(filePath, fs.R_OK); + return true; + } catch (e) {} + return false; }; class SnapshotFile { @@ -64,16 +62,8 @@ class SnapshotFile { return this.get(key) === value; } - replace(key, value) { - this._content[key] = value; - } - add(key, value) { - if (!this.has(key)) { - this.replace(key, value); - } else { - throw new Error('Trying to add a snapshot that already exists'); - } + this._content[key] = value; } } diff --git a/packages/jest-snapshot/src/__tests__/SnapShotFile.js b/packages/jest-snapshot/src/__tests__/SnapShotFile.js index b1961eaa1d42..ba33e8f0f531 100644 --- a/packages/jest-snapshot/src/__tests__/SnapShotFile.js +++ b/packages/jest-snapshot/src/__tests__/SnapShotFile.js @@ -12,25 +12,21 @@ let accessShouldThrow = false; jest - .autoMockOff() - .mock('mkdirp', () => { - return { - sync: jest.fn(), - }; - }) + .disableAutomock() + .mock('mkdirp', () => ({sync: jest.fn()})) .mock('fs', () => ({ - accessSync : jest.fn(() => { + accessSync: jest.fn(() => { if (accessShouldThrow) { throw new Error(); } return true; }), - readFileSync : jest.fn(fileName => { + readFileSync: jest.fn(fileName => { const EXPECTED_FILE_NAME = '/foo/__tests__/__snapshots__/baz.js.snap'; expect(fileName).toBe(EXPECTED_FILE_NAME); return '{}'; }), - writeFileSync : jest.fn((path, content) => { + writeFileSync: jest.fn((path, content) => { expect(content).toBe('{"foo":"bar"}'); }), })); @@ -82,16 +78,16 @@ describe('SnapshotFile', () => { const snapshotFile = SnapshotFile.forFile(TEST_FILE); snapshotFile.add(SNAPSHOT, SNAPSHOT_VALUE); expect(snapshotFile.matches(SNAPSHOT, SNAPSHOT_VALUE)).toBe(true); - snapshotFile.replace(SNAPSHOT, NEW_VALUE); + snapshotFile.add(SNAPSHOT, NEW_VALUE); expect(snapshotFile.matches(SNAPSHOT, NEW_VALUE)).toBe(true); }); - it('cannot add the same key twice', () => { + it('can add the same key twice', () => { const snapshotFile = SnapshotFile.forFile(TEST_FILE); snapshotFile.add(SNAPSHOT, SNAPSHOT_VALUE); - expect(() => { - snapshotFile.add(SNAPSHOT, SNAPSHOT_VALUE); - }).toThrow(); + expect( + () => snapshotFile.add(SNAPSHOT, SNAPSHOT_VALUE) + ).not.toThrow(); }); it('loads and saves file correctly', () => { diff --git a/packages/jest-snapshot/src/__tests__/getMatchers.js b/packages/jest-snapshot/src/__tests__/getMatchers.js index 839a93eb4133..496f19a79705 100644 --- a/packages/jest-snapshot/src/__tests__/getMatchers.js +++ b/packages/jest-snapshot/src/__tests__/getMatchers.js @@ -22,14 +22,18 @@ describe('getMatchers', () => { const toMatchSnapshot = getMatchers().toMatchSnapshot(); const rendered = null; const expected = {}; - expect(() => {toMatchSnapshot.compare(rendered, expected);}).toThrow(); + expect( + () => toMatchSnapshot.compare(rendered, expected) + ).toThrow(); }); it('only accepts strings', () => { const toMatchSnapshot = getMatchers().toMatchSnapshot(); const rendered = {}; const expected = undefined; - expect(() => {toMatchSnapshot.compare(rendered, expected);}).toThrow(); + expect( + () => toMatchSnapshot.compare(rendered, expected) + ).toThrow(); }); }); }); diff --git a/packages/jest-snapshot/src/getMatchers.js b/packages/jest-snapshot/src/getMatchers.js index f51ccee226d8..80e91ac21c7b 100644 --- a/packages/jest-snapshot/src/getMatchers.js +++ b/packages/jest-snapshot/src/getMatchers.js @@ -12,7 +12,7 @@ module.exports = (filePath, options, jasmine, snapshotState) => ({ return { compare(rendered, expected) { - const specRunningFullName = snapshotState.specRunningFullName; + const currentSpecName = snapshotState.currentSpecName; if (expected !== undefined) { throw new Error('toMatchSnapshot() does not accepts parameters.'); @@ -23,46 +23,37 @@ module.exports = (filePath, options, jasmine, snapshotState) => ({ const snapshot = snapshotState.snapshot; - const specRunningCallCounter = ( - snapshotState.specsNextCallCounter[specRunningFullName] - ); + const callCount = snapshotState.getCounter(); + snapshotState.incrementCounter(); let pass = false; let message; - let res = {}; - const key = specRunningFullName + ' ' + specRunningCallCounter; - - if (!snapshot.fileExists()) { - snapshot.replace(key, rendered); + const key = currentSpecName + ' ' + callCount; + if ( + !snapshot.fileExists() || + (snapshot.has(key) && options.updateSnapshot) || + !snapshot.has(key) + ) { + snapshot.add(key, rendered); pass = true; } else { - if (!snapshot.has(key)) { - snapshot.add(key, rendered); - pass = true; - } else { - if (options.updateSnapshot) { - snapshot.replace(key, rendered); - pass = true; - } else { - pass = snapshot.matches(key, rendered); - if (!pass) { - const matcherName = 'toMatchSnapshot'; - res = { - matcherName, - expected: snapshot.get(key), - actual: rendered, - }; + pass = snapshot.matches(key, rendered); + if (!pass) { + const matcherName = 'toMatchSnapshot'; + const res = { + matcherName, + expected: snapshot.get(key), + actual: rendered, + }; - const JasmineFormatter = require('jest-util').JasmineFormatter; + const JasmineFormatter = require('jest-util').JasmineFormatter; - const formatter = new JasmineFormatter(jasmine, {global: {}}, {}); - formatter.addDiffableMatcher('toMatchSnapshot'); - message = formatter.formatMatchFailure(res).replace( - 'toMatchSnapshot:', - 'toMatchSnapshot #' + (specRunningCallCounter + 1) + ':' - ); - } - } + const formatter = new JasmineFormatter(jasmine, {global: {}}, {}); + formatter.addDiffableMatcher('toMatchSnapshot'); + message = formatter.formatMatchFailure(res).replace( + 'toMatchSnapshot:', + 'toMatchSnapshot #' + (callCount + 1) + ':' + ); } } diff --git a/packages/jest-snapshot/src/index.js b/packages/jest-snapshot/src/index.js index c5e09e192f87..f38cb7faa5d8 100644 --- a/packages/jest-snapshot/src/index.js +++ b/packages/jest-snapshot/src/index.js @@ -14,11 +14,9 @@ const patchAttr = (attr, state) => { return function(context) { const specRunning = context.getFullName(); let index = 0; - Object.defineProperty(state.specsNextCallCounter, specRunning, { - get() {return index++;}, - enumerable: true, - }); - state.specRunningFullName = specRunning; + state.getCounter = () => index; + state.incrementCounter = () => index++; + state.currentSpecName = specRunning; if (onStart) { onStart(context); } @@ -34,7 +32,7 @@ const patchJasmine = (jasmine, state) => { }; Spec.prototype = realSpec.prototype; for (const statics in realSpec) { - if (realSpec.hasOwnProperty(statics)) { + if (Object.prototype.hasOwnProperty.call(realSpec, statics)) { Spec[statics] = realSpec[statics]; } } @@ -46,9 +44,11 @@ module.exports = { getMatchers: require('./getMatchers'), getSnapshotState: (jasmine, filePath) => { const state = Object.create(null); - state.specsNextCallCounter = Object.create(null); - patchJasmine(jasmine, state); + state.currentSpecName = null; + state.getCounter = null; + state.incrementCounter = null; state.snapshot = SnapshotFile.forFile(filePath); + patchJasmine(jasmine, state); return state; }, };