Skip to content

Commit

Permalink
Fix invalid re-run of tests in watch mode (#7347)
Browse files Browse the repository at this point in the history
* Add failing e2e test

* Stop re-running tests when mtime has not changed

* fix test

* fix copyright

* adjust file accessed check

* bring back reading fileMetadata from async callback

Co-authored-by: Michał Pierzchała <[email protected]>
  • Loading branch information
rubennorte and thymikee authored May 3, 2020
1 parent 5d1be03 commit 968a301
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
- `[jest-circus]` [**BREAKING**] Fail tests if a test takes a done callback and have return values ([#9129](https://github.com/facebook/jest/pull/9129))
- `[jest-circus]` [**BREAKING**] Throw a proper error if a test / hook is defined asynchronously ([#8096](https://github.com/facebook/jest/pull/8096))
- `[jest-config, jest-resolve]` [**BREAKING**] Remove support for `browser` field ([#9943](https://github.com/facebook/jest/pull/9943))
- `[jest-haste-map]` Stop reporting files as changed when they are only accessed ([#7347](https://github.com/facebook/jest/pull/7347))
- `[jest-resolve]` Show relative path from root dir for `module not found` errors ([#9963](https://github.com/facebook/jest/pull/9963))

### Chore & Maintenance
Expand Down
76 changes: 76 additions & 0 deletions e2e/__tests__/watchModeNoAccess.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
*/

import * as os from 'os';
import * as path from 'path';
import * as fs from 'graceful-fs';
import {cleanup, writeFiles} from '../Utils';
import {runContinuous} from '../runJest';

const DIR = path.resolve(os.tmpdir(), 'watch_mode_no_access');

const sleep = (time: number) =>
new Promise(resolve => setTimeout(resolve, time));

beforeEach(() => cleanup(DIR));
afterAll(() => cleanup(DIR));

const setupFiles = () => {
writeFiles(DIR, {
'__tests__/foo.test.js': `
const foo = require('../foo');
test('foo', () => { expect(typeof foo).toBe('number'); });
`,
'foo.js': `
module.exports = 0;
`,
'package.json': JSON.stringify({
jest: {},
}),
});
};

let testRun: ReturnType<typeof runContinuous>;

afterEach(async () => {
if (testRun) {
await testRun.end();
}
});

test('does not re-run tests when only access time is modified', async () => {
setupFiles();

testRun = runContinuous(DIR, ['--watchAll', '--no-watchman']);

const testCompletedRE = /Ran all test suites./g;
const numberOfTestRuns = (stderr: string): number => {
const matches = stderr.match(testCompletedRE);
return matches ? matches.length : 0;
};

// First run
await testRun.waitUntil(({stderr}) => numberOfTestRuns(stderr) === 1);

// Should re-run the test
const modulePath = path.join(DIR, 'foo.js');
const stat = fs.lstatSync(modulePath);
fs.utimesSync(modulePath, stat.atime.getTime(), stat.mtime.getTime());

await testRun.waitUntil(({stderr}) => numberOfTestRuns(stderr) === 2);

// Should NOT re-run the test
const fakeATime = 1541723621;
fs.utimesSync(modulePath, fakeATime, stat.mtime.getTime());
await sleep(3000);
expect(numberOfTestRuns(testRun.getCurrentOutput().stderr)).toBe(2);

// Should re-run the test
fs.writeFileSync(modulePath, 'module.exports = 1;', {encoding: 'utf-8'});
await testRun.waitUntil(({stderr}) => numberOfTestRuns(stderr) === 3);
});
14 changes: 13 additions & 1 deletion packages/jest-haste-map/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -846,6 +846,19 @@ class HasteMap extends EventEmitter {
return;
}

const relativeFilePath = fastPath.relative(rootDir, filePath);
const fileMetadata = hasteMap.files.get(relativeFilePath);

// The file has been accessed, not modified
if (
type === 'change' &&
fileMetadata &&
stat &&
fileMetadata[H.MTIME] === stat.mtime.getTime()
) {
return;
}

changeQueue = changeQueue
.then(() => {
// If we get duplicate events for the same file, ignore them.
Expand Down Expand Up @@ -879,7 +892,6 @@ class HasteMap extends EventEmitter {
return null;
};

const relativeFilePath = fastPath.relative(rootDir, filePath);
const fileMetadata = hasteMap.files.get(relativeFilePath);

// If it's not an addition, delete the file and all its metadata
Expand Down

0 comments on commit 968a301

Please sign in to comment.