From f102dba8ed957a9aab1bb5dc456c35f41cf21661 Mon Sep 17 00:00:00 2001 From: Jean Lauliac Date: Thu, 20 Jul 2017 11:58:18 +0100 Subject: [PATCH] fix script_transformer test --- .../src/__tests__/script_transformer.test.js | 39 +++++++++++++------ .../jest-runtime/src/script_transformer.js | 11 +++++- 2 files changed, 37 insertions(+), 13 deletions(-) diff --git a/packages/jest-runtime/src/__tests__/script_transformer.test.js b/packages/jest-runtime/src/__tests__/script_transformer.test.js index c1c1b5d56a20..423f3231a3bd 100644 --- a/packages/jest-runtime/src/__tests__/script_transformer.test.js +++ b/packages/jest-runtime/src/__tests__/script_transformer.test.js @@ -9,9 +9,11 @@ */ 'use strict'; +const crypto = require('crypto'); const slash = require('slash'); jest + .mock('fs') .mock('graceful-fs') .mock('jest-haste-map', () => ({ getCacheFilePath: (cacheDir, baseDir, version) => cacheDir + baseDir, @@ -100,6 +102,14 @@ let fs; let mockFs; let object; let vm; +let writeFileAtomic; + +jest.mock('write-file-atomic', () => ({ + sync: jest.fn().mockImplementation((filePath, data) => { + const normalizedPath = require('slash')(filePath); + mockFs[normalizedPath] = data; + }), +})); describe('ScriptTransformer', () => { const reset = () => { @@ -145,6 +155,8 @@ describe('ScriptTransformer', () => { fs.existsSync = jest.fn(path => !!mockFs[path]); + writeFileAtomic = require('write-file-atomic'); + config = { cache: true, cacheDirectory: '/cache/', @@ -290,10 +302,13 @@ describe('ScriptTransformer', () => { mapCoverage: true, }); expect(result.sourceMapPath).toEqual(expect.any(String)); - expect(fs.writeFileSync).toBeCalledWith( + const mapStr = JSON.stringify(map); + expect( + writeFileAtomic.sync, + ).toBeCalledWith( result.sourceMapPath, - JSON.stringify(map), - 'utf8', + crypto.createHash('md5').update(mapStr).digest('hex') + mapStr, + {encoding: 'utf8'}, ); }); @@ -320,10 +335,12 @@ describe('ScriptTransformer', () => { mapCoverage: true, }); expect(result.sourceMapPath).toEqual(expect.any(String)); - expect(fs.writeFileSync).toBeCalledWith( + expect( + writeFileAtomic.sync, + ).toBeCalledWith( result.sourceMapPath, - sourceMap, - 'utf8', + crypto.createHash('md5').update(sourceMap).digest('hex') + sourceMap, + {encoding: 'utf8'}, ); }); @@ -348,7 +365,7 @@ describe('ScriptTransformer', () => { mapCoverage: false, }); expect(result.sourceMapPath).toBeFalsy(); - expect(fs.writeFileSync).toHaveBeenCalledTimes(1); + expect(writeFileAtomic.sync).toHaveBeenCalledTimes(1); }); it('reads values from the cache', () => { @@ -359,8 +376,8 @@ describe('ScriptTransformer', () => { scriptTransformer.transform('/fruits/banana.js', {}); const cachePath = getCachePath(mockFs, config); - expect(fs.writeFileSync).toBeCalled(); - expect(fs.writeFileSync.mock.calls[0][0]).toBe(cachePath); + expect(writeFileAtomic.sync).toBeCalled(); + expect(writeFileAtomic.sync.mock.calls[0][0]).toBe(cachePath); // Cache the state in `mockFsCopy` const mockFsCopy = mockFs; @@ -375,7 +392,7 @@ describe('ScriptTransformer', () => { expect(fs.readFileSync.mock.calls.length).toBe(2); expect(fs.readFileSync).toBeCalledWith('/fruits/banana.js', 'utf8'); expect(fs.readFileSync).toBeCalledWith(cachePath, 'utf8'); - expect(fs.writeFileSync).not.toBeCalled(); + expect(writeFileAtomic.sync).not.toBeCalled(); // Don't read from the cache when `config.cache` is false. jest.resetModuleRegistry(); @@ -388,6 +405,6 @@ describe('ScriptTransformer', () => { expect(fs.readFileSync.mock.calls.length).toBe(1); expect(fs.readFileSync).toBeCalledWith('/fruits/banana.js', 'utf8'); expect(fs.readFileSync).not.toBeCalledWith(cachePath, 'utf8'); - expect(fs.writeFileSync).toBeCalled(); + expect(writeFileAtomic.sync).toBeCalled(); }); }); diff --git a/packages/jest-runtime/src/script_transformer.js b/packages/jest-runtime/src/script_transformer.js index 41d22881c747..bc71d536564b 100644 --- a/packages/jest-runtime/src/script_transformer.js +++ b/packages/jest-runtime/src/script_transformer.js @@ -191,6 +191,10 @@ class ScriptTransformer { : null; if (code) { + // This is broken: we return the code, and a path for the source map + // directly from the cache. But, nothing ensures the source map actually + // matches that source code. They could have gotten out-of-sync in case + // two separate processes write concurrently to the same cache files. return { code, sourceMapPath, @@ -385,8 +389,11 @@ const readCacheFile = (filename: Path, cachePath: Path): ?string => { return null; } } catch (e) { - e.message = 'jest: failed to read cache file: ' + cachePath + - '\nFailure message: ' + e.message; + e.message = + 'jest: failed to read cache file: ' + + cachePath + + '\nFailure message: ' + + e.message; removeFile(cachePath); throw e; }