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

Support custom inline snapshot matchers #9278

Merged
merged 14 commits into from
Dec 8, 2019
Merged
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

### Features

- `[expect, jest-snapshot]` Support custom inline snapshot matchers ([#9278](https://github.com/facebook/jest/pull/9278))
SimenB marked this conversation as resolved.
Show resolved Hide resolved
- `[babel-plugin-jest-hoist]` Show codeframe on static hoisting issues ([#8865](https://github.com/facebook/jest/pull/8865))
- `[babel-plugin-jest-hoist]` Add `BigInt` to `WHITELISTED_IDENTIFIERS` ([#8382](https://github.com/facebook/jest/pull/8382))
- `[babel-preset-jest]` Add `@babel/plugin-syntax-bigint` ([#8382](https://github.com/facebook/jest/pull/8382))
Expand Down
52 changes: 52 additions & 0 deletions e2e/__tests__/__snapshots__/toMatchInlineSnapshot.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,41 @@ test('handles property matchers', () => {

`;

exports[`multiple custom matchers and native matchers: multiple matchers 1`] = `
const {toMatchInlineSnapshot} = require('jest-snapshot');
expect.extend({
toMatchCustomInlineSnapshot(received, ...args) {
return toMatchInlineSnapshot.call(this, received, ...args);
},
toMatchCustomInlineSnapshot2(received, ...args) {
return toMatchInlineSnapshot.call(this, received, ...args);
},
});
test('inline snapshots', () => {
expect({apple: 'value 1'}).toMatchCustomInlineSnapshot(\`
Object {
"apple": "value 1",
}
\`);
expect({apple: 'value 2'}).toMatchInlineSnapshot(\`
Object {
"apple": "value 2",
}
\`);
expect({apple: 'value 3'}).toMatchCustomInlineSnapshot2(\`
Object {
"apple": "value 3",
}
\`);
expect({apple: 'value 4'}).toMatchInlineSnapshot(\`
Object {
"apple": "value 4",
}
\`);
});

`;

exports[`removes obsolete external snapshots: external snapshot cleaned 1`] = `
test('removes obsolete external snapshots', () => {
expect('1').toMatchInlineSnapshot(\`"1"\`);
Expand Down Expand Up @@ -160,6 +195,23 @@ test('inline snapshots', async () => {

`;

exports[`supports custom matchers: custom matchers 1`] = `
const {toMatchInlineSnapshot} = require('jest-snapshot');
expect.extend({
toMatchCustomInlineSnapshot(received, ...args) {
return toMatchInlineSnapshot.call(this, received, ...args);
},
});
test('inline snapshots', () => {
expect({apple: 'original value'}).toMatchCustomInlineSnapshot(\`
Object {
"apple": "original value",
}
\`);
});

`;

exports[`writes snapshots with non-literals in expect(...) 1`] = `
it('works with inline snapshots', () => {
expect({a: 1}).toMatchInlineSnapshot(\`
Expand Down
50 changes: 50 additions & 0 deletions e2e/__tests__/toMatchInlineSnapshot.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -266,3 +266,53 @@ test('handles mocking native modules prettier relies on', () => {
expect(stderr).toMatch('1 snapshot written from 1 test suite.');
expect(exitCode).toBe(0);
});

test('supports custom matchers', () => {
const filename = 'custom-matchers.test.js';
const test = `
const { toMatchInlineSnapshot } = require('jest-snapshot');
expect.extend({
toMatchCustomInlineSnapshot(received, ...args) {
return toMatchInlineSnapshot.call(this, received, ...args);
}
});
test('inline snapshots', () => {
expect({apple: "original value"}).toMatchCustomInlineSnapshot();
});
`;

writeFiles(TESTS_DIR, {[filename]: test});
const {stderr, exitCode} = runJest(DIR, ['-w=1', '--ci=false', filename]);
const fileAfter = readFile(filename);
expect(stderr).toMatch('1 snapshot written from 1 test suite.');
expect(exitCode).toBe(0);
expect(wrap(fileAfter)).toMatchSnapshot('custom matchers');
});

test('multiple custom matchers and native matchers', () => {
const filename = 'multiple-matchers.test.js';
const test = `
const { toMatchInlineSnapshot } = require('jest-snapshot');
expect.extend({
toMatchCustomInlineSnapshot(received, ...args) {
return toMatchInlineSnapshot.call(this, received, ...args);
},
toMatchCustomInlineSnapshot2(received, ...args) {
return toMatchInlineSnapshot.call(this, received, ...args);
},
});
test('inline snapshots', () => {
expect({apple: "value 1"}).toMatchCustomInlineSnapshot();
expect({apple: "value 2"}).toMatchInlineSnapshot();
expect({apple: "value 3"}).toMatchCustomInlineSnapshot2();
expect({apple: "value 4"}).toMatchInlineSnapshot();
});
`;

writeFiles(TESTS_DIR, {[filename]: test});
const {stderr, exitCode} = runJest(DIR, ['-w=1', '--ci=false', filename]);
const fileAfter = readFile(filename);
expect(stderr).toMatch('4 snapshots written from 1 test suite.');
expect(exitCode).toBe(0);
expect(wrap(fileAfter)).toMatchSnapshot('multiple matchers');
});
16 changes: 12 additions & 4 deletions packages/expect/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ const makeThrowingMatcher = (
}
};

const handlError = (error: Error) => {
const handleError = (error: Error) => {
kevin940726 marked this conversation as resolved.
Show resolved Hide resolved
if (
matcher[INTERNAL_MATCHER_FLAG] === true &&
!(error instanceof JestAssertionError) &&
Expand All @@ -314,7 +314,15 @@ const makeThrowingMatcher = (
let potentialResult: ExpectationResult;

try {
potentialResult = matcher.call(matcherContext, actual, ...args);
potentialResult =
matcher[INTERNAL_MATCHER_FLAG] === true
? matcher.call(matcherContext, actual, ...args)
: // It's a trap specifically for inline snapshot to capture this name
// in the stack trace, so that it can correctly get the custom matcher
// function call.
(function __EXTERNAL_MATCHER_TRAP__() {
return matcher.call(matcherContext, actual, ...args);
})();

if (isPromise(potentialResult)) {
const asyncResult = potentialResult as AsyncExpectationResult;
Expand All @@ -325,14 +333,14 @@ const makeThrowingMatcher = (

return asyncResult
.then(aResult => processResult(aResult, asyncError))
.catch(error => handlError(error));
.catch(error => handleError(error));
} else {
const syncResult = potentialResult as SyncExpectationResult;

return processResult(syncResult);
}
} catch (error) {
return handlError(error);
return handleError(error);
}
};

Expand Down
5 changes: 4 additions & 1 deletion packages/jest-snapshot/src/State.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
getSnapshotData,
keyToTestName,
removeExtraLineBreaks,
removeLinesBeforeExternalMatcherTrap,
saveSnapshotFile,
serialize,
testNameToKey,
Expand Down Expand Up @@ -104,7 +105,9 @@ export default class SnapshotState {
this._dirty = true;
if (options.isInline) {
const error = options.error || new Error();
const lines = getStackTraceLines(error.stack || '');
const lines = getStackTraceLines(
removeLinesBeforeExternalMatcherTrap(error.stack || ''),
Copy link
Member

@SimenB SimenB Dec 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should getTopFrame do this work internally instead of the caller having to do it? It seems like the behaviour we'd always want, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the stack trace containing __EXTERNAL_MATCHER_TRAP__ will already got removed by getStackTraceLines?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we instead modify getStackTraceLines? Or both?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, good point. Is it enough to just change https://github.com/facebook/jest/blob/9ac2dcd55c0204960285498c590c1aa7860e6aa8/packages/jest-message-util/src/index.ts#L49 to not filter out this new capture line? If not I think the approach you have here makes sense

Copy link
Contributor Author

@kevin940726 kevin940726 Dec 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried, it works fine in this context, but it would break in formatStackTrace.

https://github.com/facebook/jest/blob/9ac2dcd55c0204960285498c590c1aa7860e6aa8/packages/jest-message-util/src/index.ts#L254-L255

If we omit all stack traces before the trap, then the custom matcher won't correctly format the stack trace. It's covered here in an e2e test.

https://github.com/facebook/jest/blob/9ac2dcd55c0204960285498c590c1aa7860e6aa8/e2e/custom-matcher-stack-trace/__tests__/sync.test.js#L41-L54

I believe that we have 2 options:

  1. Leave it be like this to only remove stack traces from the caller
  2. Add logic in formatStackTrace to un-skip the trap.

I think (1) is more resilient while (2) feel more error-prone. Wonder what's your opinion on this?

);
const frame = getTopFrame(lines);
if (!frame) {
throw new Error(
Expand Down
38 changes: 38 additions & 0 deletions packages/jest-snapshot/src/__tests__/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
getSnapshotData,
keyToTestName,
removeExtraLineBreaks,
removeLinesBeforeExternalMatcherTrap,
saveSnapshotFile,
serialize,
testNameToKey,
Expand Down Expand Up @@ -266,6 +267,43 @@ describe('ExtraLineBreaks', () => {
});
});

describe('removeLinesBeforeExternalMatcherTrap', () => {
test('contains external matcher trap', () => {
const stack = `Error:
at SnapshotState._addSnapshot (/jest/packages/jest-snapshot/build/State.js:150:9)
at SnapshotState.match (/jest/packages/jest-snapshot/build/State.js:303:14)
at _toMatchSnapshot (/jest/packages/jest-snapshot/build/index.js:399:32)
at _toThrowErrorMatchingSnapshot (/jest/packages/jest-snapshot/build/index.js:585:10)
at Object.toThrowErrorMatchingInlineSnapshot (/jest/packages/jest-snapshot/build/index.js:504:10)
at Object.<anonymous> (/jest/packages/expect/build/index.js:138:20)
at __EXTERNAL_MATCHER_TRAP__ (/jest/packages/expect/build/index.js:378:30)
at throwingMatcher (/jest/packages/expect/build/index.js:379:15)
at /jest/packages/expect/build/index.js:285:72
at Object.<anonymous> (/jest/e2e/to-throw-error-matching-inline-snapshot/__tests__/should-support-rejecting-promises.test.js:3:7)`;

const expected = ` at throwingMatcher (/jest/packages/expect/build/index.js:379:15)
at /jest/packages/expect/build/index.js:285:72
at Object.<anonymous> (/jest/e2e/to-throw-error-matching-inline-snapshot/__tests__/should-support-rejecting-promises.test.js:3:7)`;

expect(removeLinesBeforeExternalMatcherTrap(stack)).toBe(expected);
});

test("doesn't contain external matcher trap", () => {
const stack = `Error:
at SnapshotState._addSnapshot (/jest/packages/jest-snapshot/build/State.js:150:9)
at SnapshotState.match (/jest/packages/jest-snapshot/build/State.js:303:14)
at _toMatchSnapshot (/jest/packages/jest-snapshot/build/index.js:399:32)
at _toThrowErrorMatchingSnapshot (/jest/packages/jest-snapshot/build/index.js:585:10)
at Object.toThrowErrorMatchingInlineSnapshot (/jest/packages/jest-snapshot/build/index.js:504:10)
at Object.<anonymous> (/jest/packages/expect/build/index.js:138:20)
at throwingMatcher (/jest/packages/expect/build/index.js:379:15)
at /jest/packages/expect/build/index.js:285:72
at Object.<anonymous> (/jest/e2e/to-throw-error-matching-inline-snapshot/__tests__/should-support-rejecting-promises.test.js:3:7)`;

expect(removeLinesBeforeExternalMatcherTrap(stack)).toBe(stack);
});
});

describe('DeepMerge with property matchers', () => {
const matcher = expect.any(String);

Expand Down
28 changes: 24 additions & 4 deletions packages/jest-snapshot/src/inline_snapshots.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,21 +78,34 @@ const saveSnapshotsForFile = (
? prettier.getFileInfo.sync(sourceFilePath).inferredParser
: (config && config.parser) || simpleDetectParser(sourceFilePath);

// Record the matcher names seen in insertion parser and pass them down one
// by one to formatting parser.
const snapshotMatcherNames: Array<string> = [];

// Insert snapshots using the custom parser API. After insertion, the code is
// formatted, except snapshot indentation. Snapshots cannot be formatted until
// after the initial format because we don't know where the call expression
// will be placed (specifically its indentation).
const newSourceFile = prettier.format(sourceFile, {
...config,
filepath: sourceFilePath,
parser: createInsertionParser(snapshots, inferredParser, babelTraverse),
parser: createInsertionParser(
snapshots,
snapshotMatcherNames,
inferredParser,
babelTraverse,
),
});

// Format the snapshots using the custom parser API.
const formattedNewSourceFile = prettier.format(newSourceFile, {
...config,
filepath: sourceFilePath,
parser: createFormattingParser(inferredParser, babelTraverse),
parser: createFormattingParser(
snapshotMatcherNames,
inferredParser,
babelTraverse,
),
});

if (formattedNewSourceFile !== sourceFile) {
Expand Down Expand Up @@ -120,7 +133,7 @@ const groupSnapshotsByFile = groupSnapshotsBy(({frame: {file}}) => file);

const indent = (snapshot: string, numIndents: number, indentation: string) => {
const lines = snapshot.split('\n');
// Prevent re-identation of inline snapshots.
// Prevent re-indentation of inline snapshots.
if (
lines.length >= 2 &&
lines[1].startsWith(indentation.repeat(numIndents + 1))
Expand Down Expand Up @@ -166,6 +179,7 @@ const getAst = (
// This parser inserts snapshots into the AST.
const createInsertionParser = (
snapshots: Array<InlineSnapshot>,
snapshotMatcherNames: Array<string>,
inferredParser: string,
babelTraverse: Function,
) => (
Expand Down Expand Up @@ -198,6 +212,9 @@ const createInsertionParser = (
'Jest: Multiple inline snapshots for the same call are not supported.',
);
}

snapshotMatcherNames.push(callee.property.name);

const snapshotIndex = args.findIndex(
({type}) => type === 'TemplateLiteral',
);
Expand Down Expand Up @@ -228,6 +245,7 @@ const createInsertionParser = (

// This parser formats snapshots to the correct indentation.
const createFormattingParser = (
snapshotMatcherNames: Array<string>,
inferredParser: string,
babelTraverse: Function,
) => (
Expand All @@ -244,7 +262,7 @@ const createFormattingParser = (
if (
callee.type !== 'MemberExpression' ||
callee.property.type !== 'Identifier' ||
callee.property.name !== 'toMatchInlineSnapshot' ||
callee.property.name !== snapshotMatcherNames[0] ||
!callee.loc ||
callee.computed
) {
Expand All @@ -264,6 +282,8 @@ const createFormattingParser = (
return;
}

snapshotMatcherNames.shift();
SimenB marked this conversation as resolved.
Show resolved Hide resolved

const useSpaces = !options.useTabs;
snapshot = indent(
snapshot,
Expand Down
14 changes: 14 additions & 0 deletions packages/jest-snapshot/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,20 @@ export const removeExtraLineBreaks = (string: string): string =>
? string.slice(1, -1)
: string;

export const removeLinesBeforeExternalMatcherTrap = (stack: string): string => {
kevin940726 marked this conversation as resolved.
Show resolved Hide resolved
const lines = stack.split('\n');

for (let i = 0; i < lines.length; i += 1) {
// It's a function name specified in `packages/expect/src/index.ts`
// for external custom matchers.
if (lines[i].includes('__EXTERNAL_MATCHER_TRAP__')) {
return lines.slice(i + 1).join('\n');
}
}

return stack;
};

const escapeRegex = true;
const printFunctionName = false;

Expand Down