From b1b6d7ae382eab41baa682f4504fcbed40500f63 Mon Sep 17 00:00:00 2001
From: Mark Stacey <markjstacey@gmail.com>
Date: Thu, 3 Dec 2020 14:30:50 -0330
Subject: [PATCH] Fix intermittent metrics e2e test failure (#9980)

The metrics e2e test would fail if the segment events still weren't
dispatched when the page loaded. The Segment events are sent on a set
interval, so it isn't abnormal for them to lag behind the page load
itself. The `waitUntilCalled` utility has been used to wait until all
required events have been dispatched.

The `wait-until-called` module was converted to an ES5 module, so that
it could be used from an e2e test. The optional `callCount` parameter
has also been added, to allow waiting for more than one call.

The `segmentSpy` had to be converted to a `segmentStub`, to allow the
`waitUntilCalled` utility to be used.
---
 .eslintrc.js                  |  1 +
 test/e2e/helpers.js           |  8 ++++----
 test/e2e/metrics.spec.js      | 16 +++++++++-------
 test/lib/wait-until-called.js | 26 +++++++++++++++++++-------
 4 files changed, 33 insertions(+), 18 deletions(-)

diff --git a/.eslintrc.js b/.eslintrc.js
index 35541a9ba..184f6ce89 100644
--- a/.eslintrc.js
+++ b/.eslintrc.js
@@ -197,6 +197,7 @@ module.exports = {
         'stylelint.config.js',
         'development/**/*.js',
         'test/e2e/**/*.js',
+        'test/lib/wait-until-called.js',
         'test/env.js',
         'test/setup.js',
       ],
diff --git a/test/e2e/helpers.js b/test/e2e/helpers.js
index de5986546..926f67177 100644
--- a/test/e2e/helpers.js
+++ b/test/e2e/helpers.js
@@ -27,7 +27,7 @@ async function withFixtures(options, testSuite) {
   const ganacheServer = new Ganache()
   let dappServer
   let segmentServer
-  let segmentSpy
+  let segmentStub
 
   let webDriver
   try {
@@ -52,10 +52,10 @@ async function withFixtures(options, testSuite) {
       })
     }
     if (mockSegment) {
-      segmentSpy = sinon.spy()
+      segmentStub = sinon.stub()
       segmentServer = createSegmentServer((_request, response, events) => {
         for (const event of events) {
-          segmentSpy(event)
+          segmentStub(event)
         }
         response.statusCode = 200
         response.end()
@@ -67,7 +67,7 @@ async function withFixtures(options, testSuite) {
 
     await testSuite({
       driver,
-      segmentSpy,
+      segmentStub,
     })
 
     if (process.env.SELENIUM_BROWSER === 'chrome') {
diff --git a/test/e2e/metrics.spec.js b/test/e2e/metrics.spec.js
index 9c32df14b..585a44e35 100644
--- a/test/e2e/metrics.spec.js
+++ b/test/e2e/metrics.spec.js
@@ -1,5 +1,6 @@
 const { strict: assert } = require('assert')
 const { By, Key } = require('selenium-webdriver')
+const waitUntilCalled = require('../lib/wait-until-called')
 const { withFixtures } = require('./helpers')
 
 /**
@@ -26,26 +27,27 @@ describe('Segment metrics', function () {
         title: this.test.title,
         mockSegment: true,
       },
-      async ({ driver, segmentSpy }) => {
+      async ({ driver, segmentStub }) => {
+        const threeSegmentEventsReceived = waitUntilCalled(segmentStub, null, 3)
         await driver.navigate()
+
         const passwordField = await driver.findElement(By.css('#password'))
         await passwordField.sendKeys('correct horse battery staple')
         await passwordField.sendKeys(Key.ENTER)
 
-        // find arbitary element to ensure Home page has loaded
-        await driver.findElement(By.css('[data-testid="eth-overview-send"]'))
+        await threeSegmentEventsReceived
 
-        assert.ok(segmentSpy.called, 'Segment should receive metrics')
+        assert.ok(segmentStub.called, 'Segment should receive metrics')
 
-        const firstSegmentEvent = segmentSpy.getCall(0).args[0]
+        const firstSegmentEvent = segmentStub.getCall(0).args[0]
         assert.equal(firstSegmentEvent.name, 'Home')
         assert.equal(firstSegmentEvent.context.page.path, '/')
 
-        const secondSegmentEvent = segmentSpy.getCall(1).args[0]
+        const secondSegmentEvent = segmentStub.getCall(1).args[0]
         assert.equal(secondSegmentEvent.name, 'Unlock Page')
         assert.equal(secondSegmentEvent.context.page.path, '/unlock')
 
-        const thirdSegmentEvent = segmentSpy.getCall(2).args[0]
+        const thirdSegmentEvent = segmentStub.getCall(2).args[0]
         assert.equal(thirdSegmentEvent.name, 'Home')
         assert.equal(thirdSegmentEvent.context.page.path, '/')
       },
diff --git a/test/lib/wait-until-called.js b/test/lib/wait-until-called.js
index 7a2eb2704..752ee7675 100644
--- a/test/lib/wait-until-called.js
+++ b/test/lib/wait-until-called.js
@@ -9,18 +9,30 @@
  *
  * @param {import('sinon').stub} stub - A sinon stub of a function
  * @param {unknown} [wrappedThis] - The object the stubbed function was called on, if any (i.e. the `this` value)
+ * @param {number} [callCount] - The number of calls to wait for. Defaults to 1.
  * @returns {Promise} A Promise that resolves when the stub has been called
  */
-export default function waitUntilCalled(stub, wrappedThis = null) {
-  let wasCalled
-  const stubHasBeenCalled = new Promise((resolve) => {
-    wasCalled = resolve
+function waitUntilCalled(stub, wrappedThis = null, callCount = 1) {
+  let numCalls = 0
+  let resolve
+  const stubHasBeenCalled = new Promise((_resolve) => {
+    resolve = _resolve
   })
   stub.callsFake((...args) => {
-    if (stub.wrappedMethod) {
-      stub.wrappedMethod.call(wrappedThis, ...args)
+    try {
+      if (stub.wrappedMethod) {
+        stub.wrappedMethod.call(wrappedThis, ...args)
+      }
+    } finally {
+      if (numCalls < callCount) {
+        numCalls += 1
+        if (numCalls === callCount) {
+          resolve()
+        }
+      }
     }
-    wasCalled()
   })
   return stubHasBeenCalled
 }
+
+module.exports = waitUntilCalled