Do not write snapshots by default on CI systems.

jestjs · May 3, 2017 · 1fd4838 · 1fd4838
1 parent e795008
commit 1fd4838
Show file tree

Hide file tree

Showing 11 changed files with 84 additions and 72 deletions.
diff --git a/packages/jest-cli/src/TestRunner.js b/packages/jest-cli/src/TestRunner.js
@@ -140,7 +140,8 @@ class TestRunner {
         );
         aggregatedResults.snapshot.filesRemoved += status.filesRemoved;
       });
-      aggregatedResults.snapshot.didUpdate = this._globalConfig.updateSnapshot;
+      aggregatedResults.snapshot.didUpdate =
+        this._globalConfig.updateSnapshot === 'all';
       aggregatedResults.snapshot.failure = !!(!this._globalConfig
         .updateSnapshot &&
         (aggregatedResults.snapshot.unchecked ||

diff --git a/packages/jest-cli/src/cli/args.js b/packages/jest-cli/src/cli/args.js
@@ -12,6 +12,8 @@
 
 import type {Argv} from 'types/Argv';
 
+const isCI = require('is-ci');
+
 const check = (argv: Argv) => {
   if (argv.runInBand && argv.hasOwnProperty('maxWorkers')) {
     throw new Error(
@@ -80,6 +82,13 @@ const options = {
       ' dependency information.',
     type: 'string',
   },
+  ci: {
+    default: isCI,
+    description: 'Whether to run Jest in continuous integration (CI) mode. ' +
+      'This option is on by default in most popular CI environments. It will ' +
+      ' prevent snapshots from being written unless explicitly requested.',
+    type: 'boolean',
+  },
   clearMocks: {
     default: undefined,
     description: 'Automatically clear mock calls and instances between every ' +

diff --git a/packages/jest-config/src/normalize.js b/packages/jest-config/src/normalize.js
@@ -420,7 +420,6 @@ function normalize(options: InitialOptions, argv: Argv) {
       case 'testRegex':
       case 'testURL':
       case 'timers':
-      case 'updateSnapshot':
       case 'useStderr':
       case 'verbose':
       case 'watch':
@@ -432,6 +431,10 @@ function normalize(options: InitialOptions, argv: Argv) {
     return newOptions;
   }, newOptions);
 
+  newOptions.updateSnapshot = argv.ci && !argv.updateSnapshot
+    ? 'none'
+    : argv.updateSnapshot ? 'all' : 'new';
+
   if (babelJest) {
     const regeneratorRuntimePath = Resolver.findNodeModule(
       'regenerator-runtime/runtime',

diff --git a/packages/jest-jasmine2/src/setup-jest-globals.js b/packages/jest-jasmine2/src/setup-jest-globals.js
@@ -142,13 +142,10 @@ module.exports = ({
   config.snapshotSerializers.concat().reverse().forEach(path => {
     addSerializer(localRequire(path));
   });
-  setState({testPath});
   patchJasmine();
-  const snapshotState = new SnapshotState(testPath, {
-    expand: globalConfig.expand,
-    shouldUpdate: globalConfig.updateSnapshot,
-  });
-  setState({snapshotState});
+  const {expand, updateSnapshot} = globalConfig;
+  const snapshotState = new SnapshotState(testPath, {expand, updateSnapshot});
+  setState({snapshotState, testPath});
   // Return it back to the outer scope (test runner outside the VM).
   return snapshotState;
 };
diff --git a/packages/jest-snapshot/src/State.js b/packages/jest-snapshot/src/State.js
@@ -10,7 +10,7 @@
 
 'use strict';
 
-import type {Path} from 'types/Config';
+import type {Path, SnapshotUpdateState} from 'types/Config';
 
 const {
   saveSnapshotFile,
@@ -24,7 +24,7 @@ const {
 const fs = require('fs');
 
 export type SnapshotStateOptions = {|
-  shouldUpdate: boolean,
+  updateSnapshot: SnapshotUpdateState,
   snapshotPath?: string,
   expand?: boolean,
 |};
@@ -33,7 +33,7 @@ class SnapshotState {
   _counters: Map<string, number>;
   _dirty: boolean;
   _index: number;
-  _shouldUpdate: boolean;
+  _updateSnapshot: SnapshotUpdateState;
   _snapshotData: {[key: string]: string};
   _snapshotPath: Path;
   _uncheckedKeys: Set<string>;
@@ -43,14 +43,11 @@ class SnapshotState {
   unmatched: number;
   updated: number;
 
-  constructor(
-    testPath: Path,
-    options: SnapshotStateOptions,
-  ) {
+  constructor(testPath: Path, options: SnapshotStateOptions) {
     this._snapshotPath = options.snapshotPath || getSnapshotPath(testPath);
     const {data, dirty} = getSnapshotData(
       this._snapshotPath,
-      options.shouldUpdate,
+      options.updateSnapshot,
     );
     this._snapshotData = data;
     this._dirty = dirty;
@@ -61,7 +58,7 @@ class SnapshotState {
     this.added = 0;
     this.matched = 0;
     this.unmatched = 0;
-    this._shouldUpdate = options.shouldUpdate;
+    this._updateSnapshot = options.updateSnapshot;
     this.updated = 0;
   }
 
@@ -78,19 +75,18 @@ class SnapshotState {
     this._snapshotData[key] = receivedSerialized;
   }
 
-  save(shouldUpdate: boolean) {
+  save(shouldUpdate: SnapshotUpdateState) {
     const status = {
       deleted: false,
       saved: false,
     };
-
     const isEmpty = Object.keys(this._snapshotData).length === 0;
 
     if ((this._dirty || this._uncheckedKeys.size) && !isEmpty) {
       saveSnapshotFile(this._snapshotData, this._snapshotPath);
       status.saved = true;
     } else if (isEmpty && fs.existsSync(this._snapshotPath)) {
-      if (shouldUpdate) {
+      if (shouldUpdate === 'all') {
         fs.unlinkSync(this._snapshotPath);
       }
       status.deleted = true;
@@ -138,10 +134,11 @@ class SnapshotState {
 
     if (
       !fs.existsSync(this._snapshotPath) || // there's no snapshot file
-      (hasSnapshot && this._shouldUpdate) || // there is a file, but we're updating
-      !hasSnapshot // there is a file, but it doesn't have this snaphsot
+      (hasSnapshot && this._updateSnapshot === 'all') || // there is a file, but we're updating
+      (!hasSnapshot &&
+        (this._updateSnapshot === 'new' || this._updateSnapshot === 'all')) // there is a file, but it doesn't have this snaphsot
     ) {
-      if (this._shouldUpdate) {
+      if (this._updateSnapshot === 'all') {
         if (!pass) {
           if (hasSnapshot) {
             this.updated++;
@@ -169,7 +166,7 @@ class SnapshotState {
         return {
           actual: unescape(receivedSerialized),
           count,
-          expected: unescape(expected),
+          expected: expected ? unescape(expected) : null,
           pass: false,
         };
       } else {

diff --git a/packages/jest-snapshot/src/__tests__/utils-test.js b/packages/jest-snapshot/src/__tests__/utils-test.js
@@ -87,7 +87,7 @@ test('saveSnapshotFile() works with \r', () => {
 test('getSnapshotData() throws when no snapshot version', () => {
   const filename = path.join(__dirname, 'old-snapshot.snap');
   fs.readFileSync = jest.fn(() => 'exports[`myKey`] = `<div>\n</div>`;\n');
-  const update = false;
+  const update = 'none';
 
   expect(() => getSnapshotData(filename, update)).toThrowError(
     chalk.red(
@@ -106,7 +106,7 @@ test('getSnapshotData() throws for older snapshot version', () => {
       `// Jest Snapshot v0.99, ${SNAPSHOT_GUIDE_LINK}\n\n` +
       'exports[`myKey`] = `<div>\n</div>`;\n',
   );
-  const update = false;
+  const update = 'none';
 
   expect(() => getSnapshotData(filename, update)).toThrowError(
     chalk.red(
@@ -129,7 +129,7 @@ test('getSnapshotData() throws for newer snapshot version', () => {
       `// Jest Snapshot v2, ${SNAPSHOT_GUIDE_LINK}\n\n` +
       'exports[`myKey`] = `<div>\n</div>`;\n',
   );
-  const update = false;
+  const update = 'none';
 
   expect(() => getSnapshotData(filename, update)).toThrowError(
     chalk.red(
@@ -148,15 +148,15 @@ test('getSnapshotData() throws for newer snapshot version', () => {
 test('getSnapshotData() does not throw for when updating', () => {
   const filename = path.join(__dirname, 'old-snapshot.snap');
   fs.readFileSync = jest.fn(() => 'exports[`myKey`] = `<div>\n</div>`;\n');
-  const update = true;
+  const update = 'all';
 
   expect(() => getSnapshotData(filename, update)).not.toThrow();
 });
 
 test('getSnapshotData() marks invalid snapshot dirty when updating', () => {
   const filename = path.join(__dirname, 'old-snapshot.snap');
   fs.readFileSync = jest.fn(() => 'exports[`myKey`] = `<div>\n</div>`;\n');
-  const update = true;
+  const update = 'all';
 
   expect(getSnapshotData(filename, update)).toMatchObject({dirty: true});
 });
@@ -168,7 +168,7 @@ test('getSnapshotData() marks valid snapshot not dirty when updating', () => {
       `// Jest Snapshot v${SNAPSHOT_VERSION}, ${SNAPSHOT_GUIDE_LINK}\n\n` +
       'exports[`myKey`] = `<div>\n</div>`;\n',
   );
-  const update = true;
+  const update = 'all';
 
   expect(getSnapshotData(filename, update)).toMatchObject({dirty: false});
 });

diff --git a/packages/jest-snapshot/src/index.js b/packages/jest-snapshot/src/index.js
@@ -10,7 +10,8 @@
 'use strict';
 
 import type {HasteFS} from 'types/HasteMap';
-import type {Path} from 'types/Config';
+import type {MatcherState} from 'types/Matchers';
+import type {Path, SnapshotUpdateState} from 'types/Config';
 
 const diff = require('jest-diff');
 const fs = require('fs');
@@ -29,7 +30,7 @@ const {SNAPSHOT_EXTENSION} = require('./utils');
 const fileExists = (filePath: Path, hasteFS: HasteFS): boolean =>
   hasteFS.exists(filePath) || fs.existsSync(filePath);
 
-const cleanup = (hasteFS: HasteFS, update: boolean) => {
+const cleanup = (hasteFS: HasteFS, update: SnapshotUpdateState) => {
   const pattern = '\\.' + SNAPSHOT_EXTENSION + '$';
   const files = hasteFS.matchFiles(pattern);
   const filesRemoved = files
@@ -45,7 +46,7 @@ const cleanup = (hasteFS: HasteFS, update: boolean) => {
         ),
     )
     .map(snapshotFile => {
-      if (update) {
+      if (update === 'all') {
         fs.unlinkSync(snapshotFile);
       }
     }).length;
@@ -58,15 +59,7 @@ const cleanup = (hasteFS: HasteFS, update: boolean) => {
 const toMatchSnapshot = function(received: any, testName?: string) {
   this.dontThrow && this.dontThrow();
 
-  const {
-    currentTestName,
-    isNot,
-    snapshotState,
-  }: {
-    currentTestName: string,
-    isNot: boolean,
-    snapshotState: SnapshotState,
-  } = this;
+  const {currentTestName, isNot, snapshotState}: MatcherState = this;
 
   if (isNot) {
     throw new Error('Jest: `.not` cannot be used with `.toMatchSnapshot()`.');
@@ -76,45 +69,51 @@ const toMatchSnapshot = function(received: any, testName?: string) {
     throw new Error('Jest: snapshot state must be initialized.');
   }
 
-  const {actual, expected, count, pass} = snapshotState.match(
-    testName || currentTestName,
+  const result = snapshotState.match(
+    testName || currentTestName || '',
     received,
   );
+  const {count, pass} = result;
+  let {actual, expected} = result;
 
+  let report;
   if (pass) {
     return {message: '', pass: true};
+  } else if (!expected) {
+    report = () =>
+      `New snapshot was ${RECEIVED_COLOR('not written')}. The update flag ` +
+      `must be explicitly passed to write a new snapshot.\n\n` +
+      `This is likely because this test is run in a continuous integration ` +
+      `(CI) environment in which snapshots are not written by default.`;
   } else {
-    const expectedString = expected.trim();
-    const actualString = actual.trim();
-    const diffMessage = diff(expectedString, actualString, {
+    expected = (expected || '').trim();
+    actual = (actual || '').trim();
+    const diffMessage = diff(expected, actual, {
       aAnnotation: 'Snapshot',
       bAnnotation: 'Received',
       expand: snapshotState.expand,
     });
 
-    const report = () =>
+    report = () =>
       `${RECEIVED_COLOR('Received value')} does not match ` +
       `${EXPECTED_COLOR('stored snapshot ' + count)}.\n\n` +
       (diffMessage ||
-        RECEIVED_COLOR('- ' + expectedString) +
+        RECEIVED_COLOR('- ' + (expected || '')) +
           '\n' +
-          EXPECTED_COLOR('+ ' + actualString));
-
-    const message = () =>
-      matcherHint('.toMatchSnapshot', 'value', '') + '\n\n' + report();
-
-    // Passing the the actual and expected objects so that a custom reporter
-    // could access them, for example in order to display a custom visual diff,
-    // or create a different error message
-    return {
-      actual: actualString,
-      expected: expectedString,
-      message,
-      name: 'toMatchSnapshot',
-      pass: false,
-      report,
-    };
+          EXPECTED_COLOR('+ ' + actual));
   }
+  // Passing the the actual and expected objects so that a custom reporter
+  // could access them, for example in order to display a custom visual diff,
+  // or create a different error message
+  return {
+    actual,
+    expected,
+    message: () =>
+      matcherHint('.toMatchSnapshot', 'value', '') + '\n\n' + report(),
+    name: 'toMatchSnapshot',
+    pass: false,
+    report,
+  };
 };
 
 const toThrowErrorMatchingSnapshot = function(received: any, expected: void) {

diff --git a/packages/jest-snapshot/src/utils.js b/packages/jest-snapshot/src/utils.js
@@ -10,7 +10,7 @@
 
 'use strict';
 
-import type {Path} from 'types/Config';
+import type {Path, SnapshotUpdateState} from 'types/Config';
 
 const chalk = require('chalk');
 const createDirectory = require('jest-util').createDirectory;
@@ -96,7 +96,7 @@ const getSnapshotPath = (testPath: Path) =>
     path.basename(testPath) + '.' + SNAPSHOT_EXTENSION,
   );
 
-const getSnapshotData = (snapshotPath: Path, update: boolean) => {
+const getSnapshotData = (snapshotPath: Path, update: SnapshotUpdateState) => {
   const data = Object.create(null);
   let snapshotContents = '';
   let dirty = false;
@@ -113,11 +113,11 @@ const getSnapshotData = (snapshotPath: Path, update: boolean) => {
   const validationResult = validateSnapshotVersion(snapshotContents);
   const isInvalid = snapshotContents && validationResult;
 
-  if (!update && isInvalid) {
+  if (update === 'none' && isInvalid) {
     throw validationResult;
   }
 
-  if (update && isInvalid) {
+  if ((update === 'all' || update === 'new') && isInvalid) {
     dirty = true;
   }
 

diff --git a/types/Argv.js b/types/Argv.js
@@ -18,6 +18,7 @@ export type Argv = {|
   cache: boolean,
   cacheDirectory: string,
   clearMocks: boolean,
+  ci: boolean,
   collectCoverage: boolean,
   collectCoverageFrom: Array<string>,
   collectCoverageOnlyFrom: Array<string>,

diff --git a/types/Config.js b/types/Config.js
@@ -123,6 +123,8 @@ export type InitialOptions = {|
   watchman?: boolean,
 |};
 
+export type SnapshotUpdateState = 'all' | 'new' | 'none';
+
 export type GlobalConfig = {|
   bail: boolean,
   collectCoverage: boolean,
@@ -145,7 +147,7 @@ export type GlobalConfig = {|
   testNamePattern: string,
   testPathPattern: string,
   testResultsProcessor: ?string,
-  updateSnapshot: boolean,
+  updateSnapshot: SnapshotUpdateState,
   useStderr: boolean,
   verbose: ?boolean,
   watch: boolean,