Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(expect, jest-snapshot): Pass test.failing tests when containing failing snapshot matchers #14313

Merged
merged 10 commits into from
Oct 3, 2023
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,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"\`);
});
"
`;
61 changes: 61 additions & 0 deletions e2e/__tests__/testFailingSnapshot.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/**
* 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 runJest from '../runJest';

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"
}
}
3 changes: 3 additions & 0 deletions jest.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ export default {
'/packages/jest-snapshot/src/__tests__/fixtures/',
'/e2e/__tests__/iterator-to-null-test.ts',
'/e2e/__tests__/tsIntegration.test.ts', // this test needs types to be build, it runs in a separate CI job through `jest.config.ts.mjs`
...(process.env.JEST_JASMINE
? ['/e2e/__tests__/testFailingSnapshot.test.js']
: []),
KhaledElmorsy marked this conversation as resolved.
Show resolved Hide resolved
],
testTimeout: 70_000,
transform: {
Expand Down
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 @@ -50,6 +50,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 pure?: boolean;
};

type SnapshotReturnOptions = {
Expand Down Expand Up @@ -197,6 +198,7 @@ export default class SnapshotState {
inlineSnapshot,
isInline,
error,
pure = 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 runs, return the match result while skipping any updates
// reports.
if (pure) {
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
19 changes: 18 additions & 1 deletion packages/jest-snapshot/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,23 @@ 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;

/**
* Run the snapshot matcher as a normal throwing matcher with
* no side effects and no error supression.
*
* Skipped side effects occur in {@link snapshotState.match}
* and include updating the snapshot and recording passed/failed
* snapshots in the snapshot state.
*/
const pure = testFailing;
SimenB marked this conversation as resolved.
Show resolved Hide resolved

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

const {currentConcurrentTestName, isNot, snapshotState} = context;
const currentTestName =
Expand Down Expand Up @@ -359,6 +375,7 @@ const _toMatchSnapshot = (config: MatchSnapshotConfig) => {
error: context.error,
inlineSnapshot,
isInline,
pure,
received,
testName: fullTestName,
});
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