Push persisted interceptors to the end instead of ignore on remove (#2350)

Michael Solomon · web-flow · commit 8aab603f2b88 · 2023-06-25T16:55:40.000-07:00
Currently, if you do:

```
nock(..).get('/').reply(200).persist();
nock(..).get('/').reply(400); // this will never get called
```
The second interceptor won't get called ever.

But sometimes in tests (queue for examples), I want to do:
```
beforeAll(() =&gt; {
  // some default behaviour
  nock(..).get('/').reply(200).persist();
});
test('should ...', () =&gt; {
  // test specific scenario for this same endpoint above
  nock(..).get('/').reply(400)
})
```

I find a workaround using the body matcher function:
```
const createInterceptor = () =&gt; {
  nock(..).get('/', () =&gt; {
    createInterceptor();
    return true;
  })
  .reply(200)
}
```
Which mimics the wanted behavior but is slower and way less elegant.
diff --git a/README.md b/README.md
@@ -1002,7 +1002,7 @@ const scope = nock('http://example.com')
   .reply(200, 'Persisting all the way')
 ```
 
-Note that while a persisted scope will always intercept the requests, it is considered "done" after the first interception.
+Note that while a persisted scope will always intercept the requests, it is considered "done" after the first interception, and they are pushed to the bottom of the stack after consumption.
 
 If you want to stop persisting an individual persisted mock you can call `persist(false)`:
 
diff --git a/lib/intercept.js b/lib/intercept.js
@@ -110,7 +110,7 @@ function addInterceptor(key, interceptor, scope, scopeOptions, host) {
 }
 
 function remove(interceptor) {
-  if (interceptor.__nock_scope.shouldPersist() || --interceptor.counter > 0) {
+  if (--interceptor.counter > 0) {
     return
   }
 
@@ -122,6 +122,12 @@ function remove(interceptor) {
   // matching instance. I'm also not sure why we couldn't delete _all_
   // matching instances.
   interceptors.some(function (thisInterceptor, i) {
+    if (interceptor.__nock_scope.shouldPersist()) {
+      return thisInterceptor === interceptor
+        ? interceptors.push(interceptors.splice(i, 1)[0])
+        : false
+    }
+
     return thisInterceptor === interceptor ? interceptors.splice(i, 1) : false
   })
 }
diff --git a/tests/test_persist_optionally.js b/tests/test_persist_optionally.js
@@ -144,6 +144,25 @@ describe('`persist()`', () => {
     expect(scope.isDone()).to.be.true()
   })
 
+  it('persisted mocks go to the bottom of the stack after matching', async () => {
+    const scope = nock('http://example.test')
+      .persist()
+      .get('/')
+      .reply(200, 'Persisting all the way')
+
+    const specificTestScope = nock('http://example.test')
+      .get('/')
+      .reply(201, 'return different response once')
+
+    await got('http://example.test/')
+
+    expect(scope.isDone()).to.be.true()
+
+    await got('http://example.test/')
+
+    expect(specificTestScope.isDone()).to.be.true()
+  })
+
   it('persisted mocks appear in `pendingMocks()`', async () => {
     const scope = nock('http://example.test')
       .get('/abc')

Original file line number	Diff line number	Diff line change
`@@ -110,7 +110,7 @@ function addInterceptor(key, interceptor, scope, scopeOptions, host) {`
`110`	`110`	`}`
`111`	`111`
`112`	`112`	`function remove(interceptor) {`
`113`		`- if (interceptor.__nock_scope.shouldPersist() \|\| --interceptor.counter > 0) {`
	`113`	`+ if (--interceptor.counter > 0) {`
`114`	`114`	`return`
`115`	`115`	`}`
`116`	`116`
`@@ -122,6 +122,12 @@ function remove(interceptor) {`
`122`	`122`	`// matching instance. I'm also not sure why we couldn't delete _all_`
`123`	`123`	`// matching instances.`
`124`	`124`	`interceptors.some(function (thisInterceptor, i) {`
	`125`	`+ if (interceptor.__nock_scope.shouldPersist()) {`
	`126`	`+ return thisInterceptor === interceptor`
	`127`	`+ ? interceptors.push(interceptors.splice(i, 1)[0])`
	`128`	`+ : false`
	`129`	`+ }`
	`130`	`+`
`125`	`131`	`return thisInterceptor === interceptor ? interceptors.splice(i, 1) : false`
`126`	`132`	`})`
`127`	`133`	`}`