fix(docs-infra): do not detect a running job as failed (#45948)

Previously, the preview server would incorrectly identify a running
`aio_preview` CI job as failed and therefore skip creating a preview.
This happened because it only checked whether the job's status is
`success`, failing to account for the fact that the job would have a
`running` status.
(This bug was accidentally introduced in #45934.)

This commit avoids the problem by getting rid of the job status check
altogether. This check does not offer any benefit, since the CI job will
always be in a `running` state (i.e. neither successfully completed nor
failed).

PR Close #45948
This commit is contained in:
George Kalpakas
2022-05-11 01:04:50 +03:00
committed by Jessica Janiuk
parent 6375fa7987
commit aea7b98a56
8 changed files with 2 additions and 31 deletions
@@ -33,9 +33,6 @@ export interface BuildInfo {
/** The job name (e.g. `'aio_preview'`). */
name: string;
/** The job status (e.g. `'success'` or `'failed'`). */
status: string;
/** Info about the organization which the project related to this job belongs to. */
organization: {
name: string;
@@ -13,7 +13,6 @@ export interface GithubInfo {
pr: number;
repo: string;
sha: string;
success: boolean;
}
/**
@@ -42,7 +41,6 @@ export class BuildRetriever {
pr: +pipelineInfo.vcs.review_id,
repo: buildInfo.project.name,
sha: pipelineInfo.vcs.revision,
success: buildInfo.status === 'success',
};
return githubInfo;
}
@@ -120,13 +120,7 @@ export class PreviewServerFactory {
return;
}
const { pr, sha, org, repo, success } = await buildRetriever.getGithubInfo(buildNum);
if (!success) {
res.sendStatus(204);
logger.log(`PR:${pr}, Build:${buildNum} - Skipping preview processing because this build did not succeed.`);
return;
}
const { pr, sha, org, repo } = await buildRetriever.getGithubInfo(buildNum);
assert(cfg.githubOrg === org,
`Invalid webhook: expected "githubOrg" property to equal "${cfg.githubOrg}" but got "${org}".`);
@@ -1,7 +1,6 @@
export const enum BuildNums {
BUILD_INFO_ERROR = 1,
BUILD_INFO_404,
BUILD_INFO_BUILD_FAILED,
BUILD_INFO_INVALID_GH_ORG,
BUILD_INFO_INVALID_GH_REPO,
PIPELINE_INFO_ERROR,
@@ -31,7 +31,6 @@ const UNTRUSTED_USER = 'untrusted-user';
const BASIC_BUILD_INFO = {
name: 'test_job',
status: 'success',
organization: {name: AIO_GITHUB_ORGANIZATION},
project: {name: AIO_GITHUB_REPO},
pipeline: {id: PipelineIds.PIPELINE_INFO_OK},
@@ -114,7 +113,6 @@ circleCiApi.get(pipelineInfoUrl(PipelineIds.PIPELINE_INFO_OK)).reply(200, BASIC_
// BUILD INFO errors
circleCiApi.get(buildInfoUrl(BuildNums.BUILD_INFO_ERROR)).replyWithError('BUILD_INFO_ERROR');
circleCiApi.get(buildInfoUrl(BuildNums.BUILD_INFO_404)).reply(404, 'BUILD_INFO_404');
circleCiApi.get(buildInfoUrl(BuildNums.BUILD_INFO_BUILD_FAILED)).reply(200, { ...BASIC_BUILD_INFO, status: 'failed' });
circleCiApi.get(buildInfoUrl(BuildNums.BUILD_INFO_INVALID_GH_ORG)).reply(200, { ...BASIC_BUILD_INFO, organization: { name: 'bad' } });
circleCiApi.get(buildInfoUrl(BuildNums.BUILD_INFO_INVALID_GH_REPO)).reply(200, { ...BASIC_BUILD_INFO, project: { name: 'bad' } });
@@ -152,10 +152,6 @@ describe('preview-server', () => {
await curl(payload(BuildNums.PIPELINE_INFO_404)).then(h.verifyResponse(500));
});
it('should respond with 204 if the build on CircleCI failed', async () => {
await curl(payload(BuildNums.BUILD_INFO_BUILD_FAILED)).then(h.verifyResponse(204));
});
it('should respond with 500 if the github org from CircleCI does not match what is configured', async () => {
await curl(payload(BuildNums.BUILD_INFO_INVALID_GH_ORG)).then(h.verifyResponse(500));
});
@@ -24,7 +24,6 @@ describe('BuildRetriever', () => {
BUILD_INFO = {
number: 12345,
name: 'test_job',
status: 'success',
organization: {name: 'ORG'},
project: {name: 'REPO'},
pipeline: {id: 'test_pipeline'},
@@ -77,7 +76,7 @@ describe('BuildRetriever', () => {
const info = await retriever.getGithubInfo(12345);
expect(api.getBuildInfo).toHaveBeenCalledWith(12345);
expect(api.getPipelineInfo).toHaveBeenCalledWith('test_pipeline');
expect(info).toEqual({org: 'ORG', pr: 777, repo: 'REPO', sha: 'COMMIT', success: true});
expect(info).toEqual({org: 'ORG', pr: 777, repo: 'REPO', sha: 'COMMIT'});
});
});
@@ -410,7 +410,6 @@ describe('PreviewServerFactory', () => {
pr: PR,
repo: defaultConfig.githubRepo,
sha: SHA,
success: true,
};
BASIC_PAYLOAD = { payload: { build_num: BUILD_NUM, build_parameters: { CIRCLE_JOB: 'aio_preview' } } };
AFFECTS_SIGNIFICANT_FILES = true;
@@ -481,15 +480,6 @@ describe('PreviewServerFactory', () => {
await agent.post(URL).send(BASIC_PAYLOAD).expect(202);
});
it('should not create a preview if the build was not successful', async () => {
BUILD_INFO.success = false;
await agent.post(URL).send(BASIC_PAYLOAD).expect(204);
expect(getGithubInfoSpy).toHaveBeenCalledWith(BUILD_NUM);
expect(downloadBuildArtifactSpy).not.toHaveBeenCalled();
expect(getPrIsTrustedSpy).not.toHaveBeenCalled();
expect(createBuildSpy).not.toHaveBeenCalled();
});
it('should fail if the CircleCI request fails', async () => {
// Note it is important to put the `reject` into `and.callFake`;
// If you just `and.returnValue` the rejected promise