All of @cpojer comments.

also adds some more tests in the integration tests part
jestjs · May 20, 2016 · 00f5b32 · 00f5b32
1 parent 5e6e211
commit 00f5b32
Show file tree

Hide file tree

Showing 9 changed files with 106 additions and 76 deletions.
diff --git a/integration_tests/__tests__/snapshot-test.js 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/...s/jest-snapshot/src/__tests__/snapshot.js → ...tion_tests/snapshot/__tests__/snapshot.js b/...s/jest-snapshot/src/__tests__/snapshot.js → ...tion_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
@@ -0,0 +1 @@
+{}
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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;
   },
 };