refactor(docs-infra): take advantage of latest Jasmine features in preview server tests (#36837)

This commit updates the preview server tests to take advantage of
features supported in the latest version of Jasmine that were not
supported when the rests were first written.

Changes include:

- Use [async/await] in tests.
- Use the new [toBeInstanceOf()] and [toHaveBeenCalledBefore()] matchers.
- Use the new [toBeResolved()] and [toBeRejected()] async matchers (and
  their variants).
- Use the new [withArgs()] method of `Spy` to simplify "trained"
  responses.
- Use the new [resolveTo()]/[rejectWith()] methods of `SpyStrategy` (and
  their variants) to simplify promise-based spies.
- Implement custom async matchers (via [addAsyncMatchers()]) to simplify
  certain tests.

[addAsyncMatchers()]: https://jasmine.github.io/api/3.5/jasmine.html#.addAsyncMatchers
[async/await]: https://jasmine.github.io/tutorials/async
[rejectWith()]: https://jasmine.github.io/api/3.5/SpyStrategy.html#rejectWith
[resolveTo()]: https://jasmine.github.io/api/3.5/SpyStrategy.html#resolveTo
[toBeInstanceOf()]: https://jasmine.github.io/api/3.5/matchers.html#toBeInstanceOf
[toBeRejected()]: https://jasmine.github.io/api/3.5/async-matchers.html#toBeRejected
[toBeResolved()]: https://jasmine.github.io/api/3.5/async-matchers.html#toBeResolved
[toHaveBeenCalledBefore()]: https://jasmine.github.io/api/3.5/matchers.html#toHaveBeenCalledBefore
[withArgs()]: https://jasmine.github.io/api/3.5/Spy.html#withArgs

PR Close #36837
This commit is contained in:
George Kalpakas
2020-05-02 16:14:14 +03:00
committed by Alex Rickabaugh
parent b3cf5b246f
commit 1b8752e595
17 changed files with 623 additions and 808 deletions

View File

@ -229,7 +229,7 @@ describe('PreviewServerFactory', () => {
expect(prsAddCommentSpy).toHaveBeenCalledTimes(2);
expect(prs).toBe(allCalls[1].object);
expect(prs).toEqual(jasmine.any(GithubPullRequests));
expect(prs).toBeInstanceOf(GithubPullRequests);
expect(prs.repoSlug).toBe('organisation/repo');
});
@ -301,9 +301,8 @@ describe('PreviewServerFactory', () => {
let bvGetSignificantFilesChangedSpy: jasmine.Spy;
beforeEach(() => {
bvGetPrIsTrustedSpy = spyOn(buildVerifier, 'getPrIsTrusted').and.returnValue(Promise.resolve(true));
bvGetSignificantFilesChangedSpy = spyOn(buildVerifier, 'getSignificantFilesChanged').
and.returnValue(Promise.resolve(true));
bvGetPrIsTrustedSpy = spyOn(buildVerifier, 'getPrIsTrusted').and.resolveTo(true);
bvGetSignificantFilesChangedSpy = spyOn(buildVerifier, 'getSignificantFilesChanged').and.resolveTo(true);
});
@ -330,7 +329,7 @@ describe('PreviewServerFactory', () => {
it('should respond appropriately if the PR did not touch any significant files', async () => {
bvGetSignificantFilesChangedSpy.and.returnValue(Promise.resolve(false));
bvGetSignificantFilesChangedSpy.and.resolveTo(false);
const expectedResponse = {canHavePublicPreview: false, reason: 'No significant files touched.'};
const expectedLog = `PR:${pr} - Cannot have a public preview, because it did not touch any significant files.`;
@ -344,7 +343,7 @@ describe('PreviewServerFactory', () => {
it('should respond appropriately if the PR is not automatically verifiable as "trusted"', async () => {
bvGetPrIsTrustedSpy.and.returnValue(Promise.resolve(false));
bvGetPrIsTrustedSpy.and.resolveTo(false);
const expectedResponse = {canHavePublicPreview: false, reason: 'Not automatically verifiable as "trusted".'};
const expectedLog =
@ -371,7 +370,7 @@ describe('PreviewServerFactory', () => {
it('should respond with error if `getSignificantFilesChanged()` fails', async () => {
bvGetSignificantFilesChangedSpy.and.callFake(() => Promise.reject('getSignificantFilesChanged error'));
bvGetSignificantFilesChangedSpy.and.rejectWith('getSignificantFilesChanged error');
await agent.get(url).expect(500, 'getSignificantFilesChanged error');
expect(loggerErrorSpy).toHaveBeenCalledWith('Previewability check error', 'getSignificantFilesChanged error');
@ -379,11 +378,10 @@ describe('PreviewServerFactory', () => {
it('should respond with error if `getPrIsTrusted()` fails', async () => {
const error = new Error('getPrIsTrusted error');
bvGetPrIsTrustedSpy.and.callFake(() => { throw error; });
bvGetPrIsTrustedSpy.and.throwError('getPrIsTrusted error');
await agent.get(url).expect(500, 'getPrIsTrusted error');
expect(loggerErrorSpy).toHaveBeenCalledWith('Previewability check error', error);
expect(loggerErrorSpy).toHaveBeenCalledWith('Previewability check error', new Error('getPrIsTrusted error'));
});
});
@ -496,7 +494,7 @@ describe('PreviewServerFactory', () => {
// Note it is important to put the `reject` into `and.callFake`;
// If you just `and.returnValue` the rejected promise
// then you get an "unhandled rejection" message in the console.
getGithubInfoSpy.and.callFake(() => Promise.reject('Test Error'));
getGithubInfoSpy.and.rejectWith('Test Error');
await agent.post(URL).send(BASIC_PAYLOAD).expect(500, 'Test Error');
expect(getGithubInfoSpy).toHaveBeenCalledWith(BUILD_NUM);
expect(downloadBuildArtifactSpy).not.toHaveBeenCalled();
@ -517,7 +515,7 @@ describe('PreviewServerFactory', () => {
});
it('should fail if the artifact fetch request fails', async () => {
downloadBuildArtifactSpy.and.callFake(() => Promise.reject('Test Error'));
downloadBuildArtifactSpy.and.rejectWith('Test Error');
await agent.post(URL).send(BASIC_PAYLOAD).expect(500, 'Test Error');
expect(getGithubInfoSpy).toHaveBeenCalledWith(BUILD_NUM);
expect(downloadBuildArtifactSpy).toHaveBeenCalled();
@ -526,7 +524,7 @@ describe('PreviewServerFactory', () => {
});
it('should fail if verifying the PR fails', async () => {
getPrIsTrustedSpy.and.callFake(() => Promise.reject('Test Error'));
getPrIsTrustedSpy.and.rejectWith('Test Error');
await agent.post(URL).send(BASIC_PAYLOAD).expect(500, 'Test Error');
expect(getGithubInfoSpy).toHaveBeenCalledWith(BUILD_NUM);
expect(downloadBuildArtifactSpy).toHaveBeenCalled();
@ -535,7 +533,7 @@ describe('PreviewServerFactory', () => {
});
it('should fail if creating the preview build fails', async () => {
createBuildSpy.and.callFake(() => Promise.reject('Test Error'));
createBuildSpy.and.rejectWith('Test Error');
await agent.post(URL).send(BASIC_PAYLOAD).expect(500, 'Test Error');
expect(getGithubInfoSpy).toHaveBeenCalledWith(BUILD_NUM);
expect(downloadBuildArtifactSpy).toHaveBeenCalled();
@ -604,7 +602,7 @@ describe('PreviewServerFactory', () => {
it('should propagate errors from BuildVerifier', async () => {
bvGetPrIsTrustedSpy.and.callFake(() => Promise.reject('Test'));
bvGetPrIsTrustedSpy.and.rejectWith('Test');
await createRequest(+pr).expect(500, 'Test');
@ -614,7 +612,9 @@ describe('PreviewServerFactory', () => {
it('should call \'BuildCreator#updatePrVisibility()\' with the correct arguments', async () => {
bvGetPrIsTrustedSpy.and.callFake((pr2: number) => Promise.resolve(pr2 === 42));
bvGetPrIsTrustedSpy.
withArgs(24).and.resolveTo(false).
withArgs(42).and.resolveTo(true);
await createRequest(24);
expect(bcUpdatePrVisibilitySpy).toHaveBeenCalledWith(24, false);
@ -625,7 +625,7 @@ describe('PreviewServerFactory', () => {
it('should propagate errors from BuildCreator', async () => {
bcUpdatePrVisibilitySpy.and.callFake(() => Promise.reject('Test'));
bcUpdatePrVisibilitySpy.and.rejectWith('Test');
await createRequest(+pr).expect(500, 'Test');
});
@ -633,7 +633,9 @@ describe('PreviewServerFactory', () => {
describe('on success', () => {
it('should respond with 200 (action: undefined)', async () => {
bvGetPrIsTrustedSpy.and.returnValues(Promise.resolve(true), Promise.resolve(false));
bvGetPrIsTrustedSpy.
withArgs(2).and.resolveTo(false).
withArgs(4).and.resolveTo(true);
const reqs = [4, 2].map(num => createRequest(num).expect(200, http.STATUS_CODES[200]));
await Promise.all(reqs);
@ -641,7 +643,9 @@ describe('PreviewServerFactory', () => {
it('should respond with 200 (action: labeled)', async () => {
bvGetPrIsTrustedSpy.and.returnValues(Promise.resolve(true), Promise.resolve(false));
bvGetPrIsTrustedSpy.
withArgs(2).and.resolveTo(false).
withArgs(4).and.resolveTo(true);
const reqs = [4, 2].map(num => createRequest(num, 'labeled').expect(200, http.STATUS_CODES[200]));
await Promise.all(reqs);
@ -649,7 +653,9 @@ describe('PreviewServerFactory', () => {
it('should respond with 200 (action: unlabeled)', async () => {
bvGetPrIsTrustedSpy.and.returnValues(Promise.resolve(true), Promise.resolve(false));
bvGetPrIsTrustedSpy.
withArgs(2).and.resolveTo(false).
withArgs(4).and.resolveTo(true);
const reqs = [4, 2].map(num => createRequest(num, 'unlabeled').expect(200, http.STATUS_CODES[200]));
await Promise.all(reqs);