test: use proper teardown

Tests are were not waiting for their servers to close.
nodejs · Nov 3, 2020 · 95a7305 · 95a7305
1 parent b2bfa22
commit 95a7305
Showing 1 changed file with 68 additions and 74 deletions.
diff --git a/test/test-download.js b/test/test-download.js
@@ -15,128 +15,126 @@ const log = require('npmlog')
 
 log.level = 'warn'
 
-test('download over http', (t) => {
+test('download over http', async (t) => {
   t.plan(2)
 
   const server = http.createServer((req, res) => {
     t.strictEqual(req.headers['user-agent'], `node-gyp v42 (node ${process.version})`)
     res.end('ok')
-    server.close()
   })
 
+  t.tearDown(() => new Promise((resolve) => server.close(resolve)))
+
   const host = 'localhost'
-  return new Promise(resolve => server.listen(0, host, async () => {
-    const { port } = server.address()
-    const gyp = {
-      opts: {},
-      version: '42'
-    }
-    const url = `http://${host}:${port}`
-    const res = await install.test.download(gyp, url)
-    t.strictEqual(await res.text(), 'ok')
-    resolve()
-  }))
+  await new Promise((resolve) => server.listen(0, host, resolve))
+  const { port } = server.address()
+  const gyp = {
+    opts: {},
+    version: '42'
+  }
+  const url = `http://${host}:${port}`
+  const res = await install.test.download(gyp, url)
+  t.strictEqual(await res.text(), 'ok')
 })
 
 test('download over https with custom ca', async (t) => {
   t.plan(3)
 
-  const [cert, key] = await Promise.all([
+  const cafile = path.join(__dirname, '/fixtures/ca.crt')
+  const [cert, key, ca] = await Promise.all([
     fs.promises.readFile(path.join(__dirname, 'fixtures/server.crt'), 'utf8'),
-    fs.promises.readFile(path.join(__dirname, 'fixtures/server.key'), 'utf8')
+    fs.promises.readFile(path.join(__dirname, 'fixtures/server.key'), 'utf8'),
+    install.test.readCAFile(cafile)
   ])
 
-  const cafile = path.join(__dirname, '/fixtures/ca.crt')
-  const ca = await install.test.readCAFile(cafile)
   t.strictEqual(ca.length, 1)
 
   const options = { ca: ca, cert: cert, key: key }
   const server = https.createServer(options, (req, res) => {
     t.strictEqual(req.headers['user-agent'], `node-gyp v42 (node ${process.version})`)
     res.end('ok')
-    server.close()
   })
 
+  t.tearDown(() => new Promise((resolve) => server.close(resolve)))
+
   server.on('clientError', (err) => { throw err })
 
   const host = 'localhost'
-  return new Promise(resolve => server.listen(0, host, async () => {
-    const { port } = server.address()
-    const gyp = {
-      opts: { cafile },
-      version: '42'
-    }
-    const url = `https://${host}:${port}`
-    const res = await install.test.download(gyp, url)
-    t.strictEqual(await res.text(), 'ok')
-    resolve()
-  }))
+  await new Promise((resolve) => server.listen(0, host, resolve))
+  const { port } = server.address()
+  const gyp = {
+    opts: { cafile },
+    version: '42'
+  }
+  const url = `https://${host}:${port}`
+  const res = await install.test.download(gyp, url)
+  t.strictEqual(await res.text(), 'ok')
 })
 
-test('download over http with proxy', (t) => {
+test('download over http with proxy', async (t) => {
   t.plan(2)
 
   const server = http.createServer((_, res) => {
     res.end('ok')
-    pserver.close(() => { server.close() })
   })
 
   const pserver = http.createServer((req, res) => {
     t.strictEqual(req.headers['user-agent'], `node-gyp v42 (node ${process.version})`)
     res.end('proxy ok')
-    server.close(() => { pserver.close() })
   })
 
+  t.tearDown(() => Promise.all([
+    new Promise((resolve) => server.close(resolve)),
+    new Promise((resolve) => pserver.close(resolve))
+  ]))
+
   const host = 'localhost'
-  return new Promise(resolve => server.listen(0, host, () => {
-    const { port } = server.address()
-    pserver.listen(port + 1, host, async () => {
-      const gyp = {
-        opts: {
-          proxy: `http://${host}:${port + 1}`,
-          noproxy: 'bad'
-        },
-        version: '42'
-      }
-      const url = `http://${host}:${port}`
-      const res = await install.test.download(gyp, url)
-      t.strictEqual(await res.text(), 'proxy ok')
-      resolve()
-    })
-  }))
+  await new Promise((resolve) => server.listen(0, host, resolve))
+  const { port } = server.address()
+  await new Promise((resolve) => pserver.listen(port + 1, host, resolve))
+  const gyp = {
+    opts: {
+      proxy: `http://${host}:${port + 1}`,
+      noproxy: 'bad'
+    },
+    version: '42'
+  }
+  const url = `http://${host}:${port}`
+  const res = await install.test.download(gyp, url)
+  t.strictEqual(await res.text(), 'proxy ok')
 })
 
-test('download over http with noproxy', (t) => {
+test('download over http with noproxy', async (t) => {
   t.plan(2)
 
   const server = http.createServer((req, res) => {
     t.strictEqual(req.headers['user-agent'], `node-gyp v42 (node ${process.version})`)
     res.end('ok')
-    pserver.close(() => { server.close() })
   })
 
   const pserver = http.createServer((_, res) => {
     res.end('proxy ok')
-    server.close(() => { pserver.close() })
   })
 
+  t.tearDown(() => Promise.all([
+    new Promise((resolve) => server.close(resolve)),
+    new Promise((resolve) => pserver.close(resolve))
+  ]))
+
   const host = 'localhost'
-  return new Promise(resolve => server.listen(0, host, () => {
-    const { port } = server.address()
-    pserver.listen(port + 1, host, async () => {
-      const gyp = {
-        opts: {
-          proxy: `http://${host}:${port + 1}`,
-          noproxy: host
-        },
-        version: '42'
-      }
-      const url = `http://${host}:${port}`
-      const res = await install.test.download(gyp, url)
-      t.strictEqual(await res.text(), 'ok')
-      resolve()
-    })
-  }))
+  await new Promise((resolve) => server.listen(0, host, resolve))
+  const { port } = server.address()
+  await new Promise((resolve) => pserver.listen(port + 1, host, resolve))
+  const gyp = {
+    opts: {
+      proxy: `http://${host}:${port + 1}`,
+      noproxy: host
+    },
+    version: '42'
+  }
+  const url = `http://${host}:${port}`
+  const res = await install.test.download(gyp, url)
+  t.strictEqual(await res.text(), 'ok')
 })
 
 test('download with missing cafile', async (t) => {
@@ -179,14 +177,10 @@ test('download headers (actual)', async (t) => {
   log.level = 'warn'
   await util.promisify(install)(prog, [])
 
-  const [data, list, contents] = await Promise.all([
-    fs.promises.readFile(path.join(expectedDir, 'installVersion'), 'utf8'),
-    fs.promises.readdir(path.join(expectedDir, 'include/node')),
-    fs.promises.readFile(path.join(expectedDir, 'include/node/node_version.h'), 'utf8')
-  ])
-
+  const data = await fs.promises.readFile(path.join(expectedDir, 'installVersion'), 'utf8')
   t.strictEqual(data, '9\n', 'correct installVersion')
 
+  const list = await fs.promises.readdir(path.join(expectedDir, 'include/node'))
   t.ok(list.includes('common.gypi'))
   t.ok(list.includes('config.gypi'))
   t.ok(list.includes('node.h'))
@@ -198,7 +192,7 @@ test('download headers (actual)', async (t) => {
   t.ok(list.includes('v8.h'))
   t.ok(list.includes('zlib.h'))
 
-  const lines = contents.split('\n')
+  const lines = (await fs.promises.readFile(path.join(expectedDir, 'include/node/node_version.h'), 'utf8')).split('\n')
 
   // extract the 3 version parts from the defines to build a valid version string and
   // and check them against our current env version