From 5c665a70daee4d7980e98cc661707a01d583d04c Mon Sep 17 00:00:00 2001
From: Paul Melnikow <github@paulmelnikow.com>
Date: Sun, 23 Dec 2018 11:24:22 -0500
Subject: [PATCH] Overhaul initialization pattern for server + server tests
(#2519)
Because `server.js` was long a monolith, there are a bunch of shims in place to facilitate unit testing. A few of the test suites share port 1111 which means if one of them fails to set up, the port won't be freed and other unrelated tests will fail. Some of the tests which trigger server setup include timeouts which were added to give setup code time to run. In one the test suites, we actually modify `process.argv`, which seems completely gross.
This implements a few changes which improve this:
1. Separate the server from the server startup script, splitting out `lib/server.js`.
2. Inject config into the server and validate the config schema.
3. Inject config into the service test runner.
4. Use `portfinder`, a popular utility for grabbing open ports during testing.
5. Switch more of the setup code from callbacks to async-await.
Overall it leaves everything acting more reliably and looking rather cleaner, if in a few places more verbose.
It also fixes the root cause of #1455, a `setTimeout` in `rate-limit`. Off and on during development of this changeset, Mocha would decide not to exit, and that turned out to be the culprit.
Fix #1455
---
lib/in-process-server-test-helpers.js | 79 +++------
lib/request-handler.spec.js | 45 +++--
lib/server.js | 236 ++++++++++++++++++++++++++
server.spec.js => lib/server.spec.js | 84 +++------
lib/service-test-runner/cli.js | 37 ++--
lib/service-test-runner/runner.js | 10 +-
lib/sys/monitor.js | 11 +-
lib/sys/prometheus-metrics.spec.js | 45 +++--
lib/sys/rate-limit.js | 7 +-
lib/test-config.js | 11 --
package-lock.json | 19 +++
package.json | 5 +-
server.js | 132 ++------------
services/github/auth/acceptor.spec.js | 27 +--
services/github/auth/admin.spec.js | 18 +-
services/service-tester.js | 10 +-
16 files changed, 439 insertions(+), 337 deletions(-)
create mode 100644 lib/server.js
rename server.spec.js => lib/server.spec.js (53%)
delete mode 100644 lib/test-config.js
diff --git a/lib/in-process-server-test-helpers.js b/lib/in-process-server-test-helpers.js
index 1b2f90bd17..e7d1a60701 100644
--- a/lib/in-process-server-test-helpers.js
+++ b/lib/in-process-server-test-helpers.js
@@ -1,69 +1,28 @@
-/**
- * Helpers to run a Shields server in process.
- *
- * Usage:
- * let server;
- * before('Start running the server', function () {
- * this.timeout(5000);
- * server = serverHelpers.start();
- * });
- * after('Shut down the server', function () { serverHelpers.stop(server); });
- */
-
'use strict'
-const config = require('./test-config')
const serverConfig = require('./server-config')
-
-let startCalled = false
-
-/**
- * Start the server.
- *
- * @param {Number} port number (optional)
- * @return {Object} The scoutcamp instance
- */
-function start() {
- if (startCalled) {
- throw Error(
- 'Because of the way Shields works, you can only use this ' +
- 'once per node process. Once you call stop(), the game is over.'
- )
+const Server = require('./server')
+
+function createTestServer({ port }) {
+ const config = {
+ ...serverConfig,
+ bind: {
+ port,
+ address: 'localhost',
+ },
+ ssl: {
+ isSecure: false,
+ },
+ baseUri: `http://localhost:${port}`,
+ cors: {
+ allowedOrigin: [],
+ },
+ rateLimit: false,
}
- startCalled = true
- // Modifying config is a bit dirty, but it works, and avoids making bigger
- // changes to server.js.
- serverConfig.bind = {
- host: 'localhost',
- port: config.port,
- }
- const server = require('../server')
- return server
-}
-
-/**
- * Reset the server, to avoid or reduce side effects between tests.
- *
- * @param {Object} server instance
- */
-function reset(server) {
- server.reset()
-}
-
-/**
- * Stop the server.
- *
- * @param {Object} server instance
- */
-async function stop(server) {
- if (server) {
- await server.stop()
- }
+ return new Server(config)
}
module.exports = {
- start,
- reset,
- stop,
+ createTestServer,
}
diff --git a/lib/request-handler.spec.js b/lib/request-handler.spec.js
index cd269f77d7..8b81871a7c 100644
--- a/lib/request-handler.spec.js
+++ b/lib/request-handler.spec.js
@@ -2,9 +2,9 @@
const { expect } = require('chai')
const fetch = require('node-fetch')
-const config = require('./test-config')
-const Camp = require('camp')
+const portfinder = require('portfinder')
const analytics = require('./analytics')
+const Camp = require('camp')
const { makeBadgeData: getBadgeData } = require('./badge-data')
const {
handleRequest,
@@ -12,11 +12,9 @@ const {
_requestCache,
} = require('./request-handler')
-const baseUri = `http://127.0.0.1:${config.port}`
-
-async function performTwoRequests(first, second) {
- expect((await fetch(`${baseUri}${first}`)).ok).to.be.true
- expect((await fetch(`${baseUri}${second}`)).ok).to.be.true
+async function performTwoRequests(baseUrl, first, second) {
+ expect((await fetch(`${baseUrl}${first}`)).ok).to.be.true
+ expect((await fetch(`${baseUrl}${second}`)).ok).to.be.true
}
function fakeHandler(queryParams, match, sendBadge, request) {
@@ -29,9 +27,15 @@ function fakeHandler(queryParams, match, sendBadge, request) {
describe('The request handler', function() {
before(analytics.load)
+ let port, baseUrl
+ beforeEach(async function() {
+ port = await portfinder.getPortPromise()
+ baseUrl = `http://127.0.0.1:${port}`
+ })
+
let camp
beforeEach(function(done) {
- camp = Camp.start({ port: config.port, hostname: '::' })
+ camp = Camp.start({ port, hostname: '::' })
camp.on('listening', () => done())
})
afterEach(function(done) {
@@ -53,7 +57,7 @@ describe('The request handler', function() {
})
it('should return the expected response', async function() {
- const res = await fetch(`${baseUri}/testing/123.json`)
+ const res = await fetch(`${baseUrl}/testing/123.json`)
expect(res.ok).to.be.true
expect(await res.json()).to.deep.equal({ name: 'testing', value: '123' })
})
@@ -68,7 +72,7 @@ describe('The request handler', function() {
})
it('should return the expected response', async function() {
- const res = await fetch(`${baseUri}/testing/123.json`)
+ const res = await fetch(`${baseUrl}/testing/123.json`)
expect(res.ok).to.be.true
expect(await res.json()).to.deep.equal({ name: 'testing', value: '123' })
})
@@ -100,12 +104,17 @@ describe('The request handler', function() {
})
it('should cache identical requests', async function() {
- await performTwoRequests('/testing/123.svg', '/testing/123.svg')
+ await performTwoRequests(
+ baseUrl,
+ '/testing/123.svg',
+ '/testing/123.svg'
+ )
expect(handlerCallCount).to.equal(1)
})
it('should differentiate known query parameters', async function() {
await performTwoRequests(
+ baseUrl,
'/testing/123.svg?label=foo',
'/testing/123.svg?label=bar'
)
@@ -114,6 +123,7 @@ describe('The request handler', function() {
it('should ignore unknown query parameters', async function() {
await performTwoRequests(
+ baseUrl,
'/testing/123.svg?foo=1',
'/testing/123.svg?foo=2'
)
@@ -123,7 +133,7 @@ describe('The request handler', function() {
it('should set the expires header to current time + defaultCacheLengthSeconds', async function() {
register({ cacheHeaderConfig: { defaultCacheLengthSeconds: 900 } })
- const res = await fetch(`${baseUri}/testing/123.json`)
+ const res = await fetch(`${baseUrl}/testing/123.json`)
const expectedExpiry = new Date(
+new Date(res.headers.get('date')) + 900000
).toGMTString()
@@ -133,7 +143,7 @@ describe('The request handler', function() {
it('should set the expires header to current time + maxAge', async function() {
register({ cacheHeaderConfig: { defaultCacheLengthSeconds: 0 } })
- const res = await fetch(`${baseUri}/testing/123.json?maxAge=3600`)
+ const res = await fetch(`${baseUrl}/testing/123.json?maxAge=3600`)
const expectedExpiry = new Date(
+new Date(res.headers.get('date')) + 3600000
).toGMTString()
@@ -143,7 +153,7 @@ describe('The request handler', function() {
it('should ignore maxAge if maxAge < defaultCacheLengthSeconds', async function() {
register({ cacheHeaderConfig: { defaultCacheLengthSeconds: 600 } })
- const res = await fetch(`${baseUri}/testing/123.json?maxAge=300`)
+ const res = await fetch(`${baseUrl}/testing/123.json?maxAge=300`)
const expectedExpiry = new Date(
+new Date(res.headers.get('date')) + 600000
).toGMTString()
@@ -153,7 +163,7 @@ describe('The request handler', function() {
it('should set Cache-Control: no-cache, no-store, must-revalidate if maxAge=0', async function() {
register({ cacheHeaderConfig: { defaultCacheLengthSeconds: 0 } })
- const res = await fetch(`${baseUri}/testing/123.json`)
+ const res = await fetch(`${baseUrl}/testing/123.json`)
expect(res.headers.get('expires')).to.equal(res.headers.get('date'))
expect(res.headers.get('cache-control')).to.equal(
'no-cache, no-store, must-revalidate'
@@ -167,14 +177,14 @@ describe('The request handler', function() {
const expectedCacheKey = '/testing/123.json?colorB=123&label=foo'
it('should match expected and use canonical order - 1', async function() {
const res = await fetch(
- `${baseUri}/testing/123.json?colorB=123&label=foo`
+ `${baseUrl}/testing/123.json?colorB=123&label=foo`
)
expect(res.ok).to.be.true
expect(_requestCache.cache).to.have.keys(expectedCacheKey)
})
it('should match expected and use canonical order - 2', async function() {
const res = await fetch(
- `${baseUri}/testing/123.json?label=foo&colorB=123`
+ `${baseUrl}/testing/123.json?label=foo&colorB=123`
)
expect(res.ok).to.be.true
expect(_requestCache.cache).to.have.keys(expectedCacheKey)
@@ -200,6 +210,7 @@ describe('The request handler', function() {
it('should differentiate them', async function() {
await performTwoRequests(
+ baseUrl,
'/testing/123.svg?foo=1',
'/testing/123.svg?foo=2'
)
diff --git a/lib/server.js b/lib/server.js
new file mode 100644
index 0000000000..f8436e307e
--- /dev/null
+++ b/lib/server.js
@@ -0,0 +1,236 @@
+'use strict'
+
+const path = require('path')
+const Joi = require('joi')
+const Camp = require('camp')
+const makeBadge = require('../gh-badges/lib/make-badge')
+const GithubConstellation = require('../services/github/github-constellation')
+const { loadServiceClasses } = require('../services')
+const analytics = require('./analytics')
+const { makeBadgeData } = require('./badge-data')
+const log = require('./log')
+const { staticBadgeUrl } = require('./make-badge-url')
+const suggest = require('./suggest')
+const sysMonitor = require('./sys/monitor')
+const PrometheusMetrics = require('./sys/prometheus-metrics')
+const { makeSend } = require('./result-sender')
+const { handleRequest, clearRequestCache } = require('./request-handler')
+const { clearRegularUpdateCache } = require('./regular-update')
+
+const optionalUrl = Joi.string().uri({ scheme: ['http', 'https'] })
+const requiredUrl = optionalUrl.required()
+
+const configSchema = Joi.object({
+ bind: {
+ port: Joi.number()
+ .port()
+ .required(),
+ address: Joi.alternatives()
+ .try(
+ Joi.string()
+ .ip()
+ .required(),
+ Joi.string()
+ .hostname()
+ .required()
+ )
+ .required(),
+ },
+ metrics: {
+ prometheus: {
+ enabled: Joi.boolean().required(),
+ allowedIps: Joi.string(),
+ },
+ },
+ ssl: {
+ isSecure: Joi.boolean().required(),
+ key: Joi.string(),
+ cert: Joi.string(),
+ },
+ baseUri: requiredUrl,
+ redirectUri: optionalUrl,
+ cors: {
+ allowedOrigin: Joi.array()
+ .items(optionalUrl)
+ .required(),
+ },
+ persistence: {
+ dir: Joi.string().required(),
+ redisUrl: optionalUrl,
+ },
+ services: {
+ github: {
+ baseUri: requiredUrl,
+ debug: {
+ enabled: Joi.boolean().required(),
+ intervalSeconds: Joi.number()
+ .integer()
+ .min(1),
+ },
+ },
+ trace: Joi.boolean().required(),
+ },
+ profiling: {
+ makeBadge: Joi.boolean().required(),
+ },
+ cacheHeaders: {
+ defaultCacheLengthSeconds: Joi.number().integer(),
+ },
+ rateLimit: Joi.boolean().required(),
+ handleInternalErrors: Joi.boolean().required(),
+}).required()
+
+module.exports = class Server {
+ constructor(config) {
+ Joi.assert(config, configSchema)
+ this.config = config
+
+ this.githubConstellation = new GithubConstellation({
+ persistence: config.persistence,
+ service: config.services.github,
+ })
+ this.metrics = new PrometheusMetrics(config.metrics.prometheus)
+ }
+
+ get baseUrl() {
+ return this.config.baseUri
+ }
+
+ registerErrorHandlers() {
+ const { camp } = this
+
+ camp.notfound(/\.(svg|png|gif|jpg|json)/, (query, match, end, request) => {
+ const format = match[1]
+ const badgeData = makeBadgeData('404', query)
+ badgeData.text[1] = 'badge not found'
+ badgeData.colorscheme = 'red'
+ // Add format to badge data.
+ badgeData.format = format
+ const svg = makeBadge(badgeData)
+ makeSend(format, request.res, end)(svg)
+ })
+
+ camp.notfound(/.*/, (query, match, end, request) => {
+ end(null, { template: '404.html' })
+ })
+ }
+
+ registerServices() {
+ const { config, camp } = this
+ const { apiProvider: githubApiProvider } = this.githubConstellation
+
+ loadServiceClasses().forEach(serviceClass =>
+ serviceClass.register(
+ { camp, handleRequest, githubApiProvider },
+ {
+ handleInternalErrors: config.handleInternalErrors,
+ cacheHeaders: config.cacheHeaders,
+ profiling: config.profiling,
+ }
+ )
+ )
+ }
+
+ registerRedirects() {
+ const { config, camp } = this
+
+ // Any badge, old version. This route must be registered last.
+ camp.route(/^\/([^/]+)\/(.+).png$/, (queryParams, match, end, ask) => {
+ const [, label, message] = match
+ const { color } = queryParams
+
+ const redirectUrl = staticBadgeUrl({
+ label,
+ message,
+ color,
+ format: 'png',
+ })
+
+ ask.res.statusCode = 301
+ ask.res.setHeader('Location', redirectUrl)
+
+ // The redirect is permanent.
+ const cacheDuration = (365 * 24 * 3600) | 0 // 1 year
+ ask.res.setHeader('Cache-Control', `max-age=${cacheDuration}`)
+
+ ask.res.end()
+ })
+
+ if (config.redirectUri) {
+ camp.route(/^\/$/, (data, match, end, ask) => {
+ ask.res.statusCode = 302
+ ask.res.setHeader('Location', config.redirectUri)
+ ask.res.end()
+ })
+ }
+ }
+
+ async start() {
+ const {
+ bind: { port, address: hostname },
+ ssl: { isSecure: secure, cert, key },
+ cors: { allowedOrigin },
+ baseUri,
+ rateLimit,
+ } = this.config
+
+ log(`Server is starting up: ${baseUri}`)
+
+ const camp = (this.camp = Camp.start({
+ documentRoot: path.join(__dirname, '..', 'public'),
+ port,
+ hostname,
+ secure,
+ cert,
+ key,
+ }))
+
+ analytics.load()
+ analytics.scheduleAutosaving()
+ analytics.setRoutes(camp)
+
+ this.cleanupMonitor = sysMonitor.setRoutes({ rateLimit }, camp)
+
+ const { githubConstellation, metrics } = this
+ githubConstellation.initialize(camp)
+ metrics.initialize(camp)
+
+ const { githubApiProvider } = this.githubConstellation
+ suggest.setRoutes(allowedOrigin, githubApiProvider, camp)
+
+ this.registerErrorHandlers()
+ this.registerServices()
+
+ await new Promise(resolve => camp.on('listening', () => resolve()))
+ }
+
+ static resetGlobalState() {
+ // This state should be migrated to instance state. When possible, do not add new
+ // global state.
+ clearRequestCache()
+ clearRegularUpdateCache()
+ }
+
+ reset() {
+ this.constructor.resetGlobalState()
+ }
+
+ async stop() {
+ if (this.camp) {
+ await new Promise(resolve => this.camp.close(resolve))
+ this.camp = undefined
+ }
+
+ if (this.cleanupMonitor) {
+ this.cleanupMonitor()
+ this.cleanupMonitor = undefined
+ }
+
+ if (this.githubConstellation) {
+ await this.githubConstellation.stop()
+ this.githubConstellation = undefined
+ }
+
+ analytics.cancelAutosaving()
+ }
+}
diff --git a/server.spec.js b/lib/server.spec.js
similarity index 53%
rename from server.spec.js
rename to lib/server.spec.js
index 9320288b3a..972cc549ef 100644
--- a/server.spec.js
+++ b/lib/server.spec.js
@@ -1,68 +1,33 @@
'use strict'
const { expect } = require('chai')
-const config = require('./lib/test-config')
-const serverConfig = require('./lib/server-config')
const fetch = require('node-fetch')
const fs = require('fs')
const isPng = require('is-png')
const isSvg = require('is-svg')
const path = require('path')
-const serverHelpers = require('./lib/in-process-server-test-helpers')
const sinon = require('sinon')
-const Camp = require('camp')
-const svg2img = require('./gh-badges/lib/svg-to-img')
-const { handleRequest } = require('./lib/request-handler')
-const { loadServiceClasses } = require('./services')
+const portfinder = require('portfinder')
+const svg2img = require('../gh-badges/lib/svg-to-img')
+const { createTestServer } = require('./in-process-server-test-helpers')
describe('The server', function() {
- let dummyCamp = Camp.start({ port: config.port, hostname: '::' })
- before('Check the services', function() {
- // The responsibility of this `before()` hook is to verify that the server
- // will be able to register all the services. When it fails, the balance of
- // this `describe()` block – that is, the server tests – does not run.
- //
- // Without this block, the next `before()` hook fails while printing this
- // quite opaque message:
- //
- // Error: listen EADDRINUSE :::1111
- // at Object._errnoException (util.js:1022:11)
- // at _exceptionWithHostPort (util.js:1044:20)
- // at Camp.setupListenHandle [as _listen2] (net.js:1367:14)
- // at listenInCluster (net.js:1408:12)
- // at doListen (net.js:1517:7)
- // at _combinedTickCallback (internal/process/next_tick.js:141:11)
- // at process._tickDomainCallback (internal/process/next_tick.js:218:9)
- loadServiceClasses().forEach(serviceClass =>
- serviceClass.register({ camp: dummyCamp, handleRequest }, serverConfig)
- )
- dummyCamp.close()
- dummyCamp = undefined
+ let server, baseUrl
+ before('Start the server', async function() {
+ const port = await portfinder.getPortPromise()
+ server = createTestServer({ port })
+ baseUrl = server.baseUrl
+ await server.start()
})
- after(function() {
- // Free up the port and shut down the server immediately, even when the
- // `before()` block fails during registration.
- if (dummyCamp) {
- dummyCamp.close()
- dummyCamp = undefined
+ after('Shut down the server', async function() {
+ if (server) {
+ await server.stop()
}
- })
-
- const baseUri = `http://127.0.0.1:${config.port}`
-
- let server
- before('Start the server', function() {
- this.timeout(5000)
- server = serverHelpers.start()
- })
- after('Shut down the server', function() {
- serverHelpers.stop(server)
+ server = undefined
})
it('should produce colorscheme badges', async function() {
- // This is the first server test to run, and often times out.
- this.timeout(5000)
- const res = await fetch(`${baseUri}/:fruit-apple-green.svg`)
+ const res = await fetch(`${baseUrl}/:fruit-apple-green.svg`)
expect(res.ok).to.be.true
expect(await res.text())
.to.satisfy(isSvg)
@@ -71,14 +36,13 @@ describe('The server', function() {
})
it('should produce colorscheme PNG badges', async function() {
- this.timeout(5000)
- const res = await fetch(`${baseUri}/:fruit-apple-green.png`)
+ const res = await fetch(`${baseUrl}/:fruit-apple-green.png`)
expect(res.ok).to.be.true
expect(await res.buffer()).to.satisfy(isPng)
})
it('should preserve label case', async function() {
- const res = await fetch(`${baseUri}/:fRuiT-apple-green.svg`)
+ const res = await fetch(`${baseUrl}/:fRuiT-apple-green.svg`)
expect(res.ok).to.be.true
expect(await res.text())
.to.satisfy(isSvg)
@@ -87,7 +51,7 @@ describe('The server', function() {
// https://github.com/badges/shields/pull/1319
it('should not crash with a numeric logo', async function() {
- const res = await fetch(`${baseUri}/:fruit-apple-green.svg?logo=1`)
+ const res = await fetch(`${baseUrl}/:fruit-apple-green.svg?logo=1`)
expect(res.ok).to.be.true
expect(await res.text())
.to.satisfy(isSvg)
@@ -96,7 +60,7 @@ describe('The server', function() {
})
it('should not crash with a numeric link', async function() {
- const res = await fetch(`${baseUri}/:fruit-apple-green.svg?link=1`)
+ const res = await fetch(`${baseUrl}/:fruit-apple-green.svg?link=1`)
expect(res.ok).to.be.true
expect(await res.text())
.to.satisfy(isSvg)
@@ -105,7 +69,7 @@ describe('The server', function() {
})
it('should not crash with a boolean link', async function() {
- const res = await fetch(`${baseUri}/:fruit-apple-green.svg?link=true`)
+ const res = await fetch(`${baseUrl}/:fruit-apple-green.svg?link=true`)
expect(res.ok).to.be.true
expect(await res.text())
.to.satisfy(isSvg)
@@ -114,7 +78,7 @@ describe('The server', function() {
})
it('should return the 404 badge for unknown badges', async function() {
- const res = await fetch(`${baseUri}/this/is/not/a/badge.svg`)
+ const res = await fetch(`${baseUrl}/this/is/not/a/badge.svg`)
expect(res.status).to.equal(404)
expect(await res.text())
.to.satisfy(isSvg)
@@ -123,14 +87,14 @@ describe('The server', function() {
})
it('should return the 404 html page for rando links', async function() {
- const res = await fetch(`${baseUri}/this/is/most/definitely/not/a/badge.js`)
+ const res = await fetch(`${baseUrl}/this/is/most/definitely/not/a/badge.js`)
expect(res.status).to.equal(404)
expect(await res.text()).to.include('blood, toil, tears and sweat')
})
context('with svg2img error', function() {
const expectedError = fs.readFileSync(
- path.resolve(__dirname, 'public', '500.html')
+ path.resolve(__dirname, '..', 'public', '500.html')
)
let toBufferStub
@@ -144,7 +108,7 @@ describe('The server', function() {
})
it('should emit the 500 message', async function() {
- const res = await fetch(`${baseUri}/:some_new-badge-green.png`)
+ const res = await fetch(`${baseUrl}/:some_new-badge-green.png`)
// This emits status code 200, though 500 would be preferable.
expect(res.status).to.equal(200)
expect(await res.text()).to.include(expectedError)
@@ -153,7 +117,7 @@ describe('The server', function() {
describe('analytics endpoint', function() {
it('should return analytics in the expected format', async function() {
- const res = await fetch(`${baseUri}/$analytics/v1`)
+ const res = await fetch(`${baseUrl}/$analytics/v1`)
expect(res.ok).to.be.true
const json = await res.json()
const expectedKeys = [
diff --git a/lib/service-test-runner/cli.js b/lib/service-test-runner/cli.js
index 96b8d17635..3fa281fa6f 100644
--- a/lib/service-test-runner/cli.js
+++ b/lib/service-test-runner/cli.js
@@ -52,26 +52,39 @@
'use strict'
const minimist = require('minimist')
+const envFlag = require('node-env-flag')
const readAllStdinSync = require('read-all-stdin-sync')
const Runner = require('./runner')
-const serverHelpers = require('../../lib/in-process-server-test-helpers')
+const { createTestServer } = require('../../lib/in-process-server-test-helpers')
require('../../lib/unhandled-rejection.spec')
-let server
-before('Start running the server', function() {
- this.timeout(5000)
- server = serverHelpers.start()
-})
-after('Shut down the server', async function() {
- await serverHelpers.stop(server)
-})
+let baseUrl, server
+if (process.env.TESTED_SERVER_URL) {
+ baseUrl = process.env.TESTED_SERVER_URL
+} else {
+ const port = 1111
+ baseUrl = 'http://localhost:1111'
+ before('Start running the server', function() {
+ server = createTestServer({ port })
+ server.start()
+ })
+ after('Shut down the server', async function() {
+ if (server) {
+ await server.stop()
+ }
+ })
+}
-const runner = new Runner()
+const skipIntercepted = envFlag(process.env.SKIP_INTERCEPTED, false)
+const runner = new Runner({ baseUrl, skipIntercepted })
runner.prepare()
+
// The server's request cache causes side effects between tests.
-runner.beforeEach = () => {
- serverHelpers.reset(server)
+if (!process.env.TESTED_SERVER_URL) {
+ runner.beforeEach = () => {
+ server.reset()
+ }
}
const args = minimist(process.argv.slice(3))
diff --git a/lib/service-test-runner/runner.js b/lib/service-test-runner/runner.js
index 49694d30a0..ba7daa82b8 100644
--- a/lib/service-test-runner/runner.js
+++ b/lib/service-test-runner/runner.js
@@ -6,6 +6,11 @@ const { loadTesters } = require('../../services')
* Load a collection of ServiceTester objects and register them with Mocha.
*/
class Runner {
+ constructor({ baseUrl, skipIntercepted }) {
+ this.baseUrl = baseUrl
+ this.skipIntercepted = skipIntercepted
+ }
+
/**
* Function to invoke before each test. This is a stub which can be
* overridden on instances.
@@ -59,9 +64,8 @@ class Runner {
* Register the tests with Mocha.
*/
toss() {
- this.testers.forEach(tester => {
- tester.toss()
- })
+ const { testers, baseUrl, skipIntercepted } = this
+ testers.forEach(tester => tester.toss({ baseUrl, skipIntercepted }))
}
}
module.exports = Runner
diff --git a/lib/sys/monitor.js b/lib/sys/monitor.js
index 0f0050991c..9e62cb2bb2 100644
--- a/lib/sys/monitor.js
+++ b/lib/sys/monitor.js
@@ -2,7 +2,6 @@
const secretIsValid = require('./secret-is-valid')
const serverSecrets = require('../server-secrets')
-const config = require('../server-config')
const RateLimit = require('./rate-limit')
const log = require('../log')
@@ -17,7 +16,7 @@ function secretInvalid(req, res) {
return false
}
-function setRoutes(server) {
+function setRoutes({ rateLimit }, server) {
const ipRateLimit = new RateLimit({
whitelist: /^192\.30\.252\.\d+$/, // Whitelist GitHub IPs.
})
@@ -34,7 +33,7 @@ function setRoutes(server) {
}
}
- if (config.rateLimit) {
+ if (rateLimit) {
const ip =
(req.headers['x-forwarded-for'] || '').split(', ')[0] ||
req.socket.remoteAddress
@@ -86,6 +85,12 @@ function setRoutes(server) {
referer: refererRateLimit.toJSON(),
})
})
+
+ return function() {
+ ipRateLimit.stop()
+ badgeTypeRateLimit.stop()
+ refererRateLimit.stop()
+ }
}
module.exports = {
diff --git a/lib/sys/prometheus-metrics.spec.js b/lib/sys/prometheus-metrics.spec.js
index 32d1861e77..e6d9759dcd 100644
--- a/lib/sys/prometheus-metrics.spec.js
+++ b/lib/sys/prometheus-metrics.spec.js
@@ -2,32 +2,31 @@
const { expect } = require('chai')
const Camp = require('camp')
+const portfinder = require('portfinder')
const fetch = require('node-fetch')
-const config = require('../test-config')
const Metrics = require('./prometheus-metrics')
describe('Prometheus metrics route', function() {
- const baseUrl = `http://127.0.0.1:${config.port}`
+ let port, baseUrl
+ beforeEach(async function() {
+ port = await portfinder.getPortPromise()
+ baseUrl = `http://127.0.0.1:${port}`
+ })
let camp
- afterEach(function(done) {
+ beforeEach(async function() {
+ camp = Camp.start({ port, hostname: '::' })
+ await new Promise(resolve => camp.on('listening', () => resolve()))
+ })
+ afterEach(async function() {
if (camp) {
- camp.close(() => done())
+ await new Promise(resolve => camp.close(resolve))
camp = undefined
}
})
- function startServer(metricsConfig) {
- return new Promise((resolve, reject) => {
- camp = Camp.start({ port: config.port, hostname: '::' })
- const metrics = new Metrics(metricsConfig)
- metrics.initialize(camp)
- camp.on('listening', () => resolve())
- })
- }
-
it('returns 404 when metrics are disabled', async function() {
- startServer({ enabled: false })
+ new Metrics({ enabled: false }).initialize(camp)
const res = await fetch(`${baseUrl}/metrics`)
@@ -36,7 +35,7 @@ describe('Prometheus metrics route', function() {
})
it('returns 404 when there is no configuration', async function() {
- startServer()
+ new Metrics().initialize(camp)
const res = await fetch(`${baseUrl}/metrics`)
@@ -45,10 +44,10 @@ describe('Prometheus metrics route', function() {
})
it('returns metrics for allowed IP', async function() {
- startServer({
+ new Metrics({
enabled: true,
allowedIps: '^(127\\.0\\.0\\.1|::1|::ffff:127\\.0\\.0\\.1)$',
- })
+ }).initialize(camp)
const res = await fetch(`${baseUrl}/metrics`)
@@ -57,10 +56,10 @@ describe('Prometheus metrics route', function() {
})
it('returns metrics for request from allowed remote address', async function() {
- startServer({
+ new Metrics({
enabled: true,
allowedIps: '^(127\\.0\\.0\\.1|::1|::ffff:127\\.0\\.0\\.1)$',
- })
+ }).initialize(camp)
const res = await fetch(`${baseUrl}/metrics`)
@@ -69,10 +68,10 @@ describe('Prometheus metrics route', function() {
})
it('returns 403 for not allowed IP', async function() {
- startServer({
+ new Metrics({
enabled: true,
allowedIps: '^127\\.0\\.0\\.200$',
- })
+ }).initialize(camp)
const res = await fetch(`${baseUrl}/metrics`)
@@ -81,9 +80,9 @@ describe('Prometheus metrics route', function() {
})
it('returns 403 for every request when list with allowed IPs not defined', async function() {
- startServer({
+ new Metrics({
enabled: true,
- })
+ }).initialize(camp)
const res = await fetch(`${baseUrl}/metrics`)
diff --git a/lib/sys/rate-limit.js b/lib/sys/rate-limit.js
index 8e64f73dd6..a3378d0901 100644
--- a/lib/sys/rate-limit.js
+++ b/lib/sys/rate-limit.js
@@ -11,7 +11,12 @@ module.exports = class RateLimit {
this.banned = new Set()
this.bannedUrls = new Set()
this.whitelist = options.whitelist || /(?!)/ // Matches nothing by default.
- setInterval(this.resetHits.bind(this), this.period * 1000)
+ this.interval = setInterval(this.resetHits.bind(this), this.period * 1000)
+ }
+
+ stop() {
+ clearInterval(this.interval)
+ this.interval = undefined
}
resetHits() {
diff --git a/lib/test-config.js b/lib/test-config.js
deleted file mode 100644
index 49703320ac..0000000000
--- a/lib/test-config.js
+++ /dev/null
@@ -1,11 +0,0 @@
-'use strict'
-
-const envFlag = require('node-env-flag')
-
-module.exports = {
- port: 1111,
- get testedServerUrl() {
- return process.env.TESTED_SERVER_URL || `http://localhost:${this.port}`
- },
- skipIntercepted: envFlag(process.env.SKIP_INTERCEPTED, false),
-}
diff --git a/package-lock.json b/package-lock.json
index 366a39f116..e8eed080d3 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -12874,6 +12874,25 @@
"integrity": "sha512-ARhBOdzS3e41FbkW/XWrTEtukqqLoK5+Z/4UeDaLuSW+39JPeFgs4gCGqsrJHVZX0fUrx//4OF0K1CUGwlIFow==",
"dev": true
},
+ "portfinder": {
+ "version": "1.0.20",
+ "resolved": "https://registry.npmjs.org/portfinder/-/portfinder-1.0.20.tgz",
+ "integrity": "sha512-Yxe4mTyDzTd59PZJY4ojZR8F+E5e97iq2ZOHPz3HDgSvYC5siNad2tLooQ5y5QHyQhc3xVqvyk/eNA3wuoa7Sw==",
+ "dev": true,
+ "requires": {
+ "async": "^1.5.2",
+ "debug": "^2.2.0",
+ "mkdirp": "0.5.x"
+ },
+ "dependencies": {
+ "async": {
+ "version": "1.5.2",
+ "resolved": "http://registry.npmjs.org/async/-/async-1.5.2.tgz",
+ "integrity": "sha1-7GphrlZIDAw8skHJVhjiCJL5Zyo=",
+ "dev": true
+ }
+ }
+ },
"posix-character-classes": {
"version": "0.1.1",
"resolved": "https://registry.npmjs.org/posix-character-classes/-/posix-character-classes-0.1.1.tgz",
diff --git a/package.json b/package.json
index 144d0e1db7..0b302f6cba 100644
--- a/package.json
+++ b/package.json
@@ -58,7 +58,7 @@
},
"scripts": {
"coverage:test:frontend": "BABEL_ENV=test nyc --nycrc-path .nycrc-frontend.json node_modules/mocha/bin/_mocha --opts mocha.opts --require @babel/polyfill --require @babel/register --require mocha-yaml-loader \"frontend/**/*.spec.js\"",
- "coverage:test:server": "HANDLE_INTERNAL_ERRORS=false nyc node_modules/mocha/bin/_mocha --opts mocha.opts \"*.spec.js\" \"lib/**/*.spec.js\" \"services/**/*.spec.js\"",
+ "coverage:test:server": "HANDLE_INTERNAL_ERRORS=false nyc node_modules/mocha/bin/_mocha --opts mocha.opts \"lib/**/*.spec.js\" \"services/**/*.spec.js\"",
"coverage:test:package": "nyc node_modules/mocha/bin/_mocha --opts mocha.opts \"gh-badges/**/*.spec.js\"",
"coverage:test:integration": "nyc node_modules/mocha/bin/_mocha --opts mocha.opts \"lib/**/*.integration.js\" \"services/**/*.integration.js\"",
"coverage:test:services": "nyc node_modules/mocha/bin/_mocha --opts mocha.opts --delay lib/service-test-runner/cli.js",
@@ -71,7 +71,7 @@
"prettier-check": "prettier-check \"**/*.js\"",
"danger": "danger",
"test:js:frontend": "BABEL_ENV=test mocha --opts mocha.opts --require @babel/polyfill --require @babel/register --require mocha-yaml-loader \"frontend/**/*.spec.js\"",
- "test:js:server": "HANDLE_INTERNAL_ERRORS=false mocha --opts mocha.opts \"*.spec.js\" \"lib/**/*.spec.js\" \"services/**/*.spec.js\"",
+ "test:js:server": "HANDLE_INTERNAL_ERRORS=false mocha --opts mocha.opts \"lib/**/*.spec.js\" \"services/**/*.spec.js\"",
"test:js:package": "mocha --opts mocha.opts \"gh-badges/**/*.spec.js\"",
"test:integration": "mocha --opts mocha.opts \"lib/**/*.integration.js\" \"services/**/*.integration.js\"",
"test:services": "HANDLE_INTERNAL_ERRORS=false mocha --opts mocha.opts --delay lib/service-test-runner/cli.js",
@@ -165,6 +165,7 @@
"node-mocks-http": "^1.7.3",
"nyc": "^13.0.1",
"opn-cli": "^4.0.0",
+ "portfinder": "^1.0.20",
"prettier": "1.15.3",
"prettier-check": "^2.0.0",
"pretty": "^2.0.0",
diff --git a/server.js b/server.js
index 402d41c32e..7ddd43b366 100644
--- a/server.js
+++ b/server.js
@@ -1,131 +1,19 @@
'use strict'
-const path = require('path')
const Raven = require('raven')
-
const serverSecrets = require('./lib/server-secrets')
+
Raven.config(process.env.SENTRY_DSN || serverSecrets.sentry_dsn).install()
Raven.disableConsoleAlerts()
-const { loadServiceClasses } = require('./services')
-const analytics = require('./lib/analytics')
+const Server = require('./lib/server')
const config = require('./lib/server-config')
-const GithubConstellation = require('./services/github/github-constellation')
-const PrometheusMetrics = require('./lib/sys/prometheus-metrics')
-const sysMonitor = require('./lib/sys/monitor')
-const log = require('./lib/log')
-const { staticBadgeUrl } = require('./lib/make-badge-url')
-const makeBadge = require('./gh-badges/lib/make-badge')
-const suggest = require('./lib/suggest')
-const { makeBadgeData } = require('./lib/badge-data')
-const { handleRequest, clearRequestCache } = require('./lib/request-handler')
-const { clearRegularUpdateCache } = require('./lib/regular-update')
-const { makeSend } = require('./lib/result-sender')
-
-const camp = require('camp').start({
- documentRoot: path.join(__dirname, 'public'),
- port: config.bind.port,
- hostname: config.bind.address,
- secure: config.ssl.isSecure,
- cert: config.ssl.cert,
- key: config.ssl.key,
-})
-
-const githubConstellation = new GithubConstellation({
- persistence: config.persistence,
- service: config.services.github,
-})
-const metrics = new PrometheusMetrics(config.metrics.prometheus)
-const { apiProvider: githubApiProvider } = githubConstellation
-
-function reset() {
- clearRequestCache()
- clearRegularUpdateCache()
-}
-
-async function stop() {
- await githubConstellation.stop()
- analytics.cancelAutosaving()
- return new Promise(resolve => {
- camp.close(resolve)
- })
-}
-
-module.exports = {
- camp,
- reset,
- stop,
-}
-
-log(`Server is starting up: ${config.baseUri}`)
-
-analytics.load()
-analytics.scheduleAutosaving()
-analytics.setRoutes(camp)
-
-if (serverSecrets && serverSecrets.shieldsSecret) {
- sysMonitor.setRoutes(camp)
-}
-
-githubConstellation.initialize(camp)
-metrics.initialize(camp)
-
-suggest.setRoutes(config.cors.allowedOrigin, githubApiProvider, camp)
-
-camp.notfound(/\.(svg|png|gif|jpg|json)/, (query, match, end, request) => {
- const format = match[1]
- const badgeData = makeBadgeData('404', query)
- badgeData.text[1] = 'badge not found'
- badgeData.colorscheme = 'red'
- // Add format to badge data.
- badgeData.format = format
- const svg = makeBadge(badgeData)
- makeSend(format, request.res, end)(svg)
-})
-
-camp.notfound(/.*/, (query, match, end, request) => {
- end(null, { template: '404.html' })
-})
-
-// Vendors.
-
-loadServiceClasses().forEach(serviceClass =>
- serviceClass.register(
- { camp, handleRequest, githubApiProvider },
- {
- handleInternalErrors: config.handleInternalErrors,
- cacheHeaders: config.cacheHeaders,
- profiling: config.profiling,
- }
- )
-)
-
-// Any badge, old version. This route must be registered last.
-camp.route(/^\/([^/]+)\/(.+).png$/, (queryParams, match, end, ask) => {
- const [, label, message] = match
- const { color } = queryParams
-
- const redirectUrl = staticBadgeUrl({
- label,
- message,
- color,
- format: 'png',
- })
-
- ask.res.statusCode = 301
- ask.res.setHeader('Location', redirectUrl)
-
- // The redirect is permanent.
- const cacheDuration = (365 * 24 * 3600) | 0 // 1 year
- ask.res.setHeader('Cache-Control', `max-age=${cacheDuration}`)
-
- ask.res.end()
-})
-if (config.redirectUri) {
- camp.route(/^\/$/, (data, match, end, ask) => {
- ask.res.statusCode = 302
- ask.res.setHeader('Location', config.redirectUri)
- ask.res.end()
- })
-}
+;(async () => {
+ try {
+ await new Server(config).start()
+ } catch (e) {
+ console.error(e)
+ process.exit(1)
+ }
+})()
diff --git a/services/github/auth/acceptor.spec.js b/services/github/auth/acceptor.spec.js
index 888a8fcdca..6bb10e56ff 100644
--- a/services/github/auth/acceptor.spec.js
+++ b/services/github/auth/acceptor.spec.js
@@ -3,13 +3,12 @@
const { expect } = require('chai')
const Camp = require('camp')
const got = require('got')
+const portfinder = require('portfinder')
const queryString = require('query-string')
const nock = require('nock')
-const config = require('../../../lib/test-config')
const serverSecrets = require('../../../lib/server-secrets')
const acceptor = require('./acceptor')
-const baseUri = `http://127.0.0.1:${config.port}`
const fakeClientId = 'githubdabomb'
describe('Github token acceptor', function() {
@@ -25,15 +24,21 @@ describe('Github token acceptor', function() {
delete serverSecrets.shieldsIps
})
+ let port, baseUrl
+ beforeEach(async function() {
+ port = await portfinder.getPortPromise()
+ baseUrl = `http://127.0.0.1:${port}`
+ })
+
let camp
- beforeEach(function(done) {
- camp = Camp.start({ port: config.port, hostname: '::' })
- camp.on('listening', () => done())
+ beforeEach(async function() {
+ camp = Camp.start({ port, hostname: '::' })
+ await new Promise(resolve => camp.on('listening', () => resolve()))
})
- afterEach(function(done) {
+ afterEach(async function() {
if (camp) {
- camp.close(() => done())
- camp = null
+ await new Promise(resolve => camp.close(resolve))
+ camp = undefined
}
})
@@ -42,7 +47,7 @@ describe('Github token acceptor', function() {
})
it('should start the OAuth process', async function() {
- const res = await got(`${baseUri}/github-auth`, { followRedirect: false })
+ const res = await got(`${baseUrl}/github-auth`, { followRedirect: false })
expect(res.statusCode).to.equal(302)
@@ -57,7 +62,7 @@ describe('Github token acceptor', function() {
describe('Finishing the OAuth process', function() {
context('no code is provided', function() {
it('should return an error', async function() {
- const res = await got(`${baseUri}/github-auth/done`)
+ const res = await got(`${baseUrl}/github-auth/done`)
expect(res.body).to.equal(
'GitHub OAuth authentication failed to provide a code.'
)
@@ -93,7 +98,7 @@ describe('Github token acceptor', function() {
})
it('should finish the OAuth process', async function() {
- const res = await got(`${baseUri}/github-auth/done`, {
+ const res = await got(`${baseUrl}/github-auth/done`, {
form: true,
body: { code: fakeCode },
})
diff --git a/services/github/auth/admin.spec.js b/services/github/auth/admin.spec.js
index 8ce4bcdbda..dc16d19f3f 100644
--- a/services/github/auth/admin.spec.js
+++ b/services/github/auth/admin.spec.js
@@ -4,7 +4,7 @@ const { expect } = require('chai')
const sinon = require('sinon')
const Camp = require('camp')
const fetch = require('node-fetch')
-const config = require('../../../lib/test-config')
+const portfinder = require('portfinder')
const serverSecrets = require('../../../lib/server-secrets')
const { setRoutes } = require('./admin')
@@ -34,16 +34,20 @@ describe('GitHub admin route', function() {
sandbox.restore()
})
- const baseUrl = `http://127.0.0.1:${config.port}`
+ let port, baseUrl
+ before(async function() {
+ port = await portfinder.getPortPromise()
+ baseUrl = `http://127.0.0.1:${port}`
+ })
let camp
- before(function(done) {
- camp = Camp.start({ port: config.port, hostname: '::' })
- camp.on('listening', () => done())
+ before(async function() {
+ camp = Camp.start({ port, hostname: '::' })
+ await new Promise(resolve => camp.on('listening', () => resolve()))
})
- after(function(done) {
+ after(async function() {
if (camp) {
- camp.close(() => done())
+ await new Promise(resolve => camp.close(resolve))
camp = undefined
}
})
diff --git a/services/service-tester.js b/services/service-tester.js
index 4dc243c8e2..d04e4a7bc9 100644
--- a/services/service-tester.js
+++ b/services/service-tester.js
@@ -1,7 +1,6 @@
'use strict'
const emojic = require('emojic')
-const config = require('../lib/test-config')
const frisby = require('./icedfrisby-no-nock')(
require('icedfrisby-nock')(require('icedfrisby'))
)
@@ -60,7 +59,6 @@ class ServiceTester {
create(msg) {
const spec = frisby
.create(msg)
- .baseUri(`${config.testedServerUrl}${this.pathPrefix}`)
.before(() => {
this.beforeEach()
})
@@ -92,14 +90,16 @@ class ServiceTester {
/**
* Register the tests with Mocha.
*/
- toss() {
- const specs = this.specs
+ toss({ baseUrl, skipIntercepted }) {
+ const { specs, pathPrefix } = this
+ const testerBaseUrl = `${baseUrl}${pathPrefix}`
const fn = this._only ? describe.only : describe
// eslint-disable-next-line mocha/prefer-arrow-callback
fn(this.title, function() {
specs.forEach(spec => {
- if (!config.skipIntercepted || !spec.intercepted) {
+ if (!skipIntercepted || !spec.intercepted) {
+ spec.baseUri(testerBaseUrl)
spec.toss()
}
})
--
GitLab