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

Adds optional configuration snapshotTag #5838

Closed
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,12 @@ export const initialize = ({
});

const {expand, updateSnapshot} = globalConfig;
const snapshotState = new SnapshotState(testPath, {expand, updateSnapshot});
const snapshotTag = config.snapshotTag || globalConfig.snapshotTag || '';
const snapshotState = new SnapshotState(testPath, {
expand,
snapshotTag,
updateSnapshot,
});
setState({snapshotState, testPath});

// Return it back to the outer scope (test runner outside the VM).
Expand Down
4 changes: 4 additions & 0 deletions packages/jest-cli/src/cli/args.js
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,10 @@ export const options = {
'use for snapshot testing.',
type: 'array',
},
snapshotTag: {
description: "A tag to be appended to Jest's snapshots file name.",
type: 'string',
},
testEnvironment: {
description: 'Alias for --env',
type: 'string',
Expand Down
1 change: 1 addition & 0 deletions packages/jest-cli/src/test_scheduler.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ export default class TestScheduler {
const status = snapshot.cleanup(
context.hasteFS,
this._globalConfig.updateSnapshot,
context.config.snapshotTag,
);

aggregatedResults.snapshot.filesRemoved += status.filesRemoved;
Expand Down
4 changes: 3 additions & 1 deletion packages/jest-config/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ export function readConfig(
configPath = resolveConfigPath(packageRootOrConfig, process.cwd());
rawOptions = readConfigFileAndSetRootDir(configPath);
}

const {options, hasDeprecationWarnings} = normalize(rawOptions, argv);

const {globalConfig, projectConfig} = getConfigs(options);
return {
configPath,
Expand Down Expand Up @@ -133,6 +133,7 @@ const getConfigs = (
runTestsByPath: options.runTestsByPath,
silent: options.silent,
skipFilter: options.skipFilter,
snapshotTag: options.snapshotTag,
testFailureExitCode: options.testFailureExitCode,
testNamePattern: options.testNamePattern,
testPathPattern: options.testPathPattern,
Expand Down Expand Up @@ -178,6 +179,7 @@ const getConfigs = (
skipFilter: options.skipFilter,
skipNodeResolution: options.skipNodeResolution,
snapshotSerializers: options.snapshotSerializers,
snapshotTag: options.snapshotTag,
testEnvironment: options.testEnvironment,
testEnvironmentOptions: options.testEnvironmentOptions,
testLocationInResults: options.testLocationInResults,
Expand Down
1 change: 1 addition & 0 deletions packages/jest-config/src/normalize.js
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,7 @@ export default function normalize(options: InitialOptions, argv: Argv) {
case 'silent':
case 'skipFilter':
case 'skipNodeResolution':
case 'snapshotTag':
case 'testEnvironment':
case 'testEnvironmentOptions':
case 'testFailureExitCode':
Expand Down
8 changes: 7 additions & 1 deletion packages/jest-jasmine2/src/setup_jest_globals.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,13 @@ export default ({
});
patchJasmine();
const {expand, updateSnapshot} = globalConfig;
const snapshotState = new SnapshotState(testPath, {expand, updateSnapshot});
const snapshotTag = config.snapshotTag || globalConfig.snapshotTag || '';

const snapshotState = new SnapshotState(testPath, {
expand,
snapshotTag,
updateSnapshot,
});
setState({snapshotState, testPath});
// Return it back to the outer scope (test runner outside the VM).
return snapshotState;
Expand Down
6 changes: 3 additions & 3 deletions packages/jest-snapshot/src/State.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ import {
} from './utils';

export type SnapshotStateOptions = {|
updateSnapshot: SnapshotUpdateState,
snapshotPath?: string,
expand?: boolean,
snapshotTag?: string,
updateSnapshot: SnapshotUpdateState,
|};

export default class SnapshotState {
Expand All @@ -41,7 +41,7 @@ export default class SnapshotState {
updated: number;

constructor(testPath: Path, options: SnapshotStateOptions) {
this._snapshotPath = options.snapshotPath || getSnapshotPath(testPath);
this._snapshotPath = getSnapshotPath(testPath, options.snapshotTag || '');
const {data, dirty} = getSnapshotData(
this._snapshotPath,
options.updateSnapshot,
Expand Down
15 changes: 15 additions & 0 deletions packages/jest-snapshot/src/__tests__/utils.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,14 @@ const chalk = require('chalk');

const {
getSnapshotData,
getSnapshotExtension,
getSnapshotPath,
keyToTestName,
saveSnapshotFile,
serialize,
testNameToKey,
SNAPSHOT_FOLDER,
SNAPSHOT_EXTENSION,
SNAPSHOT_GUIDE_LINK,
SNAPSHOT_VERSION,
SNAPSHOT_VERSION_WARNING,
Expand Down Expand Up @@ -54,6 +57,18 @@ test('getSnapshotPath()', () => {
expect(getSnapshotPath('/abc/cde/a.test.js')).toBe(
path.join('/abc', 'cde', '__snapshots__', 'a.test.js.snap'),
);
expect(getSnapshotPath('/abc/cde/a.test.js')).toBe(
path.join('/abc', 'cde', SNAPSHOT_FOLDER, 'a.test.js.snap'),
);
expect(getSnapshotPath('/abc/cde/a.test.js', 'someSnapshotTag')).toBe(
path.join('/abc', 'cde', '__snapshots__', 'a.test.js.someSnapshotTag.snap'),
);
});

test('getSnapshotExtension()', () => {
expect(getSnapshotExtension()).toBe('.snap');
expect(getSnapshotExtension('')).toBe(`.${SNAPSHOT_EXTENSION}`);
expect(getSnapshotExtension('someSnapshotTag')).toBe('.someSnapshotTag.snap');
});

test('saveSnapshotFile() works with \r\n', () => {
Expand Down
35 changes: 24 additions & 11 deletions packages/jest-snapshot/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,34 @@ import * as utils from './utils';
const fileExists = (filePath: Path, hasteFS: HasteFS): boolean =>
hasteFS.exists(filePath) || fs.existsSync(filePath);

const cleanup = (hasteFS: HasteFS, update: SnapshotUpdateState) => {
const cleanup = (
hasteFS: HasteFS,
update: SnapshotUpdateState,
snapshotTag: ?string,
) => {
const pattern = '\\.' + utils.SNAPSHOT_EXTENSION + '$';
const files = hasteFS.matchFiles(pattern);
const filesRemoved = files
.filter(
snapshotFile =>
!fileExists(
path.resolve(
path.dirname(snapshotFile),
'..',
path.basename(snapshotFile, '.' + utils.SNAPSHOT_EXTENSION),
),
hasteFS,
.filter(snapshotFile => {
if (snapshotTag) {
const baseNameParts = path.basename(snapshotFile).split('.');
baseNameParts.splice(snapshotTag.split().length * -1 - 1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

ugh, I don't like splice at all, can we not mutate this array and use yet another variable?
Also, I feel like there's a nice opportunity to extract a function for finding snapshot associated with a file (some of the logic in line 37 and 44 is quite similar).

Copy link
Author

@sidferreira sidferreira Apr 26, 2018

Choose a reason for hiding this comment

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

Hey @thymikee! well, first of all, I'm all open to suggestions. The idea of the splice is the following:

I want to run my tests on two projects. One for iOS, one for Android. So my snapshotTags will be "ios" and "android".
What happens in this function is that it removes the "snapshot extension" to try to find out if there's a test file related to this snapshot or not. Now, let's imagine this:

./__tests__/RoundButton.js
./__tests__/__snapshots__/RoundButton.js.ios.snap -> created when running the project with ios tag
./__tests__/__snapshots__/RoundButton.js.android.snap -> created when running the project with android tag
./__tests__/__snapshots__/RoundButton.js.snap -> older test, when I didn't have tags yet.

for each time I run the tests, I know only the current snaptshotTag, so if I have a snapshotTag, I need to extract it from the name the snapshot (which in this case I tried to allow to have dots) and the extension. As I have no idea about how many dots will have, in the path and in the snapshotTag, I used "splice" and "split" to remove the needed elements.

As I wrote the text above, I noticed I could it make a bit simpler, and improved, dropping support to tags with "." in it.

I'm looking for your feedback on that.

const doesFileExists = path.resolve(
path.dirname(snapshotFile),
'..',
path.basename(baseNameParts.join('.')),
);
if (doesFileExists) return false;
}
return !fileExists(
path.resolve(
path.dirname(snapshotFile),
'..',
path.basename(snapshotFile, '.' + utils.SNAPSHOT_EXTENSION),
),
)
hasteFS,
);
})
.map(snapshotFile => {
if (update === 'all') {
fs.unlinkSync(snapshotFile);
Expand Down
10 changes: 7 additions & 3 deletions packages/jest-snapshot/src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import naturalCompare from 'natural-compare';
import path from 'path';
import prettyFormat from 'pretty-format';

export const SNAPSHOT_FOLDER = '__snapshots__';
export const SNAPSHOT_EXTENSION = 'snap';
export const SNAPSHOT_VERSION = '1';
const SNAPSHOT_VERSION_REGEXP = /^\/\/ Jest Snapshot v(.+),/;
Expand Down Expand Up @@ -88,10 +89,13 @@ export const keyToTestName = (key: string) => {
return key.replace(/ \d+$/, '');
};

export const getSnapshotPath = (testPath: Path) =>
export const getSnapshotExtension = (snapshotTag?: ?string) =>
(snapshotTag ? `.${snapshotTag}` : '') + '.' + SNAPSHOT_EXTENSION;

export const getSnapshotPath = (testPath: Path, snapshotTag?: ?string) =>
path.join(
path.join(path.dirname(testPath), '__snapshots__'),
path.basename(testPath) + '.' + SNAPSHOT_EXTENSION,
path.join(path.dirname(testPath), SNAPSHOT_FOLDER),
path.basename(testPath) + getSnapshotExtension(snapshotTag),
);

export const getSnapshotData = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ const validConfig = {
setupTestFrameworkScriptFile: '<rootDir>/testSetupFile.js',
silent: true,
snapshotSerializers: ['my-serializer-module'],
snapshotTag: 'valid',
testEnvironment: 'jest-environment-jsdom',
testNamePattern: 'test signature',
testPathIgnorePatterns: [NODE_MODULES_REGEXP],
Expand Down
1 change: 1 addition & 0 deletions types/Argv.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ export type Argv = {|
showConfig: boolean,
silent: boolean,
snapshotSerializers: Array<string>,
snapshotTag?: ?string,
testEnvironment: string,
testFailureExitCode: ?string,
testMatch: Array<string>,
Expand Down
4 changes: 4 additions & 0 deletions types/Config.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export type DefaultOptions = {|
runTestsByPath: boolean,
skipFilter: boolean,
snapshotSerializers: Array<Path>,
snapshotTag?: ?string,
testEnvironment: string,
testEnvironmentOptions: Object,
testFailureExitCode: string | number,
Expand Down Expand Up @@ -137,6 +138,7 @@ export type InitialOptions = {
skipFilter?: boolean,
skipNodeResolution?: boolean,
snapshotSerializers?: Array<Path>,
snapshotTag?: ?string,
testEnvironment?: string,
testEnvironmentOptions?: Object,
testFailureExitCode?: string | number,
Expand Down Expand Up @@ -204,6 +206,7 @@ export type GlobalConfig = {|
rootDir: Path,
silent: boolean,
skipFilter: boolean,
snapshotTag?: ?string,
testFailureExitCode: number,
testNamePattern: string,
testPathPattern: string,
Expand Down Expand Up @@ -250,6 +253,7 @@ export type ProjectConfig = {|
skipFilter: boolean,
skipNodeResolution: boolean,
snapshotSerializers: Array<Path>,
snapshotTag?: ?string,
testEnvironment: string,
testEnvironmentOptions: Object,
testMatch: Array<Glob>,
Expand Down