Revert "Merge pull request #87 from github/brrygrdn/test-docker-respo…

…nse"

This reverts commit 77c1a3c, reversing
changes made to 5f7342f.

__tests__/container-service.test.ts

@@ -7,36 +7,18 @@ describe('ContainerService', () => {
   const docker = new Docker()
   let container: any
 
-  describe('when a container runs successfully', () => {
-    beforeEach(async () => {
-      await ImageService.pull('alpine')
-      container = await docker.createContainer({
-        Image: 'alpine',
-        AttachStdout: true,
-        AttachStderr: true,
-        Cmd: ['/bin/sh', '-c', 'echo $VAR'],
-        Env: ['VAR=env-var']
-      })
-    })
-
-    test('it returns true', async () => {
-      expect(await ContainerService.run(container)).toBe(true)
+  beforeAll(async () => {
+    await ImageService.pull('alpine')
+    container = await docker.createContainer({
+      Image: 'alpine',
+      AttachStdout: true,
+      AttachStderr: true,
+      Cmd: ['/bin/sh', '-c', 'echo $VAR'],
+      Env: ['VAR=env-var']
     })
   })
 
-  describe('when a container runs unsuccessfully', () => {
-    beforeEach(async () => {
-      await ImageService.pull('alpine')
-      container = await docker.createContainer({
-        Image: 'alpine',
-        AttachStdout: true,
-        AttachStderr: true,
-        Cmd: ['/bin/sh', '-c', 'nosuchccommand']
-      })
-    })
-
-    test('raises an exception', async () => {
-      await expect(ContainerService.run(container)).rejects.toThrow()
-    })
+  test('runs containers', async () => {
+    await ContainerService.run(container)
   })
 })

__tests__/main.test.ts

@@ -139,9 +139,7 @@ describe('run', () => {
 
       expect(reportJobErrorSpy).toHaveBeenCalledWith({
         'error-type': 'actions_workflow_unknown',
-        'error-details': {
-          'action-error': 'error getting job details'
-        }
+        'error-detail': 'error getting job details'
       })
       expect(markJobAsProcessedSpy).toHaveBeenCalled()
     })
@@ -175,9 +173,7 @@ describe('run', () => {
 
       expect(reportJobErrorSpy).toHaveBeenCalledWith({
         'error-type': 'actions_workflow_unknown',
-        'error-details': {
-          'action-error': 'error getting credentials'
-        }
+        'error-detail': 'error getting credentials'
       })
       expect(markJobAsProcessedSpy).toHaveBeenCalled()
     })
@@ -211,9 +207,7 @@ describe('run', () => {
 
       expect(reportJobErrorSpy).toHaveBeenCalledWith({
         'error-type': 'actions_workflow_image',
-        'error-details': {
-          'action-error': 'error pulling an image'
-        }
+        'error-detail': 'error pulling an image'
       })
       expect(markJobAsProcessedSpy).toHaveBeenCalled()
     })
@@ -247,9 +241,7 @@ describe('run', () => {
 
       expect(reportJobErrorSpy).toHaveBeenCalledWith({
         'error-type': 'actions_workflow_updater',
-        'error-details': {
-          'action-error': 'error running the update'
-        }
+        'error-detail': 'error running the update'
       })
       expect(markJobAsProcessedSpy).toHaveBeenCalled()
     })

__tests__/updater.test.ts

@@ -1,22 +1,15 @@
+import {UPDATER_IMAGE_NAME, PROXY_IMAGE_NAME} from '../src/main'
 import {Updater} from '../src/updater'
-import Docker from 'dockerode'
-import {ContainerService} from '../src/container-service'
-import {ProxyBuilder} from '../src/proxy'
-
-// We do not need to build actual containers or run updates for this test.)
-jest.mock('dockerode')
-jest.mock('../src/container-service')
-jest.mock('../src/proxy')
 
 describe('Updater', () => {
   const mockApiClient: any = {
     getJobDetails: jest.fn(),
     getCredentials: jest.fn(),
     params: {
       jobId: 1,
-      jobToken: 'job-token',
-      credentialsToken: 'job-credentials-token',
-      dependabotApiUrl: 'http://localhost:3001'
+      jobToken: process.env.JOB_TOKEN,
+      credentialsToken: process.env.CREDENTIALS_TOKEN,
+      dependabotApiUrl: 'http://host.docker.internal:3001'
     }
   }
 
@@ -30,163 +23,18 @@ describe('Updater', () => {
     'package-manager': 'npm-and-yarn'
   }
 
-  const mockProxy: any = {
-    container: {
-      start: jest.fn()
-    },
-    network: jest.fn(),
-    networkName: 'mockNetworkName',
-    url: 'http://localhost',
-    cert: 'mockCertificate',
-    shutdown: jest.fn()
-  }
-
-  const mockContainer: any = {
-    id: 1
-  }
-
-  afterEach(async () => {
-    jest.clearAllMocks() // Reset any mocked classes
-  })
-
-  describe('when there is a happy path update', () => {
-    const updater = new Updater(
-      'MOCK_UPDATER_IMAGE_NAME',
-      'MOCK_PROXY_IMAGE_NAME',
-      mockApiClient,
-      mockJobDetails,
-      [],
-      '__fixtures__/output/happy_path'
-    )
-
-    beforeEach(async () => {
-      jest
-        .spyOn(Docker.prototype, 'createContainer')
-        .mockResolvedValue(mockContainer)
-
-      jest.spyOn(ProxyBuilder.prototype, 'run').mockResolvedValue(mockProxy)
-      jest.spyOn(ContainerService, 'run').mockImplementation(jest.fn())
-    })
-
-    it('should be successful', async () => {
-      expect(await updater.runUpdater()).toBe(true)
-    })
-  })
-
-  describe('when the file fetch container fails', () => {
-    const updater = new Updater(
-      'MOCK_UPDATER_IMAGE_NAME',
-      'MOCK_PROXY_IMAGE_NAME',
-      mockApiClient,
-      mockJobDetails,
-      [],
-      '__fixtures__/output/happy_path'
-    )
-
-    beforeEach(async () => {
-      jest
-        .spyOn(Docker.prototype, 'createContainer')
-        .mockResolvedValue(mockContainer)
-
-      jest.spyOn(ProxyBuilder.prototype, 'run').mockResolvedValue(mockProxy)
-
-      jest
-        .spyOn(ContainerService, 'run')
-        .mockImplementationOnce(
-          jest.fn(async () =>
-            Promise.reject(new Error('First call to container service errored'))
-          )
-        )
-    })
-
-    it('should raise an error', async () => {
-      await expect(updater.runUpdater()).rejects.toThrow()
-    })
-  })
-
-  describe('when file updater container fails', () => {
-    const updater = new Updater(
-      'MOCK_UPDATER_IMAGE_NAME',
-      'MOCK_PROXY_IMAGE_NAME',
-      mockApiClient,
-      mockJobDetails,
-      [],
-      '__fixtures__/output/happy_path'
-    )
-
-    beforeEach(async () => {
-      jest
-        .spyOn(Docker.prototype, 'createContainer')
-        .mockResolvedValue(mockContainer)
-
-      jest.spyOn(ProxyBuilder.prototype, 'run').mockResolvedValue(mockProxy)
-
-      jest
-        .spyOn(ContainerService, 'run')
-        .mockImplementationOnce(jest.fn())
-        .mockImplementationOnce(
-          jest.fn(async () =>
-            Promise.reject(
-              new Error('Second call to container service errored')
-            )
-          )
-        )
-    })
-
-    it('should raise an error', async () => {
-      await expect(updater.runUpdater()).rejects.toThrow()
-    })
-  })
-
-  describe('when the file fetch step results in empty output', () => {
-    const updater = new Updater(
-      'MOCK_UPDATER_IMAGE_NAME',
-      'MOCK_PROXY_IMAGE_NAME',
-      mockApiClient,
-      mockJobDetails,
-      [],
-      '__fixtures__/output/empty'
-    )
-
-    beforeEach(async () => {
-      jest
-        .spyOn(Docker.prototype, 'createContainer')
-        .mockResolvedValue(mockContainer)
-
-      jest.spyOn(ProxyBuilder.prototype, 'run').mockResolvedValue(mockProxy)
-
-      jest.spyOn(ContainerService, 'run').mockImplementation(jest.fn())
-    })
-
-    it('should raise an error', async () => {
-      await expect(updater.runUpdater()).rejects.toThrow(
-        new Error('No output.json created by the fetcher container')
-      )
-    })
-  })
-
-  describe('when the file fetch step results in malformed output', () => {
-    const updater = new Updater(
-      'MOCK_UPDATER_IMAGE_NAME',
-      'MOCK_PROXY_IMAGE_NAME',
-      mockApiClient,
-      mockJobDetails,
-      [],
-      '__fixtures__/output/malformed'
-    )
-
-    beforeEach(async () => {
-      jest
-        .spyOn(Docker.prototype, 'createContainer')
-        .mockResolvedValue(mockContainer)
-
-      jest.spyOn(ProxyBuilder.prototype, 'run').mockResolvedValue(mockProxy)
-
-      jest.spyOn(ContainerService, 'run').mockImplementation(jest.fn())
-    })
+  const updater = new Updater(
+    UPDATER_IMAGE_NAME,
+    PROXY_IMAGE_NAME,
+    mockApiClient,
+    mockJobDetails,
+    []
+  )
 
-    it('should raise an error', async () => {
-      await expect(updater.runUpdater()).rejects.toThrow()
+  it('should fetch job details', async () => {
+    mockApiClient.getJobDetails.mockImplementation(() => {
+      throw new Error('kaboom')
     })
+    updater.runUpdater()
   })
 })

src/api-client.ts

@@ -22,9 +22,7 @@ export type JobDetails = {
 
 export type JobError = {
   'error-type': string
-  'error-details': {
-    'action-error': string
-  }
+  'error-detail': any
 }
 
 export type Credential = {
@@ -41,12 +39,6 @@ export class ApiClient {
     readonly params: JobParameters
   ) {}
 
-  // We use a static unknown SHA when marking a job as complete from the action
-  // to remain in parity with the existing runner.
-  UnknownSha = {
-    'base-commit-sha': 'unknown'
-  }
-
   async getJobDetails(): Promise<JobDetails> {
     const detailsURL = `/update_jobs/${this.params.jobId}/details`
     const res = await this.client.get(detailsURL, {
@@ -73,29 +65,25 @@ export class ApiClient {
 
   async reportJobError(error: JobError): Promise<void> {
     const recordErrorURL = `/update_jobs/${this.params.jobId}/record_update_job_error`
-    const res = await this.client.post(
-      recordErrorURL,
-      {data: error},
-      {
-        headers: {Authorization: this.params.jobToken}
-      }
-    )
-    if (res.status !== 204) {
+    const res = await this.client.post(recordErrorURL, error, {
+      headers: {Authorization: this.params.jobToken}
+    })
+    if (res.status !== 200) {
       throw new Error(`Unexpected status code: ${res.status}`)
     }
+
+    return res.data.data.attributes
   }
 
   async markJobAsProcessed(): Promise<void> {
     const markAsProcessedURL = `/update_jobs/${this.params.jobId}/mark_as_processed`
-    const res = await this.client.patch(
-      markAsProcessedURL,
-      {data: this.UnknownSha},
-      {
-        headers: {Authorization: this.params.jobToken}
-      }
-    )
-    if (res.status !== 204) {
+    const res = await this.client.get(markAsProcessedURL, {
+      headers: {Authorization: this.params.credentialsToken}
+    })
+    if (res.status !== 200) {
       throw new Error(`Unexpected status code: ${res.status}`)
     }
+
+    return res.data.data.attributes
   }
 }

src/container-service.ts

@@ -4,8 +4,6 @@ import {pack} from 'tar-stream'
 import {FileFetcherInput, FileUpdaterInput, ProxyConfig} from './config-types'
 import {outStream, errStream} from './utils'
 
-class ContainerRuntimeError extends Error {}
-
 export const ContainerService = {
   async storeInput(
     name: string,
@@ -31,7 +29,7 @@ export const ContainerService = {
     await container.putArchive(tar, {path})
   },
 
-  async run(container: Container): Promise<boolean> {
+  async run(container: Container): Promise<void> {
     try {
       const stream = await container.attach({
         stream: true,
@@ -45,15 +43,7 @@ export const ContainerService = {
       )
 
       await container.start()
-      const outcome = await container.wait()
-
-      if (outcome.StatusCode === 0) {
-        return true
-      } else {
-        throw new ContainerRuntimeError(
-          `Failure running container ${container.id}`
-        )
-      }
+      await container.wait()
     } finally {
       await container.remove()
       core.info(`Cleaned up container ${container.id}`)