test(docs-infra): remove unnecessary test helpers (#25671)

`supertest.Request` extends `Promise` and can be used directly without
"promisifying".

PR Close #25671
This commit is contained in:
George Kalpakas 2018-09-06 15:48:00 +03:00 committed by Kara Erickson
parent d8d276c245
commit f113b49909
1 changed files with 20 additions and 27 deletions

View File

@ -2,7 +2,6 @@
import * as express from 'express'; import * as express from 'express';
import * as http from 'http'; import * as http from 'http';
import * as supertest from 'supertest'; import * as supertest from 'supertest';
import {promisify} from 'util';
import {CircleCiApi} from '../../lib/common/circle-ci-api'; import {CircleCiApi} from '../../lib/common/circle-ci-api';
import {GithubApi} from '../../lib/common/github-api'; import {GithubApi} from '../../lib/common/github-api';
import {GithubPullRequests} from '../../lib/common/github-pull-requests'; import {GithubPullRequests} from '../../lib/common/github-pull-requests';
@ -244,10 +243,6 @@ describe('PreviewServerFactory', () => {
let buildCreator: BuildCreator; let buildCreator: BuildCreator;
let agent: supertest.SuperTest<supertest.Test>; let agent: supertest.SuperTest<supertest.Test>;
// Helpers
const promisifyRequest = async (req: supertest.Request) => await promisify(req.end.bind(req))();
const verifyRequests = async (reqs: supertest.Request[]) => await Promise.all(reqs.map(promisifyRequest));
beforeEach(() => { beforeEach(() => {
const circleCiApi = new CircleCiApi(defaultConfig.githubOrg, defaultConfig.githubRepo, const circleCiApi = new CircleCiApi(defaultConfig.githubOrg, defaultConfig.githubRepo,
defaultConfig.circleCiToken); defaultConfig.circleCiToken);
@ -264,10 +259,11 @@ describe('PreviewServerFactory', () => {
agent = supertest.agent(middleware); agent = supertest.agent(middleware);
}); });
describe('GET /health-check', () => { describe('GET /health-check', () => {
it('should respond with 200', async () => { it('should respond with 200', async () => {
await verifyRequests([ await Promise.all([
agent.get('/health-check').expect(200), agent.get('/health-check').expect(200),
agent.get('/health-check/').expect(200), agent.get('/health-check/').expect(200),
]); ]);
@ -275,7 +271,7 @@ describe('PreviewServerFactory', () => {
it('should respond with 404 for non-GET requests', async () => { it('should respond with 404 for non-GET requests', async () => {
await verifyRequests([ await Promise.all([
agent.put('/health-check').expect(404), agent.put('/health-check').expect(404),
agent.post('/health-check').expect(404), agent.post('/health-check').expect(404),
agent.patch('/health-check').expect(404), agent.patch('/health-check').expect(404),
@ -285,7 +281,7 @@ describe('PreviewServerFactory', () => {
it('should respond with 404 if the path does not match exactly', async () => { it('should respond with 404 if the path does not match exactly', async () => {
await verifyRequests([ await Promise.all([
agent.get('/health-check/foo').expect(404), agent.get('/health-check/foo').expect(404),
agent.get('/health-check-foo').expect(404), agent.get('/health-check-foo').expect(404),
agent.get('/health-checknfoo').expect(404), agent.get('/health-checknfoo').expect(404),
@ -393,7 +389,8 @@ describe('PreviewServerFactory', () => {
}); });
describe('/circle-build', () => {
describe('POST /circle-build', () => {
let getGithubInfoSpy: jasmine.Spy; let getGithubInfoSpy: jasmine.Spy;
let getSignificantFilesChangedSpy: jasmine.Spy; let getSignificantFilesChangedSpy: jasmine.Spy;
let downloadBuildArtifactSpy: jasmine.Spy; let downloadBuildArtifactSpy: jasmine.Spy;
@ -566,7 +563,7 @@ describe('PreviewServerFactory', () => {
it('should respond with 404 for non-POST requests', async () => { it('should respond with 404 for non-POST requests', async () => {
await verifyRequests([ await Promise.all([
agent.get(url).expect(404), agent.get(url).expect(404),
agent.put(url).expect(404), agent.put(url).expect(404),
agent.patch(url).expect(404), agent.patch(url).expect(404),
@ -581,7 +578,7 @@ describe('PreviewServerFactory', () => {
const request1 = agent.post(url); const request1 = agent.post(url);
const request2 = agent.post(url).send(); const request2 = agent.post(url).send();
await verifyRequests([ await Promise.all([
request1.expect(400, responseBody), request1.expect(400, responseBody),
request2.expect(400, responseBody), request2.expect(400, responseBody),
]); ]);
@ -594,7 +591,7 @@ describe('PreviewServerFactory', () => {
const request1 = agent.post(url).send({}); const request1 = agent.post(url).send({});
const request2 = agent.post(url).send({number: null}); const request2 = agent.post(url).send({number: null});
await verifyRequests([ await Promise.all([
request1.expect(400, `${responseBodyPrefix} {}`), request1.expect(400, `${responseBodyPrefix} {}`),
request2.expect(400, `${responseBodyPrefix} {"number":null}`), request2.expect(400, `${responseBodyPrefix} {"number":null}`),
]); ]);
@ -602,7 +599,7 @@ describe('PreviewServerFactory', () => {
it('should call \'BuildVerifier#gtPrIsTrusted()\' with the correct arguments', async () => { it('should call \'BuildVerifier#gtPrIsTrusted()\' with the correct arguments', async () => {
await promisifyRequest(createRequest(+pr)); await createRequest(+pr);
expect(bvGetPrIsTrustedSpy).toHaveBeenCalledWith(9); expect(bvGetPrIsTrustedSpy).toHaveBeenCalledWith(9);
}); });
@ -610,9 +607,8 @@ describe('PreviewServerFactory', () => {
it('should propagate errors from BuildVerifier', async () => { it('should propagate errors from BuildVerifier', async () => {
bvGetPrIsTrustedSpy.and.callFake(() => Promise.reject('Test')); bvGetPrIsTrustedSpy.and.callFake(() => Promise.reject('Test'));
const req = createRequest(+pr).expect(500, 'Test'); await createRequest(+pr).expect(500, 'Test');
await promisifyRequest(req);
expect(bvGetPrIsTrustedSpy).toHaveBeenCalledWith(9); expect(bvGetPrIsTrustedSpy).toHaveBeenCalledWith(9);
expect(bcUpdatePrVisibilitySpy).not.toHaveBeenCalled(); expect(bcUpdatePrVisibilitySpy).not.toHaveBeenCalled();
}); });
@ -621,19 +617,17 @@ describe('PreviewServerFactory', () => {
it('should call \'BuildCreator#updatePrVisibility()\' with the correct arguments', async () => { it('should call \'BuildCreator#updatePrVisibility()\' with the correct arguments', async () => {
bvGetPrIsTrustedSpy.and.callFake((pr2: number) => Promise.resolve(pr2 === 42)); bvGetPrIsTrustedSpy.and.callFake((pr2: number) => Promise.resolve(pr2 === 42));
await promisifyRequest(createRequest(24)); await createRequest(24);
expect(bcUpdatePrVisibilitySpy).toHaveBeenCalledWith(24, false); expect(bcUpdatePrVisibilitySpy).toHaveBeenCalledWith(24, false);
await promisifyRequest(createRequest(42)); await createRequest(42);
expect(bcUpdatePrVisibilitySpy).toHaveBeenCalledWith(42, true); expect(bcUpdatePrVisibilitySpy).toHaveBeenCalledWith(42, true);
}); });
it('should propagate errors from BuildCreator', async () => { it('should propagate errors from BuildCreator', async () => {
bcUpdatePrVisibilitySpy.and.callFake(() => Promise.reject('Test')); bcUpdatePrVisibilitySpy.and.callFake(() => Promise.reject('Test'));
await createRequest(+pr).expect(500, 'Test');
const req = createRequest(+pr).expect(500, 'Test');
await verifyRequests([req]);
}); });
@ -643,7 +637,7 @@ describe('PreviewServerFactory', () => {
bvGetPrIsTrustedSpy.and.returnValues(Promise.resolve(true), Promise.resolve(false)); bvGetPrIsTrustedSpy.and.returnValues(Promise.resolve(true), Promise.resolve(false));
const reqs = [4, 2].map(num => createRequest(num).expect(200, http.STATUS_CODES[200])); const reqs = [4, 2].map(num => createRequest(num).expect(200, http.STATUS_CODES[200]));
await verifyRequests(reqs); await Promise.all(reqs);
}); });
@ -651,7 +645,7 @@ describe('PreviewServerFactory', () => {
bvGetPrIsTrustedSpy.and.returnValues(Promise.resolve(true), Promise.resolve(false)); bvGetPrIsTrustedSpy.and.returnValues(Promise.resolve(true), Promise.resolve(false));
const reqs = [4, 2].map(num => createRequest(num, 'labeled').expect(200, http.STATUS_CODES[200])); const reqs = [4, 2].map(num => createRequest(num, 'labeled').expect(200, http.STATUS_CODES[200]));
await verifyRequests(reqs); await Promise.all(reqs);
}); });
@ -659,14 +653,13 @@ describe('PreviewServerFactory', () => {
bvGetPrIsTrustedSpy.and.returnValues(Promise.resolve(true), Promise.resolve(false)); bvGetPrIsTrustedSpy.and.returnValues(Promise.resolve(true), Promise.resolve(false));
const reqs = [4, 2].map(num => createRequest(num, 'unlabeled').expect(200, http.STATUS_CODES[200])); const reqs = [4, 2].map(num => createRequest(num, 'unlabeled').expect(200, http.STATUS_CODES[200]));
await verifyRequests(reqs); await Promise.all(reqs);
}); });
it('should respond with 200 (and do nothing) if \'action\' implies no visibility change', async () => { it('should respond with 200 (and do nothing) if \'action\' implies no visibility change', async () => {
const promises = ['foo', 'notlabeled']. const promises = ['foo', 'notlabeled'].
map(action => createRequest(+pr, action).expect(200, http.STATUS_CODES[200])). map(action => createRequest(+pr, action).expect(200, http.STATUS_CODES[200]));
map(promisifyRequest);
await Promise.all(promises); await Promise.all(promises);
expect(bvGetPrIsTrustedSpy).not.toHaveBeenCalled(); expect(bvGetPrIsTrustedSpy).not.toHaveBeenCalled();
@ -683,7 +676,7 @@ describe('PreviewServerFactory', () => {
it('should respond with 404', async () => { it('should respond with 404', async () => {
const responseFor = (method: string) => `Unknown resource in request: ${method.toUpperCase()} /some/url`; const responseFor = (method: string) => `Unknown resource in request: ${method.toUpperCase()} /some/url`;
await verifyRequests([ await Promise.all([
agent.get('/some/url').expect(404, responseFor('get')), agent.get('/some/url').expect(404, responseFor('get')),
agent.put('/some/url').expect(404, responseFor('put')), agent.put('/some/url').expect(404, responseFor('put')),
agent.post('/some/url').expect(404, responseFor('post')), agent.post('/some/url').expect(404, responseFor('post')),