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: print underlying error when global hooks fail #11003

Merged
merged 6 commits into from Feb 9, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -29,6 +29,7 @@
- `[jest-circus]` Fixed the issue of beforeAll & afterAll hooks getting executed even if it is inside a skipped `describe` block [#10451](https://github.com/facebook/jest/issues/10451)
- `[jest-circus]` Fix `testLocation` on Windows when using `test.each` ([#10871](https://github.com/facebook/jest/pull/10871))
- `[jest-cli]` Use testFailureExitCode when bailing from a failed test ([#10958](https://github.com/facebook/jest/pull/10958))
- `[jest-cli]` Print custom error if error thrown from global hooks is not an error already ([#11003](https://github.com/facebook/jest/pull/11003))
- `[jest-config]` [**BREAKING**] Change default file extension order by moving json behind ts and tsx ([10572](https://github.com/facebook/jest/pull/10572))
- `[jest-console]` `console.dir` now respects the second argument correctly ([#10638](https://github.com/facebook/jest/pull/10638))
- `[jest-each]` [**BREAKING**] Ignore excess words in headings ([#8766](https://github.com/facebook/jest/pull/8766))
Expand Down
41 changes: 36 additions & 5 deletions e2e/__tests__/globalSetup.test.ts
Expand Up @@ -8,7 +8,12 @@
import {tmpdir} from 'os';
import * as path from 'path';
import * as fs from 'graceful-fs';
import {cleanup, runYarnInstall} from '../Utils';
import {
cleanup,
createEmptyPackage,
runYarnInstall,
writeFiles,
} from '../Utils';
import runJest, {json as runWithJson} from '../runJest';

const DIR = path.join(tmpdir(), 'jest-global-setup');
Expand All @@ -19,6 +24,7 @@ const customTransformDIR = path.join(
'jest-global-setup-custom-transform',
);
const nodeModulesDIR = path.join(tmpdir(), 'jest-global-setup-node-modules');
const rejectionDir = path.join(tmpdir(), 'jest-global-setup-rejection');
const e2eDir = path.resolve(__dirname, '../global-setup');

beforeAll(() => {
Expand All @@ -31,13 +37,16 @@ beforeEach(() => {
cleanup(project2DIR);
cleanup(customTransformDIR);
cleanup(nodeModulesDIR);
cleanup(rejectionDir);
});

afterAll(() => {
cleanup(DIR);
cleanup(project1DIR);
cleanup(project2DIR);
cleanup(customTransformDIR);
cleanup(nodeModulesDIR);
cleanup(rejectionDir);
});

test('globalSetup is triggered once before all test suites', () => {
Expand All @@ -62,8 +71,9 @@ test('jest throws an error when globalSetup does not export a function', () => {
]);

expect(exitCode).toBe(1);
expect(stderr).toMatch(
`TypeError: globalSetup file must export a function at ${setupPath}`,
expect(stderr).toContain('Jest: Got error running globalSetup');
expect(stderr).toContain(
`globalSetup file must export a function at ${setupPath}`,
);
});

Expand Down Expand Up @@ -145,8 +155,9 @@ test('globalSetup throws with named export', () => {
]);

expect(exitCode).toBe(1);
expect(stderr).toMatch(
`TypeError: globalSetup file must export a function at ${setupPath}`,
expect(stderr).toContain('Jest: Got error running globalSetup');
expect(stderr).toContain(
`globalSetup file must export a function at ${setupPath}`,
);
});

Expand All @@ -161,3 +172,23 @@ test('should transform node_modules if configured by transformIgnorePatterns', (

expect(exitCode).toBe(0);
});

test('properly handle rejections', () => {
createEmptyPackage(rejectionDir, {jest: {globalSetup: '<rootDir>/setup.js'}});
writeFiles(rejectionDir, {
'setup.js': `
module.exports = () => Promise.reject();
`,
'test.js': `
test('dummy', () => {
expect(true).toBe(true);
});
`,
});

const {exitCode, stderr} = runJest(rejectionDir, [`--no-cache`]);

expect(exitCode).toBe(1);
expect(stderr).toContain('Error: Jest: Got error running globalSetup');
expect(stderr).toContain('reason: undefined');
});
10 changes: 6 additions & 4 deletions e2e/__tests__/globalTeardown.test.ts
Expand Up @@ -55,8 +55,9 @@ test('jest throws an error when globalTeardown does not export a function', () =
]);

expect(exitCode).toBe(1);
expect(stderr).toMatch(
`TypeError: globalTeardown file must export a function at ${teardownPath}`,
expect(stderr).toContain('Jest: Got error running globalTeardown');
expect(stderr).toContain(
`globalTeardown file must export a function at ${teardownPath}`,
);
});

Expand Down Expand Up @@ -125,7 +126,8 @@ test('globalTeardown throws with named export', () => {
]);

expect(exitCode).toBe(1);
expect(stderr).toMatch(
`TypeError: globalTeardown file must export a function at ${teardownPath}`,
expect(stderr).toContain('Jest: Got error running globalTeardown');
expect(stderr).toContain(
`globalTeardown file must export a function at ${teardownPath}`,
);
});
2 changes: 1 addition & 1 deletion packages/jest-cli/src/cli/index.ts
Expand Up @@ -37,7 +37,7 @@ export async function run(
} catch (error) {
clearLine(process.stderr);
clearLine(process.stdout);
if (error.stack) {
if (error?.stack) {
console.error(chalk.red(error.stack));
} else {
console.error(chalk.red(error));
Expand Down
19 changes: 18 additions & 1 deletion packages/jest-core/src/runGlobalHook.ts
Expand Up @@ -5,11 +5,13 @@
* LICENSE file in the root directory of this source tree.
*/

import * as util from 'util';
import pEachSeries = require('p-each-series');
import {ScriptTransformer} from '@jest/transform';
import type {Config} from '@jest/types';
import type {Test} from 'jest-runner';
import {interopRequireDefault} from 'jest-util';
import prettyFormat from 'pretty-format';

export default async ({
allTests,
Expand All @@ -29,7 +31,7 @@ export default async ({
}

if (globalModulePaths.size > 0) {
await pEachSeries(Array.from(globalModulePaths), async modulePath => {
await pEachSeries(globalModulePaths, async modulePath => {
if (!modulePath) {
return;
}
Expand All @@ -45,6 +47,7 @@ export default async ({

const transformer = new ScriptTransformer(projectConfig);

try {
Copy link
Member Author

Choose a reason for hiding this comment

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

ignore whitespace

Copy link
Contributor

Choose a reason for hiding this comment

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

There is a link to whitespace-free diff: https://github.com/facebook/jest/pull/11003/files?w=1

await transformer.requireAndTranspileModule(modulePath, async m => {
const globalModule = interopRequireDefault(m).default;

Expand All @@ -56,6 +59,20 @@ export default async ({

await globalModule(globalConfig);
});
} catch (error) {
if (util.types.isNativeError(error)) {
error.message = `Jest: Got error running ${moduleName} - ${modulePath}, reason: ${error.message}`;

throw error;
}

throw new Error(
`Jest: Got error running ${moduleName} - ${modulePath}, reason: ${prettyFormat(
error,
{maxDepth: 3},
)}`,
);
}
});
}

Expand Down