fix: Wait for asset discovery finish when awaiting on snapshot resour…

…ces in finalize (#423)

* test: Add failing test for snapshots being added after finalize

* fix: Wait for asset discovery finish when awaiting on snapshots

* test: Clean up test file imports

* fix integration tests

* Improve tests

* Code review fixes

src/services/agent-service.ts

@@ -115,6 +115,10 @@ export class AgentService {
       logger.info(`snapshot skipped[max_file_size_exceeded]: '${request.body.name}'`)
       return response.json({ success: true })
     }
+    // tslint:disable-next-line
+    let resolve, reject
+    const deferred = new Promise((...args) => [resolve, reject] = args)
+    this.snapshotCreationPromises.push(deferred)
 
     let resources = await this.snapshotService.buildResources(
       rootURL,
@@ -140,15 +144,14 @@ export class AgentService {
       this.snapshotService.buildLogResource(snapshotLog),
     )
 
-    const snapshotCreation = this.snapshotService.create(
+    this.snapshotService.create(
       request.body.name,
       resources,
       snapshotOptions,
       request.body.clientInfo,
       request.body.environmentInfo,
-    )
+    ).then(resolve).catch(reject)
 
-    this.snapshotCreationPromises.push(snapshotCreation)
     logger.info(`snapshot taken: '${request.body.name}'`)
 
     profile('agentService.handleSnapshot')

src/services/resource-service.ts

@@ -47,7 +47,7 @@ export default class ResourceService extends PercyClientService {
         snapshotResponse.response,
         snapshotResponse.resources,
       )
-      profile('-> resourceService.uploadMissingResources', {resources: resources.length})
+      profile(`-> resourceService.uploadMissingResources ${resources.length}`)
 
       return true
     } catch (error) {

src/services/snapshot-service.ts

@@ -110,7 +110,7 @@ export default class SnapshotService extends PercyClientService {
 
       profile('-> snapshotService.finalizeSnapshot')
       await this.finalize(response.body.data.id)
-      profile('-> snapshotService.finalizeSnapshot', {snapshotId})
+      profile(`-> snapshotService.finalizeSnapshot ${snapshotId}`)
       return response
     }).catch(logError)
 

test/acceptance/asset-discovery.test.js

@@ -0,0 +1,27 @@
+import expect from 'expect'
+import {
+  run,
+  setupApiProxy,
+  setupDummyApp
+} from './helpers'
+
+describe('Asset discovery', () => {
+  let proxy = setupApiProxy()
+  let dummy = setupDummyApp()
+
+  it('waits for snapshot creation before build finalization', async () => {
+    let [stdout] = await run('percy exec -- node ./test/acceptance/dummy/snapshot-error.js');
+    let finalizeReqDate = proxy.requests['/builds/123/finalize'][0].timestamp;
+    let snapshotReqs = proxy.requests['/builds/123/snapshots'];
+    let lastSnapshotReqDate = snapshotReqs[snapshotReqs.length - 1].timestamp;
+
+    expect(finalizeReqDate).toBeGreaterThan(lastSnapshotReqDate)
+    expect(stdout).toHaveEntries([
+      '[percy] percy has started.',
+      /^\[percy\] waiting for \d+ snapshots to complete\.\.\.$/m,
+      "[percy] snapshot taken: 'Home Page - 0'",
+      '[percy] done.',
+      '[percy] finalized build #4: <<build-url>>'
+    ])
+  })
+})

test/acceptance/dummy/snapshot-error.js

@@ -0,0 +1,17 @@
+// register babel for the snapshot helper
+require('@babel/register')
+require('regenerator-runtime/runtime')
+
+const launch = require('../helpers/snapshot').default
+
+launch(async (page, snapshot) => {
+  await page.goto('http://localhost:9999')
+
+  setTimeout(() => {
+    throw new Error('Surprise!')
+  }, 100) // magic number
+
+  for (let index = 0; index < 10; index++) {
+    await snapshot(`Home Page - ${index}`)
+  }
+})

test/acceptance/helpers/proxy.js

@@ -50,7 +50,7 @@ export function createApiProxy() {
   app.all('*', (req, res, next) => {
     let path = req.path.replace(/\/$/, '')
     requests[path] = requests[path] || []
-    requests[path].push(req)
+    requests[path].push({ timestamp: Date.now(), ...req })
     next()
   })