Fix race condition with --coverage and babel-jest identical file cont…

…ents edge case (#4432) * Add test case * Fix cache race condition by including filename in cache key * make filename cache key relative to rootDir * Update snapshot
jestjs · Sep 6, 2017 · 16431b5 · 16431b5
1 parent 2c910dc
commit 16431b5
Show file tree

Hide file tree

Showing 10 changed files with 114 additions and 11 deletions.
diff --git a/integration_tests/__tests__/__snapshots__/coverage_report.test.js.snap b/integration_tests/__tests__/__snapshots__/coverage_report.test.js.snap
@@ -1,5 +1,18 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`collects coverage from duplicate files avoiding shared cache 1`] = `
+"---------------|----------|----------|----------|----------|----------------|
+File           |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
+---------------|----------|----------|----------|----------|----------------|
+All files      |      100 |      100 |      100 |      100 |                |
+ a             |      100 |      100 |      100 |      100 |                |
+  identical.js |      100 |      100 |      100 |      100 |                |
+ b             |      100 |      100 |      100 |      100 |                |
+  identical.js |      100 |      100 |      100 |      100 |                |
+---------------|----------|----------|----------|----------|----------------|
+"
+`;
+
 exports[`collects coverage only from specified files 1`] = `
 "----------|----------|----------|----------|----------|----------------|
 File      |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
@@ -30,14 +43,19 @@ Ran all test suites.
 `;
 
 exports[`outputs coverage report 1`] = `
-"-------------------------------|----------|----------|----------|----------|----------------|
-File                           |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
--------------------------------|----------|----------|----------|----------|----------------|
-All files                      |    41.18 |        0 |       25 |    41.18 |                |
- not-required-in-test-suite.js |        0 |        0 |        0 |        0 |  9,16,17,18,20 |
- other-file.js                 |      100 |      100 |      100 |      100 |                |
- sum.js                        |    85.71 |      100 |       50 |    85.71 |             13 |
- sum_dependency.js             |        0 |        0 |        0 |        0 |     9,11,12,15 |
--------------------------------|----------|----------|----------|----------|----------------|
+"-------------------------------------|----------|----------|----------|----------|----------------|
+File                                 |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
+-------------------------------------|----------|----------|----------|----------|----------------|
+All files                            |    56.52 |        0 |       50 |    56.52 |                |
+ coverage_report                     |    41.18 |        0 |       25 |    41.18 |                |
+  not-required-in-test-suite.js      |        0 |        0 |        0 |        0 |  9,16,17,18,20 |
+  other-file.js                      |      100 |      100 |      100 |      100 |                |
+  sum.js                             |    85.71 |      100 |       50 |    85.71 |             13 |
+  sum_dependency.js                  |        0 |        0 |        0 |        0 |     9,11,12,15 |
+ coverage_report/cached-duplicates/a |      100 |      100 |      100 |      100 |                |
+  identical.js                       |      100 |      100 |      100 |      100 |                |
+ coverage_report/cached-duplicates/b |      100 |      100 |      100 |      100 |                |
+  identical.js                       |      100 |      100 |      100 |      100 |                |
+-------------------------------------|----------|----------|----------|----------|----------------|
 "
 `;
diff --git a/integration_tests/__tests__/coverage_report.test.js b/integration_tests/__tests__/coverage_report.test.js
@@ -79,3 +79,25 @@ test('outputs coverage report as json', () => {
     );
   }
 });
