jest-snapshot: Distinguish empty string from external snapshot not written (#8880)

* jest-snapshot: Distinguish empty string from external snapshot not written

* Update CHANGELOG.md

* Replace null with undefined

* Add e2e test toMatchSnapshotWithStringSerializer

Author: pedrottimark

CHANGELOG.md

@@ -18,6 +18,7 @@
 - `[jest-leak-detector]` [**BREAKING**] Use `weak-napi` instead of `weak` package ([#8686](https://github.com/facebook/jest/pull/8686))
 - `[jest-mock]` Fix for mockReturnValue overriding mockImplementationOnce ([#8398](https://github.com/facebook/jest/pull/8398))
 - `[jest-snapshot]` Remove only the added newlines in multiline snapshots ([#8859](https://github.com/facebook/jest/pull/8859))
+- `[jest-snapshot]` Distinguish empty string from external snapshot not written ([#8880](https://github.com/facebook/jest/pull/8880))
 
 ### Chore & Maintenance
 

e2e/__tests__/toMatchSnapshotWithStringSerializer.test.ts

@@ -0,0 +1,55 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+import * as path from 'path';
+import {cleanup, makeTemplate, writeFiles} from '../Utils';
+import runJest from '../runJest';
+
+const DIR = path.resolve(
+  __dirname,
+  '../to-match-snapshot-with-string-serializer',
+);
+const TESTS_DIR = path.resolve(DIR, '__tests__');
+
+beforeEach(() => cleanup(TESTS_DIR));
+afterAll(() => cleanup(TESTS_DIR));
+
+test('empty external', () => {
+  // Make sure empty string as expected value of external snapshot
+  // is not confused with new snapshot not written because of --ci option.
+  const filename = 'empty-external.test.js';
+  const template = makeTemplate(
+    `test('string serializer', () => { expect($1).toMatchSnapshot(); })`,
+  );
+
+  {
+    writeFiles(TESTS_DIR, {
+      [filename]: template(['""']), // empty string
+    });
+    const {stderr, status} = runJest(DIR, ['-w=1', '--ci=false', filename]);
+    expect(stderr).toMatch('1 snapshot written from 1 test suite.');
+    expect(status).toBe(0);
+  }
+
+  {
+    const {stderr, status} = runJest(DIR, ['-w=1', '--ci=false', filename]);
+    expect(stderr).toMatch('Snapshots:   1 passed, 1 total');
+    expect(stderr).not.toMatch('1 snapshot written from 1 test suite.');
+    expect(status).toBe(0);
+  }
+
+  {
+    writeFiles(TESTS_DIR, {
+      [filename]: template(['"non-empty"']),
+    });
+    const {stderr, status} = runJest(DIR, ['-w=1', '--ci=false', filename]);
+    expect(stderr).toMatch('Snapshots:   1 failed, 1 total');
+    expect(stderr).not.toMatch('not written'); // not confused with --ci option
+    expect(stderr).toMatch(/- Snapshot|Snapshot:/); // ordinary report
+    expect(status).toBe(1);
+  }
+});

e2e/to-match-snapshot-with-string-serializer/package.json

@@ -0,0 +1,8 @@
+{
+  "jest": {
+    "testEnvironment": "node",
+    "snapshotSerializers": [
+      "./serializers/string"
+    ]
+  }
+}

e2e/to-match-snapshot-with-string-serializer/serializers/string.js

@@ -0,0 +1,14 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+'use strict';
+
+// Serialize string (especially empty) without enclosing punctuation.
+module.exports = {
+  print: val => val,
+  test: val => typeof val === 'string',
+};

packages/jest-snapshot/src/State.ts

@@ -35,6 +35,14 @@ export type SnapshotMatchOptions = {
   error?: Error;
 };
 
+type SnapshotReturnOptions = {
+  actual: string;
+  count: number;
+  expected?: string;
+  key: string;
+  pass: boolean;
+};
+
 export default class SnapshotState {
   private _counters: Map<string, number>;
   private _dirty: boolean;
@@ -173,7 +181,7 @@ export default class SnapshotState {
     key,
     inlineSnapshot,
     error,
-  }: SnapshotMatchOptions) {
+  }: SnapshotMatchOptions): SnapshotReturnOptions {
     this._counters.set(testName, (this._counters.get(testName) || 0) + 1);
     const count = Number(this._counters.get(testName));
     const isInline = inlineSnapshot !== undefined;
@@ -185,7 +193,7 @@ export default class SnapshotState {
     // Do not mark the snapshot as "checked" if the snapshot is inline and
     // there's an external snapshot. This way the external snapshot can be
     // removed with `--updateSnapshot`.
-    if (!(isInline && this._snapshotData[key])) {
+    if (!(isInline && this._snapshotData[key] !== undefined)) {
       this._uncheckedKeys.delete(key);
     }
 
@@ -248,7 +256,7 @@ export default class SnapshotState {
         return {
           actual: unescape(receivedSerialized),
           count,
-          expected: expected ? unescape(expected) : null,
+          expected: expected !== undefined ? unescape(expected) : undefined,
           key,
           pass: false,
         };

packages/jest-snapshot/src/index.ts

@@ -340,7 +340,7 @@ const _toMatchSnapshot = ({
   let report: () => string;
   if (pass) {
     return {message: () => '', pass: true};
-  } else if (!expected) {
+  } else if (expected === undefined) {
     report = () =>
       `New snapshot was ${RECEIVED_COLOR('not written')}. The update flag ` +
       `must be explicitly passed to write a new snapshot.\n\n` +
@@ -349,8 +349,8 @@ const _toMatchSnapshot = ({
       `${RECEIVED_COLOR('Received value')} ` +
       `${actual}`;
   } else {
-    expected = utils.removeExtraLineBreaks(expected || '');
-    actual = utils.removeExtraLineBreaks(actual || '');
+    expected = utils.removeExtraLineBreaks(expected);
+    actual = utils.removeExtraLineBreaks(actual);
 
     // Assign to local variable because of declaration let expected:
     // TypeScript thinks it could change before report function is called.