Skip to content

Commit 79523dc

Browse files
stipsantabrindle
stipsan
authored and
tabrindle
committed
Fix race condition with --coverage and babel-jest identical file contents edge case (jestjs#4432)
* Add test case * Fix cache race condition by including filename in cache key * make filename cache key relative to rootDir * Update snapshot
1 parent c8033a0 commit 79523dc

10 files changed

Lines changed: 114 additions & 11 deletions

File tree

‎integration_tests/__tests__/__snapshots__/coverage_report.test.js.snap‎

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,18 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

3+
exports[`collects coverage from duplicate files avoiding shared cache 1`] = `
4+
"---------------|----------|----------|----------|----------|----------------|
5+
File | % Stmts | % Branch | % Funcs | % Lines |Uncovered Lines |
6+
---------------|----------|----------|----------|----------|----------------|
7+
All files | 100 | 100 | 100 | 100 | |
8+
a | 100 | 100 | 100 | 100 | |
9+
identical.js | 100 | 100 | 100 | 100 | |
10+
b | 100 | 100 | 100 | 100 | |
11+
identical.js | 100 | 100 | 100 | 100 | |
12+
---------------|----------|----------|----------|----------|----------------|
13+
"
14+
`;
15+
316
exports[`collects coverage only from specified files 1`] = `
417
"----------|----------|----------|----------|----------|----------------|
518
File | % Stmts | % Branch | % Funcs | % Lines |Uncovered Lines |
@@ -30,14 +43,19 @@ Ran all test suites.
3043
`;
3144
3245
exports[`outputs coverage report 1`] = `
33-
"-------------------------------|----------|----------|----------|----------|----------------|
34-
File | % Stmts | % Branch | % Funcs | % Lines |Uncovered Lines |
35-
-------------------------------|----------|----------|----------|----------|----------------|
36-
All files | 41.18 | 0 | 25 | 41.18 | |
37-
not-required-in-test-suite.js | 0 | 0 | 0 | 0 | 9,16,17,18,20 |
38-
other-file.js | 100 | 100 | 100 | 100 | |
39-
sum.js | 85.71 | 100 | 50 | 85.71 | 13 |
40-
sum_dependency.js | 0 | 0 | 0 | 0 | 9,11,12,15 |
41-
-------------------------------|----------|----------|----------|----------|----------------|
46+
"-------------------------------------|----------|----------|----------|----------|----------------|
47+
File | % Stmts | % Branch | % Funcs | % Lines |Uncovered Lines |
48+
-------------------------------------|----------|----------|----------|----------|----------------|
49+
All files | 56.52 | 0 | 50 | 56.52 | |
50+
coverage_report | 41.18 | 0 | 25 | 41.18 | |
51+
not-required-in-test-suite.js | 0 | 0 | 0 | 0 | 9,16,17,18,20 |
52+
other-file.js | 100 | 100 | 100 | 100 | |
53+
sum.js | 85.71 | 100 | 50 | 85.71 | 13 |
54+
sum_dependency.js | 0 | 0 | 0 | 0 | 9,11,12,15 |
55+
coverage_report/cached-duplicates/a | 100 | 100 | 100 | 100 | |
56+
identical.js | 100 | 100 | 100 | 100 | |
57+
coverage_report/cached-duplicates/b | 100 | 100 | 100 | 100 | |
58+
identical.js | 100 | 100 | 100 | 100 | |
59+
-------------------------------------|----------|----------|----------|----------|----------------|
4260
"
4361
`;

‎integration_tests/__tests__/coverage_report.test.js‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,3 +79,25 @@ test('outputs coverage report as json', () => {
7979
);
8080
}
8181
});
82+
83+
test('collects coverage from duplicate files avoiding shared cache', () => {
84+
const args = [
85+
'--coverage',
86+
// Ensure the status code is non-zero if super edge case with coverage triggers
87+
'--coverageThreshold',
88+
'{"global": {"lines": 100}}',
89+
'--collectCoverageOnlyFrom',
90+
'cached-duplicates/a/identical.js',
91+
'--collectCoverageOnlyFrom',
92+
'cached-duplicates/b/identical.js',
93+
'--',
94+
'identical.test.js',
95+
];
96+
// Run once to prime the cache
97+
runJest(DIR, args);
98+
99+
// Run for the second time
100+
const {stdout, status} = runJest(DIR, args);
101+
expect(stdout).toMatchSnapshot();
102+
expect(status).toBe(0);
103+
});

