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.

Sign up for GitHub

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

Jump to bottom

Adds test for #6968 #7000

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Adds test for #6968 #7000

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c3a05b7
Adds test for #6968
Feb 4, 2019
00c3352
Removed debug line
Feb 4, 2019
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
24 changes: 23 additions & 1 deletion __tests__/util/request-manager.js
View file
Open in desktop
Expand Up @@ -186,6 +186,7 @@ test('RequestManager.execute timeout error with default maxRetryAttempts', async

for (const statusCode of [403, 442]) {
test(`RequestManager.execute Request ${statusCode} error`, async () => {
jest.resetModules();
// The await await is just to silence Flow - https://github.com/facebook/flow/issues/6064
const config = await await Config.create({}, new Reporter());
const mockStatusCode = statusCode;
Expand All @@ -203,13 +204,34 @@ for (const statusCode of [403, 442]) {
resolve: body => {},
reject: err => {
expect(err.message).toBe(
`https://localhost:port/?nocache: Request "https://localhost:port/?nocache" returned a 403`,
`https://localhost:port/?nocache: Request "https://localhost:port/?nocache" returned a ${statusCode}`,
);
},
});
});
}

test('RequestManager.execute propagates non HTTP errors', async () => {
jest.resetModules();
jest.mock('request', factory => options => {
options.callback(new Error('bad stuff happened'), {}, '');
return {
on: () => {},
};
});
const config = await await Config.create({}, new Reporter());
await config.requestManager.execute({
params: {
url: `https://localhost:port/?nocache`,
headers: {Connection: 'close'},
},
resolve: body => {},
reject: err => {
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this callback be spied on? At the moment Jest doesn't check whether it's actually called 🤔

Copy link
Author

Choose a reason for hiding this comment

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

I was actually just following existing convention, but it looks like the tests are invoking the callback manually, which runs the "async" resolution. The await in front of config.requestManager.execute is misleading because RequestManager#execute is a void method.

The tests technically test the correct behavior, but could probably be a bit more straightforward.

expect(err.message).toBe('https://localhost:port/?nocache: bad stuff happened');
},
});
});

test('RequestManager.execute one time password error on npm request', async () => {
jest.resetModules();
jest.mock('request', factory => options => {
Expand Down