refactor(test): consolidate local http server creation

Following the same pattern used to HTTPS local servers, added a utility that creates and listens for a HTTP server and ensures it gets closed after the test ends. There were some tests that weren't properly closing the server if the test failed, but mostly this refactor reduces boilerplate from tests. Not all tests that call `http.createServer` were updated. The ones not migrated to Mocha DSL or Chai were left alone since they're due for refactors anyway.
nock · Mar 14, 2020 · 8638e5b · 8638e5b
1 parent 263fe91
commit 8638e5b
Show file tree

Hide file tree

Showing 12 changed files with 209 additions and 274 deletions.
diff --git a/tests/ssl/ca.crt → tests/servers/ca.crt b/tests/ssl/ca.crt → tests/servers/ca.crt
diff --git a/tests/ssl/ca.key → tests/servers/ca.key b/tests/ssl/ca.key → tests/servers/ca.key
diff --git a/tests/ssl/index.js → tests/servers/index.js b/tests/ssl/index.js → tests/servers/index.js
@@ -12,23 +12,49 @@
 //   openssl x509 -req -in localhost.csr -CA ca.crt -CAkey ca.key -CAcreateserial -out localhost.crt -days 3650
 //   rm ca.srl localhost.csr
 //
+const http = require('http')
 const https = require('https')
 const path = require('path')
 const fs = require('fs')
 