‎integration_tests/coverage_report/cached-duplicates/a/__tests__/identical.test.js‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
/**
2+
* Copyright (c) 2014-present, Facebook, Inc. All rights reserved.
3+
*
4+
* This source code is licensed under the BSD-style license found in the
5+
* LICENSE file in the root directory of this source tree. An additional grant
6+
* of patent rights can be found in the PATENTS file in the same directory.
7+
*/
8+
'use strict';
9+
10+
const sum = require('../identical').sum;
11+
12+
describe('sum', () => {
13+
it('adds numbers', () => {
14+
expect(sum(1, 2)).toEqual(3);
15+
});
16+
});

‎integration_tests/coverage_report/cached-duplicates/a/identical.js‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
/**
2+
* Copyright (c) 2014-present, Facebook, Inc. All rights reserved.
3+
*
4+
* This source code is licensed under the BSD-style license found in the
5+
* LICENSE file in the root directory of this source tree. An additional grant
6+
* of patent rights can be found in the PATENTS file in the same directory.
7+
*/
8+
9+
const sum = (a, b) => {
10+
return a + b;
11+
};
12+
13+
module.exports = {sum};

‎integration_tests/coverage_report/cached-duplicates/b/__tests__/identical.test.js‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
/**
2+
* Copyright (c) 2014-present, Facebook, Inc. All rights reserved.
3+
*
4+
* This source code is licensed under the BSD-style license found in the
5+
* LICENSE file in the root directory of this source tree. An additional grant
6+
* of patent rights can be found in the PATENTS file in the same directory.
7+
*/
8+
'use strict';
9+
10+
const sum = require('../identical').sum;
11+
12+
describe('sum', () => {
13+
it('adds numbers', () => {
14+
expect(sum(1, 2)).toEqual(3);
15+
});
16+
});

‎integration_tests/coverage_report/cached-duplicates/b/identical.js‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
/**
2+
* Copyright (c) 2014-present, Facebook, Inc. All rights reserved.
3+
*
4+
* This source code is licensed under the BSD-style license found in the
5+
* LICENSE file in the root directory of this source tree. An additional grant
6+
* of patent rights can be found in the PATENTS file in the same directory.
7+
*/
8+
9+
const sum = (a, b) => {
10+
return a + b;
11+
};
12+
13+
module.exports = {sum};

‎packages/babel-jest/src/index.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
*/
1010

1111
import type {Path, ProjectConfig} from 'types/Config';
12-
import type {TransformOptions} from 'types/Transform';
12+
import type {CacheKeyOptions, TransformOptions} from 'types/Transform';
1313

1414
import crypto from 'crypto';
1515
import fs from 'fs';
@@ -82,14 +82,16 @@ const createTransformer = (options: any) => {
8282
fileData: string,
8383
filename: Path,
8484
configString: string,
85-
{instrument}: TransformOptions,
85+
{instrument, rootDir}: CacheKeyOptions,
8686
): string {
8787
return crypto
8888
.createHash('md5')
8989
.update(THIS_FILE)
9090
.update('\0', 'utf8')
9191
.update(fileData)
9292
.update('\0', 'utf8')
93+
.update(path.relative(rootDir, filename))
94+
.update('\0', 'utf8')
9395
.update(configString)
9496
.update('\0', 'utf8')
9597
.update(getBabelRC(filename))

‎packages/jest-runtime/src/__tests__/__snapshots__/script_transformer.test.js.snap‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ exports[`ScriptTransformer passes expected transform options to getCacheKey 1`]
44
Object {
55
"instrument": true,
66
"mapCoverage": true,
7+
"rootDir": "/",
78
}
89
`;
910

‎packages/jest-runtime/src/script_transformer.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ class ScriptTransformer {
7777
transformer.getCacheKey(fileData, filename, configString, {
7878
instrument,
7979
mapCoverage,
80+
rootDir: this._config.rootDir,
8081
}),
8182
)
8283
.update(CACHE_VERSION)

‎types/Transform.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ export type TransformOptions = {|
2828
export type CacheKeyOptions = {|
2929
instrument: boolean,
3030
mapCoverage: boolean,
31+
rootDir: string,
3132
|};
3233

3334
export type Transformer = {|

0 commit comments

Comments
 (0)
⚡