fix(define): Throw error when legacy reply is in wrong format

paulmelnikow · gr2m · commit 5d2fb9fc1e49 · 2019-05-01T20:25:24.000-07:00
Shockingly, `_.isNumber(NaN)` is `true` which means this conditional check didn’t even work as intended.

Testing error paths is important!
diff --git a/lib/scope.js b/lib/scope.js
@@ -268,14 +268,14 @@ function load(path) {
 }
 
 function getStatusFromDefinition(nockDef) {
-  //  Backward compatibility for when `status` was encoded as string in `reply`.
+  // Backward compatibility for when `status` was encoded as string in `reply`.
   if (!_.isUndefined(nockDef.reply)) {
-    //  Try parsing `reply` property.
     const parsedReply = parseInt(nockDef.reply, 10)
-    if (_.isNumber(parsedReply)) {
-      return parsedReply
+    if (isNaN(parsedReply)) {
+      throw Error('`reply`, when present, must be a numeric string')
     }
-    // TODO-coverage: `else` throw with an error.
+
+    return parsedReply
   }
 
   const DEFAULT_STATUS_OK = 200
diff --git a/tests/test_define.js b/tests/test_define.js
@@ -50,6 +50,24 @@ test('define() is backward compatible', t => {
   req.end()
 })
 
+test('define() throws when reply is not a numeric string', t => {
+  t.throws(
+    () =>
+      nock.define([
+        {
+          scope: 'http://example.com:1451',
+          method: 'GET',
+          path: '/',
+          reply: 'frodo',
+        },
+      ]),
+    {
+      message: '`reply`, when present, must be a numeric string',
+    }
+  )
+  t.end()
+})
+
 test('define() applies default status code when none is specified', async t => {
   const body = '�'
 

Original file line number	Diff line number	Diff line change
`@@ -268,14 +268,14 @@ function load(path) {`
`268`	`268`	`}`
`269`	`269`
`270`	`270`	`function getStatusFromDefinition(nockDef) {`
`271`		- // Backward compatibility for when `status` was encoded as string in `reply`.
	`271`	+ // Backward compatibility for when `status` was encoded as string in `reply`.
`272`	`272`	`if (!_.isUndefined(nockDef.reply)) {`
`273`		- // Try parsing `reply` property.
`274`	`273`	`const parsedReply = parseInt(nockDef.reply, 10)`
`275`		`- if (_.isNumber(parsedReply)) {`
`276`		`- return parsedReply`
	`274`	`+ if (isNaN(parsedReply)) {`
	`275`	+ throw Error('`reply`, when present, must be a numeric string')
`277`	`276`	`}`
`278`		- // TODO-coverage: `else` throw with an error.
	`277`	`+`
	`278`	`+ return parsedReply`
`279`	`279`	`}`
`280`	`280`
`281`	`281`	`const DEFAULT_STATUS_OK = 200`