-async function startServer(middleware) {
+// setup is only required here for Tap
+require('../setup')
+
+const servers = []
+
+afterEach(() => {
+  while (servers.length) {
+    const server = servers.pop()
+    server.close()
+  }
+})
+
+async function startHttpServer(requestListener) {
+  const server = http.createServer(requestListener)
+  await new Promise(resolve => server.listen(resolve))
+  servers.push(server)
+  server.port = server.address().port
+  server.origin = `http://localhost:${server.port}`
+  return server
+}
+
+async function startHttpsServer(requestListener) {
   const server = https.createServer(
     {
       key: fs.readFileSync(path.resolve(__dirname, './localhost.key')),
       cert: fs.readFileSync(path.resolve(__dirname, './localhost.crt')),
     },
-    middleware
+    requestListener
   )
   await new Promise(resolve => server.listen(resolve))
+  servers.push(server)
+  server.port = server.address().port
+  server.origin = `https://localhost:${server.port}`
   return server
 }
 
 module.exports = {
   ca: fs.readFileSync(path.resolve(__dirname, './ca.crt')),
-  startServer,
+  startHttpServer,
+  startHttpsServer,
 }
diff --git a/tests/ssl/localhost.crt → tests/servers/localhost.crt b/tests/ssl/localhost.crt → tests/servers/localhost.crt
diff --git a/tests/ssl/localhost.key → tests/servers/localhost.key b/tests/ssl/localhost.key → tests/servers/localhost.key
diff --git a/tests/test_allow_unmocked.js b/tests/test_allow_unmocked.js
@@ -1,31 +1,14 @@
 'use strict'
 
-const http = require('http')
 const { expect } = require('chai')
 const nock = require('..')
+
 const got = require('./got_client')
+const { startHttpServer } = require('./servers')
 
 require('./setup')
 
 describe('allowUnmocked option', () => {
-  let _server
-
-  afterEach(() => {
-    if (_server) {
-      _server.close()
-      _server = null
-    }
-  })
-
-  async function createServer(requestListener) {
-    const server = http.createServer(requestListener)
-    await new Promise(resolve => server.listen(resolve))
-    _server = server
-
-    const url = `http://localhost:${server.address().port}`
-    return { server, url }
-  }
-
   it('with allowUnmocked, mocked request still works', async () => {
     const scope = nock('http://example.test', { allowUnmocked: true })
       .post('/')
@@ -39,21 +22,21 @@ describe('allowUnmocked option', () => {
   })
 
   it('allow unmocked works after one interceptor is removed', async () => {
-    const { url } = await createServer((request, response) => {
+    const { origin } = await startHttpServer((request, response) => {
       response.write('live')
       response.end()
     })
 
-    nock(url, { allowUnmocked: true })
+    nock(origin, { allowUnmocked: true })
       .get('/')
       .reply(200, 'Mocked')
 
-    expect((await got(url)).body).to.equal('Mocked')
-    expect((await got(url)).body).to.equal('live')
+    expect((await got(origin)).body).to.equal('Mocked')
+    expect((await got(origin)).body).to.equal('live')
   })
 
   it('allow unmocked option allows traffic to server', async () => {
-    const { url } = await createServer((request, response) => {
+    const { origin } = await startHttpServer((request, response) => {
       switch (request.url) {
         case '/':
           response.writeHead(200)
@@ -71,12 +54,12 @@ describe('allowUnmocked option', () => {
       response.end()
     })
 
-    const scope = nock(url, { allowUnmocked: true })
+    const scope = nock(origin, { allowUnmocked: true })
       .get('/abc')
       .reply(304, 'served from our mock')
       .get('/wont/get/here')
       .reply(304, 'served from our mock')
-    const client = got.extend({ prefixUrl: url, throwHttpErrors: false })
+    const client = got.extend({ prefixUrl: origin, throwHttpErrors: false })
 
     const response1 = await client('abc')
     expect(response1.statusCode).to.equal(304)
@@ -94,17 +77,17 @@ describe('allowUnmocked option', () => {
   })
 
   it('allow unmocked post with json data', async () => {
-    const { url } = await createServer((request, response) => {
+    const { origin } = await startHttpServer((request, response) => {
       response.writeHead(200)
       response.write('{"message":"server response"}')
       response.end()
     })
 
-    nock(url, { allowUnmocked: true })
+    nock(origin, { allowUnmocked: true })
       .get('/not/accessed')
       .reply(200, '{"message":"mocked response"}')
 
-    const { body, statusCode } = await got.post(url, {
+    const { body, statusCode } = await got.post(origin, {
       json: { some: 'data' },
       responseType: 'json',
     })
@@ -113,17 +96,17 @@ describe('allowUnmocked option', () => {
   })
 
   it('allow unmocked passthrough with mismatched bodies', async () => {
-    const { url } = await createServer((request, response) => {
+    const { origin } = await startHttpServer((request, response) => {
       response.writeHead(200)
       response.write('{"message":"server response"}')
       response.end()
     })
 
-    nock(url, { allowUnmocked: true })
+    nock(origin, { allowUnmocked: true })
       .post('/post', { some: 'other data' })
       .reply(404, '{"message":"server response"}')
 
-    const { body, statusCode } = await got.post(`${url}/post`, {
+    const { body, statusCode } = await got.post(`${origin}/post`, {
       json: { some: 'data' },
       responseType: 'json',
     })
@@ -174,7 +157,7 @@ describe('allowUnmocked option', () => {
 
   // https://github.com/nock/nock/issues/835
   it('match multiple paths to domain using regexp with allowUnmocked', async () => {
-    const { url } = await createServer((request, response) => {
+    const { origin } = await startHttpServer((request, response) => {
       response.write('live')
       response.end()
     })
@@ -187,9 +170,11 @@ describe('allowUnmocked option', () => {
       .get(/bravo/)
       .reply(200, 'bravo, bravo!')
 
-    expect((await got(`${url}`)).body).to.equal('live')
-    expect((await got(`${url}/alphalicious`)).body).to.equal('this is alpha')
-    expect((await got(`${url}/bravo-company`)).body).to.equal('bravo, bravo!')
+    expect((await got(origin)).body).to.equal('live')
+    expect((await got(`${origin}/alphalicious`)).body).to.equal('this is alpha')
+    expect((await got(`${origin}/bravo-company`)).body).to.equal(
+      'bravo, bravo!'
+    )
 
     scope1.done()
     scope2.done()
@@ -222,18 +207,18 @@ describe('allowUnmocked option', () => {
 
   // https://github.com/nock/nock/issues/490
   it('match when query is specified with allowUnmocked', async () => {
-    const { url } = await createServer((request, response) => {
+    const { origin } = await startHttpServer((request, response) => {
       response.write('live')
       response.end()
     })
 
-    const scope = nock(url, { allowUnmocked: true })
+    const scope = nock(origin, { allowUnmocked: true })
       .get('/search')
       .query({ q: 'cat pictures' })
       .reply(200, '😻')
 
-    expect((await got(url)).body).to.equal('live')
-    expect((await got(`${url}/search?q=cat%20pictures`)).body).to.equal('😻')
+    expect((await got(origin)).body).to.equal('live')
+    expect((await got(`${origin}/search?q=cat%20pictures`)).body).to.equal('😻')
 
     scope.done()
   })

diff --git a/tests/test_allow_unmocked_https.js b/tests/test_allow_unmocked_https.js
@@ -2,33 +2,26 @@
 
 const { expect } = require('chai')
 const nock = require('..')
-const ssl = require('./ssl')
+
 const got = require('./got_client')
+const servers = require('./servers')
 
 require('./setup')
 
 describe('allowUnmocked option (https)', () => {
-  let server
-
-  afterEach(() => {
-    if (server) {
-      server.close()
-      server = null
-    }
-  })
-
   it('Nock with allowUnmocked and an url match', async () => {
-    server = await ssl.startServer((req, res) => {
+    const { origin } = await servers.startHttpsServer((req, res) => {
       res.writeHead(200)
       res.end({ status: 'default' })
     })
-    const url = `https://127.0.0.1:${server.address().port}`
 
-    const scope = nock(url, { allowUnmocked: true })
+    const scope = nock(origin, { allowUnmocked: true })
       .get('/urlMatch')
       .reply(201, JSON.stringify({ status: 'intercepted' }))
 
-    const { body, statusCode } = await got(`${url}/urlMatch`, { ca: ssl.ca })
+    const { body, statusCode } = await got(`${origin}/urlMatch`, {
+      ca: servers.ca,
+    })
 
     expect(statusCode).to.equal(201)
     expect(body).to.equal('{"status":"intercepted"}')
@@ -37,7 +30,7 @@ describe('allowUnmocked option (https)', () => {
   })
 
   it('allow unmocked option works with https', async () => {
-    server = await ssl.startServer((request, response) => {
+    const { origin } = await servers.startHttpsServer((request, response) => {
       if (request.url === '/does/not/exist') {
         response.writeHead(404)
         response.end()
@@ -49,15 +42,13 @@ describe('allowUnmocked option (https)', () => {
       response.end()
     })
 
-    const { port } = server.address()
-    const url = `https://localhost:${port}`
     const client = got.extend({
-      prefixUrl: url,
-      ca: ssl.ca,
+      prefixUrl: origin,
+      ca: servers.ca,
       throwHttpErrors: false,
     })
 
-    const scope = nock(url, { allowUnmocked: true })
+    const scope = nock(origin, { allowUnmocked: true })
       .get('/abc')
       .reply(200, 'mocked response')
       .get('/wont/get/here')
@@ -84,22 +75,19 @@ describe('allowUnmocked option (https)', () => {
     // there are interceptors that partially match, eg just path, but don't completely match.
     // This explicitly tests the later case in the overrider by making an HTTPS request for a path
     // that has an interceptor but fails to match the query constraint.
-    server = await ssl.startServer((request, response) => {
+    const { origin } = await servers.startHttpsServer((request, response) => {
       response.writeHead(201)
       response.write('foo')
       response.end()
     })
 
-    const { port } = server.address()
-    const origin = `https://localhost:${port}`
-
     nock(origin, { allowUnmocked: true })
       .get('/foo')
       .query({ foo: 'bar' })
       .reply(418)
 
     // no query so wont match the interceptor
-    const { statusCode, body } = await got(`${origin}/foo`, { ca: ssl.ca })
+    const { statusCode, body } = await got(`${origin}/foo`, { ca: servers.ca })
 
     expect(statusCode).to.equal(201)
     expect(body).to.equal('foo')

diff --git a/tests/test_net_connect.js b/tests/test_net_connect.js
@@ -1,11 +1,12 @@
 'use strict'
 
-const http = require('http')
 const { expect } = require('chai')
 const assertRejects = require('assert-rejects')
 const sinon = require('sinon')
 const nock = require('..')
+
 const got = require('./got_client')
+const servers = require('./servers')
 
 require('./setup')
 
@@ -40,19 +41,16 @@ describe('`disableNetConnect()`', () => {
 describe('`enableNetConnect()`', () => {
   it('enables real HTTP request only for specified domain, via string', async () => {
     const onResponse = sinon.spy()
-    const server = http.createServer((request, response) => {
+    const { origin } = await servers.startHttpServer((request, response) => {
       onResponse()
       response.writeHead(200)
       response.end()
     })
-    await new Promise(resolve => server.listen(resolve))
 
     nock.enableNetConnect('localhost')
 
-    await got(`http://localhost:${server.address().port}/`)
+    await got(origin)
     expect(onResponse).to.have.been.calledOnce()
-
-    server.close()
   })
 
   it('disallows request for other domains, via string', async () => {
@@ -66,19 +64,16 @@ describe('`enableNetConnect()`', () => {
 
   it('enables real HTTP request only for specified domain, via regexp', async () => {
     const onResponse = sinon.spy()
-    const server = http.createServer((request, response) => {
+    const { origin } = await servers.startHttpServer((request, response) => {
       onResponse()
       response.writeHead(200)
       response.end()
     })
-    await new Promise(resolve => server.listen(resolve))
 
     nock.enableNetConnect(/ocalhos/)
 
-    await got(`http://localhost:${server.address().port}/`)
+    await got(origin)
     expect(onResponse).to.have.been.calledOnce()
-
-    server.close()
   })
 
   it('disallows request for other domains, via regexp', async () => {
@@ -92,19 +87,16 @@ describe('`enableNetConnect()`', () => {
 
   it('enables real HTTP request only for specified domain, via function', async () => {
     const onResponse = sinon.spy()
-    const server = http.createServer((request, response) => {
+    const { origin } = await servers.startHttpServer((request, response) => {
       onResponse()
       response.writeHead(200)
       response.end()
     })
-    await new Promise(resolve => server.listen(resolve))
 
     nock.enableNetConnect(host => host.includes('ocalhos'))
 
-    await got(`http://localhost:${server.address().port}/`)
+    await got(origin)
     expect(onResponse).to.have.been.calledOnce()
-
-    server.close()
   })
 
   it('disallows request for other domains, via function', async () => {