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.

Sign up for GitHub

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

Refactor configs & transform #3376

Merged
merged 3 commits into from
Apr 27, 2017
Merged

Refactor configs & transform #3376

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b11b011
Remove “updateSnapshot” from `ProjectConfig`, fix watch mode behavior.
cpojer Apr 22, 2017
d446332
Remove `verbose`, `watch`, `watchman`, `testNamePattern` from Project…
cpojer Apr 22, 2017
ee6d34f
Refactor transform.js into a ScriptTransformer
cpojer Apr 26, 2017
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
1 change: 1 addition & 0 deletions integration_tests/__tests__/__snapshots__/showConfig-test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ exports[`jest --showConfig outputs config info and exits 1`] = `
"noStackTrace": false,
"notify": false,
"rootDir": "/mocked/root/path/jest/integration_tests/verbose_reporter",
"testPathPattern": "",
"testResultsProcessor": null,
"useStderr": false,
"verbose": null,
Expand Down
102 changes: 47 additions & 55 deletions packages/babel-jest/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,50 +24,47 @@ const BABEL_CONFIG_KEY = 'babel';
const PACAKAGE_JSON = 'package.json';
const THIS_FILE = fs.readFileSync(__filename);

const cache = Object.create(null);

let babel;

const getBabelRC = (filename, {useCache}) => {
const paths = [];
let directory = filename;
while (directory !== (directory = path.dirname(directory))) {
if (useCache && cache[directory]) {
break;
}
const createTransformer = (options: any) => {
const cache = Object.create(null);

paths.push(directory);
const configFilePath = path.join(directory, BABELRC_FILENAME);
if (fs.existsSync(configFilePath)) {
cache[directory] = fs.readFileSync(configFilePath, 'utf8');
break;
}
const configJsFilePath = path.join(directory, BABELRC_JS_FILENAME);
if (fs.existsSync(configJsFilePath)) {
// $FlowFixMe
cache[directory] = JSON.stringify(require(configJsFilePath));
break;
}
const packageJsonFilePath = path.join(directory, PACAKAGE_JSON);
if (fs.existsSync(packageJsonFilePath)) {
// $FlowFixMe
const packageJsonFileContents = require(packageJsonFilePath);
if (packageJsonFileContents[BABEL_CONFIG_KEY]) {
cache[directory] = JSON.stringify(
packageJsonFileContents[BABEL_CONFIG_KEY],
);
const getBabelRC = filename => {
const paths = [];
let directory = filename;
while (directory !== (directory = path.dirname(directory))) {
if (cache[directory]) {
break;
}
}
}
paths.forEach(directoryPath => {
cache[directoryPath] = cache[directory];
});

return cache[directory] || '';
};
paths.push(directory);
const configFilePath = path.join(directory, BABELRC_FILENAME);
if (fs.existsSync(configFilePath)) {
cache[directory] = fs.readFileSync(configFilePath, 'utf8');
break;
}
const configJsFilePath = path.join(directory, BABELRC_JS_FILENAME);
if (fs.existsSync(configJsFilePath)) {
// $FlowFixMe
cache[directory] = JSON.stringify(require(configJsFilePath));
break;
}
const packageJsonFilePath = path.join(directory, PACAKAGE_JSON);
if (fs.existsSync(packageJsonFilePath)) {
// $FlowFixMe
const packageJsonFileContents = require(packageJsonFilePath);
if (packageJsonFileContents[BABEL_CONFIG_KEY]) {
cache[directory] = JSON.stringify(
packageJsonFileContents[BABEL_CONFIG_KEY],
);
break;
}
}
}
paths.forEach(directoryPath => (cache[directoryPath] = cache[directory]));
return cache[directory] || '';
};

const createTransformer = (options: any) => {
options = Object.assign({}, options, {
plugins: (options && options.plugins) || [],
presets: ((options && options.presets) || []).concat([jestPreset]),
Expand All @@ -82,24 +79,20 @@ const createTransformer = (options: any) => {
fileData: string,
filename: Path,
configString: string,
{instrument, watch}: TransformOptions,
{instrument}: TransformOptions,
): string {
return (
crypto
.createHash('md5')
.update(THIS_FILE)
.update('\0', 'utf8')
.update(fileData)
.update('\0', 'utf8')
.update(configString)
.update('\0', 'utf8')
// Don't use the in-memory cache in watch mode because the .babelrc
// file may be modified.
.update(getBabelRC(filename, {useCache: !watch}))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did it even work earlier?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yap this worked earlier but the code was terrible!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, what's the idea behind moving getBabelRC into createTransformer? Just curious

Copy link
Member Author

Choose a reason for hiding this comment

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

That's the reason I rewrote transform into ScriptTransformer. What we do now when we get a transformer, we check if createTransformer exists. If it does, we call that. We do this now once per Runtime instance (once per Test). This means every test will have its own babelrc file cache instead of having a single cache for the entirety of Jest's lifetime. Does that make sense?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Totally

.update('\0', 'utf8')
.update(instrument ? 'instrument' : '')
.digest('hex')
);
return crypto
.createHash('md5')
.update(THIS_FILE)
.update('\0', 'utf8')
.update(fileData)
.update('\0', 'utf8')
.update(configString)
.update('\0', 'utf8')
.update(getBabelRC(filename))
.update('\0', 'utf8')
.update(instrument ? 'instrument' : '')
.digest('hex');
},
process(
src: string,
Expand All @@ -116,7 +109,6 @@ const createTransformer = (options: any) => {
}

const theseOptions = Object.assign({filename}, options);

if (transformOptions && transformOptions.instrument) {
theseOptions.auxiliaryCommentBefore = ' istanbul ignore next ';
// Copied from jest-runtime transform.js
Expand Down
21 changes: 12 additions & 9 deletions packages/jest-cli/src/__tests__/watch-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,21 +32,20 @@ jest.doMock(
);

const watch = require('../watch');
const globalConfig = {
watch: true,
};
afterEach(runJestMock.mockReset);

describe('Watch mode flows', () => {
let pipe;
let hasteMapInstances;
let argv;
let globalConfig;
let contexts;
let stdin;

beforeEach(() => {
const config = {roots: [], testPathIgnorePatterns: [], testRegex: ''};
pipe = {write: jest.fn()};
globalConfig = {watch: true};
hasteMapInstances = [{on: () => {}}];
argv = {};
contexts = [{config}];
Expand All @@ -55,7 +54,7 @@ describe('Watch mode flows', () => {

it('Correctly passing test path pattern', () => {
argv.testPathPattern = 'test-*';
contexts[0].config.testPathPattern = 'test-*';
globalConfig.testPathPattern = 'test-*';

watch(globalConfig, contexts, argv, pipe, hasteMapInstances, stdin);

Expand All @@ -72,7 +71,7 @@ describe('Watch mode flows', () => {

it('Correctly passing test name pattern', () => {
argv.testNamePattern = 'test-*';
contexts[0].config.testNamePattern = 'test-*';
globalConfig.testNamePattern = 'test-*';

watch(globalConfig, contexts, argv, pipe, hasteMapInstances, stdin);

Expand Down Expand Up @@ -142,11 +141,15 @@ describe('Watch mode flows', () => {

stdin.emit(KEYS.U);

expect(runJestMock.mock.calls[0][1][0].config).toEqual({
roots: [],
testPathIgnorePatterns: [],
testRegex: '',
expect(runJestMock.mock.calls[0][0]).toEqual({
updateSnapshot: true,
watch: true,
});

stdin.emit(KEYS.A);
// updateSnapshot is not sticky after a run.
expect(runJestMock.mock.calls[1][0]).toEqual({
watch: true,
});
});
});
Expand Down
2 changes: 1 addition & 1 deletion packages/jest-cli/src/cli/runCLI.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ module.exports = async (
console: new Console(pipe, pipe),
maxWorkers: getMaxWorkers(argv),
resetCache: !config.cache,
watch: config.watch,
watch: globalConfig.watch,
});
hasteMapInstances[index] = hasteMapInstance;
return createContext(config, await hasteMapInstance.build());
Expand Down
8 changes: 6 additions & 2 deletions packages/jest-cli/src/generateEmptyCoverage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import type {ProjectConfig, Path} from 'types/Config';

const IstanbulInstrument = require('istanbul-lib-instrument');

const {transformSource, shouldInstrument} = require('jest-runtime');
const {ScriptTransformer, shouldInstrument} = require('jest-runtime');

module.exports = function(
source: string,
Expand All @@ -24,7 +24,11 @@ module.exports = function(
if (shouldInstrument(filename, config)) {
// Transform file without instrumentation first, to make sure produced
// source code is ES6 (no flowtypes etc.) and can be instrumented
const transformResult = transformSource(filename, config, source, false);
const transformResult = new ScriptTransformer(config).transformSource(
filename,
source,
false,
);
const instrumenter = IstanbulInstrument.createInstrumenter();
instrumenter.instrumentSync(transformResult.code, filename);
return {
Expand Down
35 changes: 23 additions & 12 deletions packages/jest-cli/src/lib/__tests__/isValidPath-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,29 +23,39 @@ const config = {
};

it('is valid when it is a file inside roots', () => {
expect(isValidPath(config, path.resolve(rootDir, 'src', 'index.js'))).toBe(
true,
);
expect(
isValidPath(config, path.resolve(rootDir, 'src', 'components', 'Link.js')),
isValidPath({}, config, path.resolve(rootDir, 'src', 'index.js')),
).toBe(true);
expect(
isValidPath(config, path.resolve(rootDir, 'src', 'lib', 'something.js')),
isValidPath(
{},
config,
path.resolve(rootDir, 'src', 'components', 'Link.js'),
),
).toBe(true);
expect(
isValidPath(
{},
config,
path.resolve(rootDir, 'src', 'lib', 'something.js'),
),
).toBe(true);
});

it('is not valid when it is a snapshot file', () => {
expect(
isValidPath(config, path.resolve(rootDir, 'src', 'index.js.snap')),
isValidPath({}, config, path.resolve(rootDir, 'src', 'index.js.snap')),
).toBe(false);
expect(
isValidPath(
{},
config,
path.resolve(rootDir, 'src', 'components', 'Link.js.snap'),
),
).toBe(false);
expect(
isValidPath(
{},
config,
path.resolve(rootDir, 'src', 'lib', 'something.js.snap'),
),
Expand All @@ -54,16 +64,17 @@ it('is not valid when it is a snapshot file', () => {

it('is not valid when it is a file in the coverage dir', () => {
expect(
isValidPath(config, path.resolve(rootDir, 'coverage', 'lib', 'index.js')),
isValidPath(
{},
config,
path.resolve(rootDir, 'coverage', 'lib', 'index.js'),
),
).toBe(false);

const configWithCoverage = Object.assign({}, config, {
coverageDirectory: 'cov-dir',
});

expect(
isValidPath(
configWithCoverage,
{coverageDirectory: 'cov-dir'},
config,
path.resolve(rootDir, 'src', 'cov-dir', 'lib', 'index.js'),
),
).toBe(false);
Expand Down
10 changes: 7 additions & 3 deletions packages/jest-cli/src/lib/isValidPath.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,19 @@
*/
'use strict';

import type {GlobalConfig} from 'types/Config';
import type {GlobalConfig, ProjectConfig} from 'types/Config';

const path = require('path');

const SNAPSHOT_EXTENSION = 'snap';

function isValidPath(config: GlobalConfig, filePath: string) {
function isValidPath(
globalConfig: GlobalConfig,
config: ProjectConfig,
filePath: string,
) {
const coverageDirectory =
config.coverageDirectory || path.resolve(config.rootDir, 'coverage');
globalConfig.coverageDirectory || path.resolve(config.rootDir, 'coverage');

return (
!filePath.includes(coverageDirectory) &&
Expand Down
Loading