+
+test('collects coverage from duplicate files avoiding shared cache', () => {
+  const args = [
+    '--coverage',
+    // Ensure the status code is non-zero if super edge case with coverage triggers
+    '--coverageThreshold',
+    '{"global": {"lines": 100}}',
+    '--collectCoverageOnlyFrom',
+    'cached-duplicates/a/identical.js',
+    '--collectCoverageOnlyFrom',
+    'cached-duplicates/b/identical.js',
+    '--',
+    'identical.test.js',
+  ];
+  // Run once to prime the cache
+  runJest(DIR, args);
+
+  // Run for the second time
+  const {stdout, status} = runJest(DIR, args);
+  expect(stdout).toMatchSnapshot();
+  expect(status).toBe(0);
+});
diff --git a/integration_tests/coverage_report/cached-duplicates/a/__tests__/identical.test.js b/integration_tests/coverage_report/cached-duplicates/a/__tests__/identical.test.js
@@ -0,0 +1,16 @@
+/**
+ * Copyright (c) 2014-present, Facebook, Inc. All rights reserved.
+ *
+ * This source code is licensed under the BSD-style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ */
+'use strict';
+
+const sum = require('../identical').sum;
+
+describe('sum', () => {
+  it('adds numbers', () => {
+    expect(sum(1, 2)).toEqual(3);
+  });
+});
diff --git a/integration_tests/coverage_report/cached-duplicates/a/identical.js b/integration_tests/coverage_report/cached-duplicates/a/identical.js
@@ -0,0 +1,13 @@
+/**
+ * Copyright (c) 2014-present, Facebook, Inc. All rights reserved.
+ *
+ * This source code is licensed under the BSD-style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ */
+
+const sum = (a, b) => {
+  return a + b;
+};
+
+module.exports = {sum};
diff --git a/integration_tests/coverage_report/cached-duplicates/b/__tests__/identical.test.js b/integration_tests/coverage_report/cached-duplicates/b/__tests__/identical.test.js
@@ -0,0 +1,16 @@
+/**
+ * Copyright (c) 2014-present, Facebook, Inc. All rights reserved.
+ *
+ * This source code is licensed under the BSD-style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ */
+'use strict';
+
+const sum = require('../identical').sum;
+
+describe('sum', () => {
+  it('adds numbers', () => {
+    expect(sum(1, 2)).toEqual(3);
+  });
+});
diff --git a/integration_tests/coverage_report/cached-duplicates/b/identical.js b/integration_tests/coverage_report/cached-duplicates/b/identical.js
@@ -0,0 +1,13 @@
+/**
+ * Copyright (c) 2014-present, Facebook, Inc. All rights reserved.
+ *
+ * This source code is licensed under the BSD-style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ */
+
+const sum = (a, b) => {
+  return a + b;
+};
+
+module.exports = {sum};
diff --git a/packages/babel-jest/src/index.js b/packages/babel-jest/src/index.js
@@ -9,7 +9,7 @@
  */
 
 import type {Path, ProjectConfig} from 'types/Config';
-import type {TransformOptions} from 'types/Transform';
+import type {CacheKeyOptions, TransformOptions} from 'types/Transform';
 
 import crypto from 'crypto';
 import fs from 'fs';
@@ -82,14 +82,16 @@ const createTransformer = (options: any) => {
       fileData: string,
       filename: Path,
       configString: string,
-      {instrument}: TransformOptions,
+      {instrument, rootDir}: CacheKeyOptions,
     ): string {
       return crypto
         .createHash('md5')
         .update(THIS_FILE)
         .update('\0', 'utf8')
         .update(fileData)
         .update('\0', 'utf8')
+        .update(path.relative(rootDir, filename))
+        .update('\0', 'utf8')
         .update(configString)
         .update('\0', 'utf8')
         .update(getBabelRC(filename))

diff --git a/packages/jest-runtime/src/__tests__/__snapshots__/script_transformer.test.js.snap b/packages/jest-runtime/src/__tests__/__snapshots__/script_transformer.test.js.snap
@@ -4,6 +4,7 @@ exports[`ScriptTransformer passes expected transform options to getCacheKey 1`]
 Object {
   "instrument": true,
   "mapCoverage": true,
+  "rootDir": "/",
 }
 `;
 

diff --git a/packages/jest-runtime/src/script_transformer.js b/packages/jest-runtime/src/script_transformer.js
@@ -77,6 +77,7 @@ class ScriptTransformer {
           transformer.getCacheKey(fileData, filename, configString, {
             instrument,
             mapCoverage,
+            rootDir: this._config.rootDir,
           }),
         )
         .update(CACHE_VERSION)

diff --git a/types/Transform.js b/types/Transform.js
@@ -28,6 +28,7 @@ export type TransformOptions = {|
 export type CacheKeyOptions = {|
   instrument: boolean,
   mapCoverage: boolean,
+  rootDir: string,
 |};
 
 export type Transformer = {|