Merge pull request #3427 from overleaf/jpa-rewite-smoke-tests

[SmokeTests] rewrite GitOrigin-RevId: eda39db6b339d997f5669cb9bfca2aefe7d96699
2020-12-10 03:04:18 +00:00
parent e8e2264d7d
commit cb9d207ba0
16 changed files with 470 additions and 314 deletions
@@ -19,11 +19,20 @@ describe('HealthCheckController', function() {
    })

    async function performSmokeTestRequest() {
+      const start = Date.now()
      const { response, body } = await user.doRequest('GET', {
        url: '/health_check/full',
        json: true
      })
+      const end = Date.now()
+
      expect(body).to.exist
+      expect(body.stats).to.exist
+      expect(Date.parse(body.stats.start)).to.be.within(start, start + 1000)
+      expect(Date.parse(body.stats.end)).to.be.within(end - 1000, end)
+
+      expect(body.stats.duration).to.be.within(0, 10000)
+      expect(body.stats.steps).to.be.instanceof(Array)
      return { response, body }
    }

@@ -36,13 +45,30 @@ describe('HealthCheckController', function() {
      })
    })

+    describe('when the request is aborted', function() {
+      it('should not crash', async function() {
+        try {
+          await user.doRequest('GET', {
+            timeout: 1,
+            url: '/health_check/full',
+            json: true
+          })
+        } catch (err) {
+          expect(err.code).to.equal('ESOCKETTIMEDOUT')
+          return
+        }
+        expect.fail('expected request to fail with timeout error')
+      })
+    })
+
    describe('when the project does not exist', function() {
      beforeEach(function() {
        Settings.smokeTest.projectId = '404'
      })
      it('should respond with a 500 ', async function() {
-        const { response } = await performSmokeTestRequest()
+        const { response, body } = await performSmokeTestRequest()

+        expect(body.error).to.equal('run.101_loadEditor failed')
        expect(response.statusCode).to.equal(500)
      })
    })
@@ -52,8 +78,9 @@ describe('HealthCheckController', function() {
        Settings.smokeTest.password = 'foo-bar'
      })
      it('should respond with a 500 with mismatching password', async function() {
-        const { response } = await performSmokeTestRequest()
+        const { response, body } = await performSmokeTestRequest()

+        expect(body.error).to.equal('run.002_login failed')
        expect(response.statusCode).to.equal(500)
      })
    })
