refactor(tests): Rewrite some tests of delay and define using async/g…

…ot (#1522)

README.md

@@ -51,6 +51,7 @@ For instance, if a module performs HTTP requests to a CouchDB server or makes HT
   - [Socket timeout](#socket-timeout)
   - [Chaining](#chaining)
   - [Scope filtering](#scope-filtering)
+  - [Conditional scope filtering](#conditional-scope-filtering)
   - [Path filtering](#path-filtering)
   - [Request Body filtering](#request-body-filtering)
   - [Request Headers Matching](#request-headers-matching)

tests/test_define.js

@@ -1,13 +1,14 @@
 'use strict'
 
 const http = require('http')
+const assertRejects = require('assert-rejects')
 const { test } = require('tap')
 const nock = require('..')
 const got = require('./got_client')
 
 require('./cleanup_after_each')()
 
-test('define() is backward compatible', t => {
+test('define() is backward compatible', async t => {
   t.ok(
     nock.define([
       {
@@ -22,32 +23,13 @@ test('define() is backward compatible', t => {
     ])
   )
 
-  const req = new http.request(
-    {
-      host: 'example.com',
-      port: 12345,
-      method: 'GET',
-      path: '/',
-    },
-    res => {
-      t.equal(res.statusCode, 500)
-
-      res.once('end', () => {
-        t.end()
-      })
-      // Streams start in 'paused' mode and must be started.
-      // See https://nodejs.org/api/stream.html#stream_class_stream_readable
-      res.resume()
+  await assertRejects(
+    got('http://example.com:12345/', { retry: 0 }),
+    ({ statusCode }) => {
+      t.is(statusCode, 500)
+      return true
     }
   )
-
-  req.on('error', err => {
-    // This should never happen.
-    t.fail('Error should never occur.')
-    t.end()
-  })
-
-  req.end()
 })
 
 test('define() throws when reply is not a numeric string', t => {
@@ -176,13 +158,14 @@ test('define() works with non-JSON responses', async t => {
   })
 
   t.equal(statusCode, 200)
-  // TODO: `body` should be a buffer. This seems to be a bug.
+  // TODO: beacuse `{ encoding: false }` is passed to `got`, `body` should be
+  // a buffer, but it's a string. Is this a bug in nock or got?
   t.equal(body, exampleResponseBody)
 })
 
-// TODO: There seems to be a bug here. When testing via `got` with `{
-// encoding: false }` the body that comes back should be a buffer, but is not.
-// It's difficult to get this test to pass after porting it.
+// TODO: There seems to be a bug here. When testing via `got` with
+// `{ encoding: false }` the body that comes back should be a buffer, but is
+// not. It's difficult to get this test to pass after porting it.
 test('define() works with binary buffers', t => {
   const exampleBody = '8001'
   const exampleResponse = '8001'

tests/test_delay.js

@@ -123,32 +123,22 @@ test('calling delay with "body" and "head" delays the response', t => {
   })
 })
 
-test('calling delay with "body" delays the response body', t => {
-  // Do not base new tests on this one. Write async tests using
-  // `resolvesInAtLeast` instead.
-  checkDuration(t, 100)
-
-  nock('http://example.test')
+test('calling delay with "body" delays the response body', async t => {
+  const scope = nock('http://example.test')
     .get('/')
-    .delay({
-      body: 100,
-    })
+    .delay({ body: 100 })
     .reply(200, 'OK')
 
-  http.get('http://example.test', function(res) {
-    res.setEncoding('utf8')
-
-    let body = ''
-
-    res.on('data', function(chunk) {
-      body += chunk
-    })
-
-    res.once('end', function() {
+  await resolvesInAtLeast(
+    t,
+    async () => {
+      const { body } = await got('http://example.test/')
       t.equal(body, 'OK')
-      t.end()
-    })
-  })
+    },
+    100
+  )
+
+  scope.done()
 })
 
 test('calling delayBody delays the response', async t => {
@@ -230,32 +220,26 @@ test('delayBody works with a delayed stream', async t => {
   scope.done()
 })
 
-test('calling delay delays the response', t => {
-  // Do not base new tests on this one. Write async tests using
-  // `resolvesInAtLeast` instead.
-  checkDuration(t, 100)
-
-  nock('http://example.test')
+test('calling delay delays the response', async t => {
+  const scope = nock('http://example.test')
     .get('/')
     .delay(100)
     .reply(200, 'OK')
 
-  http.get('http://example.test', function(res) {
-    res.setEncoding('utf8')
-
-    let body = ''
-
-    res.on('data', function(chunk) {
-      body += chunk
-    })
-
-    res.once('end', function() {
+  await resolvesInAtLeast(
+    t,
+    async () => {
+      const { body } = await got('http://example.test/')
       t.equal(body, 'OK')
-      t.end()
-    })
-  })
+    },
+    100
+  )
+
+  scope.done()
 })
 
+// TODO: This test is difficult to port to got. It's not clear why the request
+// matches given that there's a request body that isn't specified by the mock.
 test('using reply callback with delay provides proper arguments', t => {
   nock('http://localhost')
     .get('/')