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

gkalpak · gkalpak · commit 936bab200cd1 · 2022-05-11T01:35:45.000+03:00
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).
diff --git a/aio/aio-builds-setup/dockerbuild/scripts-js/lib/common/circle-ci-api.ts b/aio/aio-builds-setup/dockerbuild/scripts-js/lib/common/circle-ci-api.ts
@@ -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;
diff --git a/aio/aio-builds-setup/dockerbuild/scripts-js/lib/preview-server/build-retriever.ts b/aio/aio-builds-setup/dockerbuild/scripts-js/lib/preview-server/build-retriever.ts
@@ -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;
   }
diff --git a/aio/aio-builds-setup/dockerbuild/scripts-js/lib/preview-server/preview-server-factory.ts b/aio/aio-builds-setup/dockerbuild/scripts-js/lib/preview-server/preview-server-factory.ts
@@ -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}".`);
diff --git a/aio/aio-builds-setup/dockerbuild/scripts-js/lib/verify-setup/constants.ts b/aio/aio-builds-setup/dockerbuild/scripts-js/lib/verify-setup/constants.ts
@@ -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,
diff --git a/aio/aio-builds-setup/dockerbuild/scripts-js/lib/verify-setup/mock-external-apis.ts b/aio/aio-builds-setup/dockerbuild/scripts-js/lib/verify-setup/mock-external-apis.ts
@@ -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' } });
 
diff --git a/aio/aio-builds-setup/dockerbuild/scripts-js/lib/verify-setup/preview-server.e2e.ts b/aio/aio-builds-setup/dockerbuild/scripts-js/lib/verify-setup/preview-server.e2e.ts
@@ -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));
     });
diff --git a/aio/aio-builds-setup/dockerbuild/scripts-js/test/preview-server/build-retriever.spec.ts b/aio/aio-builds-setup/dockerbuild/scripts-js/test/preview-server/build-retriever.spec.ts
@@ -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'});
     });
   });
 
diff --git a/aio/aio-builds-setup/dockerbuild/scripts-js/test/preview-server/preview-server-factory.spec.ts b/aio/aio-builds-setup/dockerbuild/scripts-js/test/preview-server/preview-server-factory.spec.ts
@@ -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

Original file line number	Diff line number	Diff line change
`@@ -13,7 +13,6 @@ export interface GithubInfo {`
`13`	`13`	`pr: number;`
`14`	`14`	`repo: string;`
`15`	`15`	`sha: string;`
`16`		`- success: boolean;`
`17`	`16`	`}`
`18`	`17`
`19`	`18`	`/**`
`@@ -42,7 +41,6 @@ export class BuildRetriever {`
`42`	`41`	`pr: +pipelineInfo.vcs.review_id,`
`43`	`42`	`repo: buildInfo.project.name,`
`44`	`43`	`sha: pipelineInfo.vcs.revision,`
`45`		`- success: buildInfo.status === 'success',`
`46`	`44`	`};`
`47`	`45`	`return githubInfo;`
`48`	`46`	`}`