@@ -0,0 +1,39 @@
+# SmokeTests
+
+For the SmokeTests we implemented a Mini-Framework that is tailored for our
+ tooling, specifically OError, and does not need a large runner, such as mocha.
+
+The SmokeTests are separated into individual `steps`.
+Each `step` can have a `run` function and a `cleanup` function.
+The former will run in sequence with the other steps, the later in reverse
+ order from the finish, or the last failure.
+
+```js
+async function run(ctx) {
+  // do something
+}
+async function cleanup(ctx) {
+  // cleanup something
+}
+module.exports = { cleanup, run }
+```
+
+Steps will get called with a context object with common helpers and details:
+- `request` a promisified `request` module with defaults for `baseUrl`,
+   `timeout` and internals for cookie handling.
+- `assertHasStatusCode` a helper for asserting response status codes, pass
+   a response and desired status code. It will throw with OError context set.
+- `getCsrfTokenFor` a helper for retrieving CSRF tokens, pass an endpoint.
+- `processWithTimeout` a helper for awaiting Promises with a timeout, pass
+   `{ work: Promise.resolve(), timeout: 42, message: 'foo timedout' }`
+- `stats` an object for performance tracking.
+- `timeout` the step timeout
+
+Steps should handle timeouts locally to ensure appropriate cleanup of timed out
+ actions.
+
+Steps may pass values along to the next steps in returning an object with the
+ desired fields from the `run` or `cleanup` function.
+The returned values will overwrite existing details in the `ctx`.
+
+Alpha-numeric sorting of step filenames determines the processing sequence.
@@ -1,215 +1,95 @@
-/* eslint-disable
-    max-len,
-    no-unused-vars,
-    no-useless-escape,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
-const child = require('child_process')
-let fs = require('fs')
-const assert = require('assert')
-const chai = require('chai')
-if (Object.prototype.should == null) {
-  chai.should()
-}
-const { expect } = chai
+const fs = require('fs')
+const Path = require('path')
+
 const Settings = require('settings-sharelatex')
-let ownPort = Settings.internal.web.port || Settings.port || 3000
-const port = (Settings.web && Settings.web.web_router_port) || ownPort // send requests to web router if this is the api process
-const cookeFilePath = `/tmp/smoke-test-cookie-${ownPort}-to-${port}.txt`
-const buildUrl = path =>
-  ` -b ${cookeFilePath} --resolve 'smoke${
-    Settings.cookieDomain
-  }:${port}:127.0.0.1' http://smoke${Settings.cookieDomain}:${port}/${path}`
-const logger = require('logger-sharelatex')
-const LoginRateLimiter = require('../../../app/src/Features/Security/LoginRateLimiter.js')
-const RateLimiter = require('../../../app/src/infrastructure/RateLimiter.js')
+const { getCsrfTokenForFactory } = require('./support/Csrf')
+const { SmokeTestFailure } = require('./support/Errors')
+const {
+  requestFactory,
+  assertHasStatusCode
+} = require('./support/requestHelper')
+const { processWithTimeout } = require('./support/timeoutHelper')

-// Change cookie to be non secure so curl will send it
-const convertCookieFile = function(callback) {
-  fs = require('fs')
-  return fs.readFile(cookeFilePath, 'utf8', (err, data) => {
-    if (err) {
-      return callback(err)
-    }
-    const firstTrue = data.indexOf('TRUE')
-    const secondTrue = data.indexOf('TRUE', firstTrue + 4)
-    const result =
-      data.slice(0, secondTrue) + 'FALSE' + data.slice(secondTrue + 4)
-    return fs.writeFile(cookeFilePath, result, 'utf8', err => {
-      if (err) {
-        return callback(err)
-      }
-      return callback()
-    })
+const STEP_TIMEOUT = Settings.smokeTest.stepTimeout
+
+const PATH_STEPS = Path.join(__dirname, './steps')
+const STEPS = fs
+  .readdirSync(PATH_STEPS)
+  .sort()
+  .map(name => {
+    const step = require(Path.join(PATH_STEPS, name))
+    step.name = Path.basename(name, '.js')
+    return step
  })
+
+async function runSmokeTests({ isAborted, stats }) {
+  let lastStep = stats.start
+  function completeStep(key) {
+    const step = Date.now()
+    stats.steps.push({ [key]: step - lastStep })
+    lastStep = step
+  }
+
+  const request = requestFactory({ timeout: STEP_TIMEOUT })
+  const getCsrfTokenFor = getCsrfTokenForFactory({ request })
+  const ctx = {
+    assertHasStatusCode,
+    getCsrfTokenFor,
+    processWithTimeout,
+    request,
+    stats,
+    timeout: STEP_TIMEOUT
+  }
+  const cleanupSteps = []
+
+  async function runAndTrack(id, fn) {
+    let result
+    try {
+      result = await fn(ctx)
+    } catch (e) {
+      throw new SmokeTestFailure(`${id} failed`, {}, e)
+    } finally {
+      completeStep(id)
+    }
+    Object.assign(ctx, result)
+  }
+
+  completeStep('init')
+
+  let err
+  try {
+    for (const step of STEPS) {
+      if (isAborted()) break
+
+      const { name, run, cleanup } = step
+      if (cleanup) cleanupSteps.unshift({ name, cleanup })
+
+      await runAndTrack(`run.${name}`, run)
+    }
+  } catch (e) {
+    err = e
+  }
+
+  const cleanupErrors = []
+  for (const step of cleanupSteps) {
+    const { name, cleanup } = step
+
+    try {
+      await runAndTrack(`cleanup.${name}`, cleanup)
+    } catch (e) {
+      // keep going with cleanup
+      cleanupErrors.push(e)
+    }
+  }
+
+  if (err) throw err
+  if (cleanupErrors.length) {
+    if (cleanupErrors.length === 1) throw cleanupErrors[0]
+    throw new SmokeTestFailure('multiple cleanup steps failed', {
+      stats,
+      cleanupErrors
+    })
+  }
 }

-describe('Opening', function() {
-  before(function(done) {
-    logger.log('smoke test: setup')
-    LoginRateLimiter.recordSuccessfulLogin(Settings.smokeTest.user, err => {
-      if (err != null) {
-        logger.err({ err }, 'smoke test: error recoring successful login')
-        return done(err)
-      }
-      return RateLimiter.clearRateLimit(
-        'open-project',
-        `${Settings.smokeTest.projectId}:${Settings.smokeTest.userId}`,
-        err => {
-          if (err != null) {
-            logger.err(
-              { err },
-              'smoke test: error clearing open-project rate limit'
-            )
-            return done(err)
-          }
-          return RateLimiter.clearRateLimit(
-            'overleaf-login',
-            Settings.smokeTest.rateLimitSubject,
-            err => {
-              if (err != null) {
-                logger.err(
-                  { err },
-                  'smoke test: error clearing overleaf-login rate limit'
-                )
-                return done(err)
-              }
-              return done()
-            }
-          )
-        }
-      )
-    })
-  })
-
-  before(function(done) {
-    logger.log('smoke test: hitting dev/csrf')
-    let command = `\
-curl -H  "X-Forwarded-Proto: https" -c ${cookeFilePath} ${buildUrl('dev/csrf')}\
-`
-    child.exec(command, (err, stdout, stderr) => {
-      if (err != null) {
-        done(err)
-      }
-      const csrf = stdout
-      logger.log('smoke test: converting cookie file 1')
-      return convertCookieFile(err => {
-        if (err != null) {
-          return done(err)
-        }
-        logger.log('smoke test: hitting /login with csrf')
-        command = `\
-curl -c ${cookeFilePath} -H "Content-Type: application/json" -H "X-Forwarded-Proto: https" -d '{"_csrf":"${csrf}", "email":"${
-          Settings.smokeTest.user
-        }", "password":"${Settings.smokeTest.password}"}' ${buildUrl('login')}\
-`
-        return child.exec(command, err => {
-          if (err != null) {
-            return done(err)
-          }
-          logger.log('smoke test: finishing setup')
-          return convertCookieFile(done)
-        })
-      })
-    })
-  })
-
-  after(function(done) {
-    logger.log('smoke test: converting cookie file 2')
-    convertCookieFile(err => {
-      if (err != null) {
-        return done(err)
-      }
-      logger.log('smoke test: cleaning up')
-      let command = `\
-curl -H  "X-Forwarded-Proto: https" -c ${cookeFilePath} ${buildUrl('dev/csrf')}\
-`
-      return child.exec(command, (err, stdout, stderr) => {
-        if (err != null) {
-          done(err)
-        }
-        const csrf = stdout
-        logger.log('smoke test: converting cookie file 3')
-        return convertCookieFile(err => {
-          if (err != null) {
-            return done(err)
-          }
-          command = `\
-curl -H "Content-Type: application/json" -H "X-Forwarded-Proto: https" -d '{"_csrf":"${csrf}"}' -c ${cookeFilePath} ${buildUrl(
-            'logout'
-          )}\
-`
-          return child.exec(command, (err, stdout, stderr) => {
-            if (err != null) {
-              return done(err)
-            }
-            return fs.unlink(cookeFilePath, done)
-          })
-        })
-      })
-    })
-  })
-
-  it('a project', function(done) {
-    logger.log('smoke test: Checking can load a project')
-    this.timeout(4000)
-    const command = `\
-curl -H "X-Forwarded-Proto: https" -v ${buildUrl(
-      `project/${Settings.smokeTest.projectId}`
-    )}\
-`
-    return child.exec(command, (error, stdout, stderr) => {
-      expect(error, 'smoke test: error in getting project').to.not.exist
-
-      const statusCodeMatch = !!stderr.match('200 OK')
-      expect(
-        statusCodeMatch,
-        'smoke test: response code is not 200 getting project'
-      ).to.equal(true)
-
-      // Check that the project id is present in the javascript that loads up the project
-      const match = !!stdout.match(
-        `window.project_id = \"${Settings.smokeTest.projectId}\"`
-      )
-      expect(
-        match,
-        'smoke test: project page html does not have project_id'
-      ).to.equal(true)
-      return done()
-    })
-  })
-
-  it('the project list', function(done) {
-    logger.log('smoke test: Checking can load project list')
-    this.timeout(4000)
-    const command = `\
-curl -H "X-Forwarded-Proto: https" -v ${buildUrl('project')}\
-`
-    return child.exec(command, (error, stdout, stderr) => {
-      expect(error, 'smoke test: error returned in getting project list').to.not
-        .exist
-      expect(
-        !!stderr.match('200 OK'),
-        'smoke test: response code is not 200 getting project list'
-      ).to.equal(true)
-      expect(
-        !!stdout.match(
-          '<title>Your Projects - .*, Online LaTeX Editor</title>'
-        ),
-        'smoke test: body does not have correct title'
-      ).to.equal(true)
-      expect(
-        !!stdout.match('ProjectPageController'),
-        'smoke test: body does not have correct angular controller'
-      ).to.equal(true)
-      return done()
-    })
-  })
-})
+module.exports = { runSmokeTests, SmokeTestFailure }
@@ -0,0 +1,7 @@
+async function run({ getCsrfTokenFor }) {
+  const loginCsrfToken = await getCsrfTokenFor('/login')
+
+  return { loginCsrfToken }
+}
+
+module.exports = { run }
@@ -0,0 +1,36 @@
+const OError = require('@overleaf/o-error')
+const Settings = require('settings-sharelatex')
+const RateLimiter = require('../../../../app/src/infrastructure/RateLimiter')
+
+async function clearRateLimit(endpointName, subject) {
+  try {
+    await RateLimiter.promises.clearRateLimit(endpointName, subject)
+  } catch (err) {
+    throw new OError(
+      'error clearing rate limit',
+      { endpointName, subject },
+      err
+    )
+  }
+}
+
+async function clearLoginRateLimit() {
+  await clearRateLimit('login', Settings.smokeTest.user)
+}
+
+async function clearOpenProjectRateLimit() {
+  await clearRateLimit(
+    'open-project',
+    `${Settings.smokeTest.projectId}:${Settings.smokeTest.userId}`
+  )
+}
+
+async function run({ processWithTimeout, timeout }) {
+  await processWithTimeout({
+    work: Promise.all([clearLoginRateLimit(), clearOpenProjectRateLimit()]),
+    timeout,
+    message: 'cleanupRateLimits timed out'
+  })
+}
+
+module.exports = { run }
@@ -0,0 +1,35 @@
+const Settings = require('settings-sharelatex')
+
+async function run({ assertHasStatusCode, loginCsrfToken, request }) {
+  const response = await request('/login', {
+    method: 'POST',
+    json: {
+      _csrf: loginCsrfToken,
+      email: Settings.smokeTest.user,
+      password: Settings.smokeTest.password
+    }
+  })
+
+  const body = response.body
+  // login success and login failure both receive a status code of 200
+  // see the frontend logic on how to handle the response:
+  //   frontend/js/directives/asyncForm.js -> submitRequest
+  if (body && body.message && body.message.type === 'error') {
+    throw new Error(`login failed: ${body.message.text}`)
+  }
+
+  assertHasStatusCode(response, 200)
+}
+
+async function cleanup({ assertHasStatusCode, getCsrfTokenFor, request }) {
+  const logoutCsrfToken = await getCsrfTokenFor('/logout')
+  const response = await request('/logout', {
+    method: 'POST',
+    headers: {
+      'X-CSRF-Token': logoutCsrfToken
+    }
+  })
+  assertHasStatusCode(response, 302)
+}
+
+module.exports = { cleanup, run }
@@ -0,0 +1,17 @@
+const ANGULAR_PROJECT_CONTROLLER_REGEX = /controller="ProjectPageController"/
+const TITLE_REGEX = /<title>Your Projects - .*, Online LaTeX Editor<\/title>/
+
+async function run({ request, assertHasStatusCode }) {
+  const response = await request('/project')
+
+  assertHasStatusCode(response, 200)
+
+  if (!TITLE_REGEX.test(response.body)) {
+    throw new Error('body does not have correct title')
+  }
+  if (!ANGULAR_PROJECT_CONTROLLER_REGEX.test(response.body)) {
+    throw new Error('body does not have correct angular controller')
+  }
+}
+
+module.exports = { run }
@@ -0,0 +1,16 @@
+const Settings = require('settings-sharelatex')
+
+async function run({ assertHasStatusCode, request }) {
+  const response = await request(`/project/${Settings.smokeTest.projectId}`)
+
+  assertHasStatusCode(response, 200)
+
+  const PROJECT_ID_REGEX = new RegExp(
+    `window.project_id = "${Settings.smokeTest.projectId}"`
+  )
+  if (!PROJECT_ID_REGEX.test(response.body)) {
+    throw new Error('project page html does not have project_id')
+  }
+}
+
+module.exports = { run }
@@ -0,0 +1,27 @@
+const OError = require('@overleaf/o-error')
+const { assertHasStatusCode } = require('./requestHelper')
+const CSRF_REGEX = /window.csrfToken = "(.+?)"/
+
+function _parseCsrf(body) {
+  const match = CSRF_REGEX.exec(body)
+  if (!match) {
+    throw new Error('Cannot find csrfToken in HTML')
+  }
+  return match[1]
+}
+
+function getCsrfTokenForFactory({ request }) {
+  return async function getCsrfTokenFor(endpoint) {
+    try {
+      const response = await request(endpoint)
+      assertHasStatusCode(response, 200)
+      return _parseCsrf(response.body)
+    } catch (err) {
+      throw new OError(`error fetching csrf token on ${endpoint}`, {}, err)
+    }
+  }
+}
+
+module.exports = {
+  getCsrfTokenForFactory
+}
@@ -0,0 +1,7 @@
+const OError = require('@overleaf/o-error')
+
+class SmokeTestFailure extends OError {}
+
+module.exports = {
+  SmokeTestFailure
+}
@@ -0,0 +1,60 @@
+const { Agent } = require('http')
+const { createConnection } = require('net')
+const { promisify } = require('util')
+
+const OError = require('@overleaf/o-error')
+const request = require('request')
+const Settings = require('settings-sharelatex')
+
+// send requests to web router if this is the api process
+const OWN_PORT = Settings.port || Settings.internal.web.port || 3000
+const PORT = (Settings.web && Settings.web.web_router_port) || OWN_PORT
+
+// like the curl option `--resolve DOMAIN:PORT:127.0.0.1`
+class LocalhostAgent extends Agent {
+  createConnection(options, callback) {
+    return createConnection(PORT, '127.0.0.1', callback)
+  }
+}
+
+// degrade the 'HttpOnly; Secure;' flags of the cookie
+class InsecureCookieJar extends request.jar().constructor {
+  setCookie(...args) {
+    const cookie = super.setCookie(...args)
+    cookie.secure = false
+    cookie.httpOnly = false
+    return cookie
+  }
+}
+
+function requestFactory({ timeout }) {
+  return promisify(
+    request.defaults({
+      agent: new LocalhostAgent(),
+      baseUrl: `http://smoke${Settings.cookieDomain}`,
+      headers: {
+        // emulate the header of a https proxy
+        // express wont emit a 'Secure;' cookie on a plain-text connection.
+        'X-Forwarded-Proto': 'https'
+      },
+      jar: new InsecureCookieJar(),
+      timeout
+    })
+  )
+}
+
+function assertHasStatusCode(response, expected) {
+  const { statusCode: actual } = response
+  if (actual !== expected) {
+    throw new OError('unexpected response code', {
+      url: response.request.uri.href,
+      actual,
+      expected
+    })
+  }
+}
+
+module.exports = {
+  assertHasStatusCode,
+  requestFactory
+}
@@ -0,0 +1,18 @@
+async function processWithTimeout({ work, timeout, message }) {
+  let workDeadLine
+  function checkInResults() {
+    clearTimeout(workDeadLine)
+  }
+  await Promise.race([
+    new Promise((resolve, reject) => {
+      workDeadLine = setTimeout(() => {
+        reject(new Error(message))
+      }, timeout)
+    }),
+    work.finally(checkInResults)
+  ])
+}
+
+module.exports = {
+  processWithTimeout
+}