Skip to content

Commit

Permalink
fix(expect, jest-snapshot): Pass test.failing tests when containing…
Browse files Browse the repository at this point in the history
… failing snapshot matchers (jestjs#14313)
  • Loading branch information
KhaledElmorsy authored and nicojs committed Oct 13, 2023
1 parent f3e41ef commit c3b95f8
Show file tree
Hide file tree
Showing 21 changed files with 220 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
- `[babel-plugin-jest-hoist]` Use `denylist` instead of the deprecated `blacklist` for Babel 8 support ([#14109](https://github.com/jestjs/jest/pull/14109))
- `[expect]` Check error instance type for `toThrow/toThrowError` ([#14576](https://github.com/jestjs/jest/pull/14576))
- `[jest-circus]` [**BREAKING**] Prevent false test failures caused by promise rejections handled asynchronously ([#14315](https://github.com/jestjs/jest/pull/14315))
- `[jest-circus, jest-expect, jest-snapshot]` Pass `test.failing` tests when containing failing snapshot matchers ([#14313](https://github.com/jestjs/jest/pull/14313))
- `[jest-config]` Make sure to respect `runInBand` option ([#14578](https://github.com/facebook/jest/pull/14578))
- `[@jest/expect-utils]` Fix comparison of `DataView` ([#14408](https://github.com/jestjs/jest/pull/14408))
- `[jest-leak-detector]` Make leak-detector more aggressive when running GC ([#14526](https://github.com/jestjs/jest/pull/14526))
Expand Down
27 changes: 27 additions & 0 deletions e2e/__tests__/__snapshots__/testFailingSnapshot.test.js.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`test.failing doesnt update or remove snapshots 1`] = `
"// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[\`snapshots not updated nor removed 1\`] = \`"1"\`;
exports[\`snapshots not updated nor removed 2\`] = \`"1"\`;
exports[\`snapshots not updated nor removed 3\`] = \`"1"\`;
"
`;

exports[`test.failing doesnt update or remove snapshots 2`] = `
"/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
test.failing('inline snapshot not updated', () => {
// eslint-disable-next-line quotes
expect('0').toMatchInlineSnapshot(\`"1"\`);
});
"
`;
64 changes: 64 additions & 0 deletions e2e/__tests__/testFailingSnapshot.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* 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 * as fs from 'graceful-fs';
import {skipSuiteOnJasmine} from '@jest/test-utils';
import runJest from '../runJest';

skipSuiteOnJasmine();

describe('test.failing', () => {
describe('should pass when', () => {
test.failing('snapshot matchers fails', () => {
expect('0').toMatchSnapshot();
});

test.failing('snapshot doesnt exist', () => {
expect('0').toMatchSnapshot();
});

test.failing('inline snapshot matchers fails', () => {
expect('0').toMatchInlineSnapshot('0');
});

test.failing('at least one snapshot fails', () => {
expect('1').toMatchSnapshot();
expect('0').toMatchSnapshot();
});
});

describe('should fail when', () => {
test.each([
['snapshot', 'snapshot'],
['inline snapshot', 'inlineSnapshot'],
])('%s matchers pass', (_, fileName) => {
const dir = path.resolve(__dirname, '../test-failing-snapshot-all-pass');
const result = runJest(dir, [`./__tests__/${fileName}.test.js`]);
expect(result.exitCode).toBe(1);
});
});

it('doesnt update or remove snapshots', () => {
const dir = path.resolve(__dirname, '../test-failing-snapshot');
const result = runJest(dir, ['-u']);
expect(result.exitCode).toBe(0);
expect(result.stdout).not.toMatch(/snapshots? (written|removed|obsolete)/);

const snapshot = fs
.readFileSync(
path.resolve(dir, './__tests__/__snapshots__/snapshot.test.js.snap'),
)
.toString();
expect(snapshot).toMatchSnapshot();

const inlineSnapshot = fs
.readFileSync(path.resolve(dir, './__tests__/inlineSnapshot.test.js'))
.toString();
expect(inlineSnapshot).toMatchSnapshot();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`snapshots not updated 1`] = `"1"`;

exports[`snapshots not updated 2`] = `"1"`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

test.failing('inline snapshot not updated', () => {
// eslint-disable-next-line quotes
expect('1').toMatchInlineSnapshot(`"1"`);
});
11 changes: 11 additions & 0 deletions e2e/test-failing-snapshot-all-pass/__tests__/snapshot.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

test.failing('snapshots not updated', () => {
expect('1').toMatchSnapshot();
expect('1').toMatchSnapshot();
});
5 changes: 5 additions & 0 deletions e2e/test-failing-snapshot-all-pass/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"jest": {
"testEnvironment": "node"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`snapshots not updated nor removed 1`] = `"1"`;

exports[`snapshots not updated nor removed 2`] = `"1"`;

exports[`snapshots not updated nor removed 3`] = `"1"`;
11 changes: 11 additions & 0 deletions e2e/test-failing-snapshot/__tests__/inlineSnapshot.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

test.failing('inline snapshot not updated', () => {
// eslint-disable-next-line quotes
expect('0').toMatchInlineSnapshot(`"1"`);
});
12 changes: 12 additions & 0 deletions e2e/test-failing-snapshot/__tests__/snapshot.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

test.failing('snapshots not updated nor removed', () => {
expect('1').toMatchSnapshot();
expect('0').toMatchSnapshot();
expect('0').toMatchSnapshot();
});
5 changes: 5 additions & 0 deletions e2e/test-failing-snapshot/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"jest": {
"testEnvironment": "node"
}
}
12 changes: 9 additions & 3 deletions packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,16 @@ const _addSnapshotData = (
results: TestResult,
snapshotState: SnapshotState,
) => {
for (const {fullName, status} of results.testResults) {
if (status === 'pending' || status === 'failed') {
// if test is skipped or failed, we don't want to mark
for (const {fullName, status, failing} of results.testResults) {
if (
status === 'pending' ||
status === 'failed' ||
(failing && status === 'passed')
) {
// If test is skipped or failed, we don't want to mark
// its snapshots as obsolete.
// When tests called with test.failing pass, they've thrown an exception,
// so maintain any snapshots after the error.
snapshotState.markSnapshotsAsCheckedForTest(fullName);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ export const runAndTransformResultsToJestFormat = async ({
return {
ancestorTitles,
duration: testResult.duration,
failing: testResult.failing,
failureDetails: testResult.errorsDetailed,
failureMessages: testResult.errors,
fullName: title
Expand Down Expand Up @@ -242,7 +243,10 @@ const handleSnapshotStateAfterRetry =
const eventHandler = async (event: Circus.Event) => {
switch (event.name) {
case 'test_start': {
jestExpect.setState({currentTestName: getTestID(event.test)});
jestExpect.setState({
currentTestName: getTestID(event.test),
testFailing: event.test.failing,
});
break;
}
case 'test_done': {
Expand Down
2 changes: 2 additions & 0 deletions packages/jest-circus/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,7 @@ export const makeSingleTestResult = (
duration: test.duration,
errors: errorsDetailed.map(getErrorStack),
errorsDetailed,
failing: test.failing,
invocations: test.invocations,
location,
numPassingAsserts: test.numPassingAsserts,
Expand Down Expand Up @@ -502,6 +503,7 @@ export const parseSingleTestResult = (
return {
ancestorTitles,
duration: testResult.duration,
failing: testResult.failing,
failureDetails: testResult.errorsDetailed,
failureMessages: Array.from(testResult.errors),
fullName,
Expand Down
2 changes: 2 additions & 0 deletions packages/jest-expect/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ type PromiseMatchers<T = unknown> = {
declare module 'expect' {
interface MatcherState {
snapshotState: SnapshotState;
/** Whether the test was called with `test.failing()` */
testFailing?: boolean;
}
interface BaseExpect {
addSnapshotSerializer: typeof addSerializer;
Expand Down
19 changes: 19 additions & 0 deletions packages/jest-snapshot/src/State.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export type SnapshotMatchOptions = {
readonly inlineSnapshot?: string;
readonly isInline: boolean;
readonly error?: Error;
readonly testFailing?: boolean;
};

type SnapshotReturnOptions = {
Expand Down Expand Up @@ -197,6 +198,7 @@ export default class SnapshotState {
inlineSnapshot,
isInline,
error,
testFailing = false,
}: SnapshotMatchOptions): SnapshotReturnOptions {
this._counters.set(testName, (this._counters.get(testName) || 0) + 1);
const count = Number(this._counters.get(testName));
Expand Down Expand Up @@ -230,6 +232,23 @@ export default class SnapshotState {
this._snapshotData[key] = receivedSerialized;
}

// In pure matching only runs, return the match result while skipping any updates
// reports.
if (testFailing) {
if (hasSnapshot && !isInline) {
// Retain current snapshot values.
this._addSnapshot(key, expected, {error, isInline});
}
return {
actual: removeExtraLineBreaks(receivedSerialized),
count,
expected:
expected === undefined ? undefined : removeExtraLineBreaks(expected),
key,
pass,
};
}

// These are the conditions on when to write snapshots:
// * There's no snapshot file in a non-CI environment.
// * There is a snapshot file and we decided to update the snapshot.
Expand Down
9 changes: 8 additions & 1 deletion packages/jest-snapshot/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,13 @@ const _toMatchSnapshot = (config: MatchSnapshotConfig) => {
config;
let {received} = config;

context.dontThrow && context.dontThrow();
/** If a test was ran with `test.failing`. Passed by Jest Circus. */
const {testFailing = false} = context;

if (!testFailing && context.dontThrow) {
// Supress errors while running tests
context.dontThrow();
}

const {currentConcurrentTestName, isNot, snapshotState} = context;
const currentTestName =
Expand Down Expand Up @@ -360,6 +366,7 @@ const _toMatchSnapshot = (config: MatchSnapshotConfig) => {
inlineSnapshot,
isInline,
received,
testFailing,
testName: fullTestName,
});
const {actual, count, expected, pass} = result;
Expand Down
1 change: 1 addition & 0 deletions packages/jest-snapshot/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import type SnapshotState from './State';

export interface Context extends MatcherContext {
snapshotState: SnapshotState;
testFailing?: boolean;
}

// This is typically implemented by `jest-haste-map`'s `HasteFS`, but we
Expand Down
5 changes: 5 additions & 0 deletions packages/jest-test-result/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,11 @@ export type TestResult = {
console?: ConsoleBuffer;
coverage?: CoverageMapData;
displayName?: Config.DisplayName;
/**
* Whether [`test.failing()`](https://jestjs.io/docs/api#testfailingname-fn-timeout)
* was used.
*/
failing?: boolean;
failureMessage?: string | null;
leaks: boolean;
memoryUsage?: number;
Expand Down
5 changes: 5 additions & 0 deletions packages/jest-types/src/Circus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,11 @@ export type TestResult = {
duration?: number | null;
errors: Array<FormattedError>;
errorsDetailed: Array<MatcherResults | unknown>;
/**
* Whether [`test.failing()`](https://jestjs.io/docs/api#testfailingname-fn-timeout)
* was used.
*/
failing?: boolean;
invocations: number;
status: TestStatus;
location?: {column: number; line: number} | null;
Expand Down
5 changes: 5 additions & 0 deletions packages/jest-types/src/TestResult.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ type Callsite = {
export type AssertionResult = {
ancestorTitles: Array<string>;
duration?: number | null;
/**
* Whether [`test.failing()`](https://jestjs.io/docs/api#testfailingname-fn-timeout)
* was used.
*/
failing?: boolean;
failureDetails: Array<unknown>;
failureMessages: Array<string>;
fullName: string;
Expand Down

0 comments on commit c3b95f8

Please sign in to comment.