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.

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

fix: Windows-specific test timeout issues introduced by Yarn 1.13.0 #7838

Merged
merged 3 commits into from
Feb 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 0 additions & 1 deletion .azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ jobs:
git config --global core.autocrlf false
git config --global core.symlinks true
displayName: 'Preserve LF endings and symbolic links on check out'
- script: npm install -g yarn@1.9.4
- template: .azure-pipelines-steps.yml

- job: macOS
Expand Down
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
/website/translated_docs
/website/i18n/*

/reports/*

coverage
lerna-debug.log
npm-debug.log
Expand Down
71 changes: 28 additions & 43 deletions e2e/runJest.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

import path from 'path';
import fs from 'fs';
import execa, {sync as spawnSync} from 'execa';
import execa from 'execa';
import {Writable} from 'readable-stream';
import stripAnsi from 'strip-ansi';
import {normalizeIcons} from './Utils';
Expand All @@ -20,7 +20,8 @@ const JEST_PATH = path.resolve(__dirname, '../packages/jest-cli/bin/jest.js');
type RunJestOptions = {
nodePath?: string,
skipPkgJsonCheck?: boolean, // don't complain if can't find package.json
stripAnsi?: boolean, // remove colors from stdout and stderr
stripAnsi?: boolean, // remove colors from stdout and stderr,
timeout?: number, // kill the Jest process after X milliseconds
};

// return the result of the spawned process:
Expand All @@ -30,6 +31,16 @@ export default function runJest(
dir: string,
args?: Array<string>,
options: RunJestOptions = {},
) {
return normalizeResult(spawnJest(dir, args, options), options);
}

// Spawns Jest and returns either a Promise (if spawnAsync is true) or the completed child process
function spawnJest(
Copy link
Contributor

Choose a reason for hiding this comment

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

Pretty nice refactor to avoid the code duplication btw, regardless of the actual fix 👍

Copy link
Member

Choose a reason for hiding this comment

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

Yeah - I was gonna circle back to this when I wrote it, but never got around to it... Nice and clean now :)

dir: string,
args?: Array<string>,
options: RunJestOptions = {},
spawnAsync: boolean = false,
) {
const isRelative = !path.isAbsolute(dir);

Expand All @@ -51,12 +62,23 @@ export default function runJest(

const env = {...process.env, FORCE_COLOR: 0};
if (options.nodePath) env['NODE_PATH'] = options.nodePath;
const result = spawnSync(JEST_PATH, args || [], {

const spawnArgs = [JEST_PATH, ...(args || [])];
const spawnOptions = {
cwd: dir,
env,
reject: false,
});
timeout: options.timeout || 0,
};

return (spawnAsync ? execa : execa.sync)(
process.execPath,
spawnArgs,
spawnOptions,
);
}

function normalizeResult(result, options) {
// For compat with cross-spawn
result.status = result.code;

Expand Down Expand Up @@ -101,34 +123,7 @@ export const until = async function(
text: string,
options: RunJestOptions = {},
) {
const isRelative = !path.isAbsolute(dir);

if (isRelative) {
dir = path.resolve(__dirname, dir);
}

const localPackageJson = path.resolve(dir, 'package.json');
if (!options.skipPkgJsonCheck && !fs.existsSync(localPackageJson)) {
throw new Error(
`
Make sure you have a local package.json file at
"${localPackageJson}".
Otherwise Jest will try to traverse the directory tree and find the
the global package.json, which will send Jest into infinite loop.
`,
);
}

const env = {...process.env, FORCE_COLOR: 0};
if (options.nodePath) env['NODE_PATH'] = options.nodePath;

const jestPromise = execa(JEST_PATH, args || [], {
cwd: dir,
env,
reject: false,
// this should never take more than 5-6 seconds, bailout after 30
timeout: 30000,
});
const jestPromise = spawnJest(dir, args, {timeout: 30000, ...options}, true);

jestPromise.stderr.pipe(
new Writable({
Expand All @@ -144,15 +139,5 @@ export const until = async function(
}),
);

const result = await jestPromise;

// For compat with cross-spawn
result.status = result.code;

result.stdout = normalizeIcons(result.stdout);
if (options.stripAnsi) result.stdout = stripAnsi(result.stdout);
result.stderr = normalizeIcons(result.stderr);
if (options.stripAnsi) result.stderr = stripAnsi(result.stderr);

return result;
return normalizeResult(await jestPromise, options);
};