Updated onStatusUpdate to consider land requests which are running. (#212)

Co-authored-by: Grace <[email protected]>

Author: GraceDmello

src/lib/Runner.ts

@@ -360,52 +360,46 @@ export class Runner {
 
   // onStatusUpdate only updates status' for landrequests, never moves a state and always exits early
   onStatusUpdate = async (statusEvent: BB.BuildStatusEvent) => {
-    const running = await this.getRunning();
-    if (!running.length) {
-      Logger.info('No builds running, status event is irrelevant', {
+    const requests = await this.getRunning();
+    const landRequestStatus = requests.find(
+      ({ state, request }) => state === 'running' && request.buildId === statusEvent.buildId,
+    );
+
+    const landRequest = landRequestStatus?.request;
+
+    if (!landRequest) {
+      Logger.info('No running build for status event, ignoring', {
         namespace: 'lib:runner:onStatusUpdate',
         statusEvent,
       });
       return;
     }
-    for (const landRequestStatus of running) {
-      const landRequest = landRequestStatus.request;
-      if (statusEvent.buildId !== landRequest.buildId) continue; // check next landRequest
-      switch (statusEvent.buildStatus) {
-        case 'SUCCESSFUL':
-          Logger.info('Moving landRequest to awaiting-merge state', {
-            namespace: 'lib:runner:onStatusUpdate',
-            landRequestId: landRequest.id,
-            pullRequestId: landRequest.pullRequestId,
-            landRequestStatus,
-          });
-          await landRequest.setStatus('awaiting-merge');
-          return this.next();
-        case 'FAILED':
-          Logger.info('Moving landRequest to failed state', {
-            namespace: 'lib:runner:onStatusUpdate',
-            landRequestId: landRequest.id,
-            pullRequestId: landRequest.pullRequestId,
-            landRequestStatus,
-          });
-          await landRequest.setStatus('fail', 'Landkid build failed');
-          return this.next();
-        case 'STOPPED':
-          Logger.info('Moving landRequest to aborted state', {
-            namespace: 'lib:runner:onStatusUpdate',
-            landRequestId: landRequest.id,
-            pullRequestId: landRequest.pullRequestId,
-            landRequestStatus,
-          });
-          await landRequest.setStatus('aborted', 'Landkid pipelines build was stopped');
-          return this.next();
-        default:
-          Logger.info('Dont know what to do with build status, ignoring', {
-            namespace: 'lib:runner:onStatusUpdate',
-            statusEvent,
-          });
-          break;
-      }
+
+    const logMessage = (message: string) =>
+      Logger.info(message, {
+        namespace: 'lib:runner:onStatusUpdate',
+        landRequestId: landRequest.id,
+        pullRequestId: landRequest.pullRequestId,
+        landRequestStatus,
+        statusEvent,
+      });
+
+    switch (statusEvent.buildStatus) {
+      case 'SUCCESSFUL':
+        logMessage('Moving landRequest to awaiting-merge state');
+        await landRequest.setStatus('awaiting-merge');
+        return this.next();
+      case 'FAILED':
+        logMessage('Moving landRequest to failed state');
+        await landRequest.setStatus('fail', 'Landkid build failed');
+        return this.next();
+      case 'STOPPED':
+        logMessage('Moving landRequest to aborted state');
+        await landRequest.setStatus('aborted', 'Landkid pipelines build was stopped');
+        return this.next();
+      default:
+        logMessage('Dont know what to do with build status, ignoring');
+        break;
     }
   };
 

src/lib/__tests__/Runner.test.ts

@@ -643,6 +643,7 @@ describe('Runner', () => {
   describe('onStatusUpdate', () => {
     let request: LandRequest;
     let nextSpy: jest.SpyInstance<any>;
+    let runningStatuses: LandRequestStatus[];
     beforeEach(() => {
       const pullRequest = new PullRequest({
         prId: mockPullRequest.pullRequestId,
@@ -659,15 +660,17 @@ describe('Runner', () => {
         pullRequestId: 1,
         pullRequest,
       });
-      const runningStatus = new LandRequestStatus({
-        date: new Date(120),
-        id: '0',
-        isLatest: true,
-        request,
-        requestId: '0',
-        state: 'running',
-      });
-      mockQueue.getRunning = jest.fn(async () => [runningStatus]);
+      runningStatuses = [
+        new LandRequestStatus({
+          date: new Date(120),
+          id: '0',
+          isLatest: true,
+          request,
+          requestId: '0',
+          state: 'running',
+        }),
+      ];
+      mockQueue.getRunning = jest.fn(async () => runningStatuses);
       nextSpy = jest.spyOn(runner, 'next').mockImplementation(async () => {});
     });
 
@@ -728,6 +731,33 @@ describe('Runner', () => {
       expect(request.setStatus).not.toHaveBeenCalled();
       expect(nextSpy).not.toHaveBeenCalled();
     });
+
+    it('should not set status of request with matching build ID and currently in awaiting-merge state', async () => {
+      runningStatuses = [
+        new LandRequestStatus({
+          date: new Date(120),
+          id: '0',
+          isLatest: true,
+          request: new LandRequest({
+            buildId: 1234,
+            created: new Date(120),
+            forCommit: 'abc',
+            id: '0',
+            triggererAaid: '123',
+            pullRequestId: 1,
+          }),
+          requestId: '0',
+          state: 'awaiting-merge',
+        }),
+      ];
+      expect(request.setStatus).not.toHaveBeenCalled();
+      await runner.onStatusUpdate({
+        buildId: 1234,
+        buildStatus: 'SUCCESSFUL',
+      });
+      expect(request.setStatus).not.toHaveBeenCalled();
+      expect(nextSpy).not.toHaveBeenCalled();
+    });
   });
 
   describe('getState', () => {