jest-snapshot: Distinguish empty string from internal snapshot not written (#8898)

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

* Update CHANGELOG.md

Author: pedrottimark

CHANGELOG.md

@@ -19,6 +19,7 @@
 - `[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))
+- `[jest-snapshot]` [**BREAKING**] Distinguish empty string from internal snapshot not written ([#8898](https://github.com/facebook/jest/pull/8898))
 
 ### Chore & Maintenance
 

e2e/__tests__/toMatchSnapshotWithStringSerializer.test.ts

@@ -5,6 +5,7 @@
  * LICENSE file in the root directory of this source tree.
  */
 
+import * as fs from 'fs';
 import * as path from 'path';
 import {cleanup, makeTemplate, writeFiles} from '../Utils';
 import runJest from '../runJest';
@@ -15,9 +16,18 @@ const DIR = path.resolve(
 );
 const TESTS_DIR = path.resolve(DIR, '__tests__');
 
+const readFile = filename =>
+  fs.readFileSync(path.join(TESTS_DIR, filename), 'utf8');
+
 beforeEach(() => cleanup(TESTS_DIR));
 afterAll(() => cleanup(TESTS_DIR));
 
+// Because the not written error might include Received,
+// match Snapshot as either diff annotation or concise label.
+const ORDINARY_FAILURE = /- Snapshot|Snapshot:/;
+
+const NOT_WRITTEN = 'not written'; // new snapshot with --ci option
+
 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.
@@ -28,7 +38,7 @@ test('empty external', () => {
 
   {
     writeFiles(TESTS_DIR, {
-      [filename]: template(['""']), // empty string
+      [filename]: template([`''`]),
     });
     const {stderr, status} = runJest(DIR, ['-w=1', '--ci=false', filename]);
     expect(stderr).toMatch('1 snapshot written from 1 test suite.');
@@ -44,12 +54,64 @@ test('empty external', () => {
 
   {
     writeFiles(TESTS_DIR, {
-      [filename]: template(['"non-empty"']),
+      [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(stderr).toMatch(ORDINARY_FAILURE);
+    expect(status).toBe(1);
+  }
+});
+
+test('empty internal ci false', () => {
+  // Make sure empty string as expected value of internal snapshot
+  // is not confused with absence of snapshot.
+  const filename = 'empty-internal-ci-false.test.js';
+  const template = makeTemplate(
+    `test('string serializer', () => { expect($1).toMatchInlineSnapshot(); })`,
+  );
+
+  const received1 = `''`;
+  const received2 = `'non-empty'`;
+
+  {
+    writeFiles(TESTS_DIR, {
+      [filename]: template([received1]),
+    });
+    const {stderr, status} = runJest(DIR, ['-w=1', '--ci=false', filename]);
+    expect(stderr).toMatch('1 snapshot written from 1 test suite.');
+    expect(status).toBe(0);
+  }
+
+  {
+    writeFiles(TESTS_DIR, {
+      [filename]: readFile(filename).replace(received1, received2),
+    });
+    const {stderr, status} = runJest(DIR, ['-w=1', '--ci=false', filename]);
+    expect(stderr).toMatch('Snapshots:   1 failed, 1 total');
+    expect(stderr).not.toMatch('1 snapshot written from 1 test suite.');
+    expect(stderr).toMatch(ORDINARY_FAILURE);
+    expect(status).toBe(1);
+  }
+});
+
+test('undefined internal ci true', () => {
+  // Make sure absence of internal snapshot
+  // is not confused with ordinary failure for empty string as expected value.
+  const filename = 'undefined-internal-ci-true.test.js';
+  const template = makeTemplate(
+    `test('explicit update', () => { expect($1).toMatchInlineSnapshot(); })`,
+  );
+
+  {
+    writeFiles(TESTS_DIR, {
+      [filename]: template([`'non-empty'`]),
+    });
+    const {stderr, status} = runJest(DIR, ['-w=1', '--ci=true', filename]);
+    expect(stderr).toMatch('Snapshots:   1 failed, 1 total');
+    expect(stderr).not.toMatch(ORDINARY_FAILURE);
+    expect(stderr).toMatch(NOT_WRITTEN);
     expect(status).toBe(1);
   }
 });

packages/jest-snapshot/src/State.ts

@@ -32,6 +32,7 @@ export type SnapshotMatchOptions = {
   received: any;
   key?: string;
   inlineSnapshot?: string;
+  isInline: boolean;
   error?: Error;
 };
 
@@ -180,11 +181,11 @@ export default class SnapshotState {
     received,
     key,
     inlineSnapshot,
+    isInline,
     error,
   }: SnapshotMatchOptions): SnapshotReturnOptions {
     this._counters.set(testName, (this._counters.get(testName) || 0) + 1);
     const count = Number(this._counters.get(testName));
-    const isInline = inlineSnapshot !== undefined;
 
     if (!key) {
       key = testNameToKey(testName, count);
@@ -200,9 +201,7 @@ export default class SnapshotState {
     const receivedSerialized = serialize(received);
     const expected = isInline ? inlineSnapshot : this._snapshotData[key];
     const pass = expected === receivedSerialized;
-    const hasSnapshot = isInline
-      ? inlineSnapshot !== ''
-      : this._snapshotData[key] !== undefined;
+    const hasSnapshot = expected !== undefined;
     const snapshotIsPersisted = isInline || fs.existsSync(this._snapshotPath);
 
     if (pass && !isInline) {

packages/jest-snapshot/src/index.ts

@@ -36,6 +36,7 @@ type MatchSnapshotConfig = {
   expectedArgument: string;
   hint?: string;
   inlineSnapshot?: string;
+  isInline: boolean;
   matcherName: string;
   options: MatcherHintOptions;
   propertyMatchers?: any;
@@ -204,6 +205,7 @@ const toMatchSnapshot = function(
     context: this,
     expectedArgument,
     hint,
+    isInline: false,
     matcherName,
     options,
     propertyMatchers,
@@ -249,7 +251,11 @@ const toMatchInlineSnapshot = function(
   return _toMatchSnapshot({
     context: this,
     expectedArgument,
-    inlineSnapshot: stripAddedIndentation(inlineSnapshot || ''),
+    inlineSnapshot:
+      inlineSnapshot !== undefined
+        ? stripAddedIndentation(inlineSnapshot)
+        : undefined,
+    isInline: true,
     matcherName,
     options,
     propertyMatchers,
@@ -262,6 +268,7 @@ const _toMatchSnapshot = ({
   expectedArgument,
   hint,
   inlineSnapshot,
+  isInline,
   matcherName,
   options,
   propertyMatchers,
@@ -331,6 +338,7 @@ const _toMatchSnapshot = ({
   const result = snapshotState.match({
     error: context.error,
     inlineSnapshot,
+    isInline,
     received,
     testName: fullTestName,
   });
@@ -404,6 +412,7 @@ const toThrowErrorMatchingSnapshot = function(
       context: this,
       expectedArgument,
       hint,
+      isInline: false,
       matcherName,
       options,
       received,
@@ -431,7 +440,8 @@ const toThrowErrorMatchingInlineSnapshot = function(
     {
       context: this,
       expectedArgument,
-      inlineSnapshot: inlineSnapshot || '',
+      inlineSnapshot,
+      isInline: true,
       matcherName,
       options,
       received,
@@ -445,6 +455,7 @@ const _toThrowErrorMatchingSnapshot = (
     context,
     expectedArgument,
     inlineSnapshot,
+    isInline,
     matcherName,
     options,
     received,
@@ -488,6 +499,7 @@ const _toThrowErrorMatchingSnapshot = (
     expectedArgument,
     hint,
     inlineSnapshot,
+    isInline,
     matcherName,
     options,
     received: error.message,