[NockBack] Record "unmatched" requests only. #1235
Conversation
| @@ -163,7 +163,6 @@ var record = { | |||
| recorder.clear(); | |||
| nock.cleanAll(); | |||
| nock.activate(); | |||
| nock.disableNetConnect(); | |||
There was a problem hiding this comment.
I don’t know why that was here before, but this change seems unrelated to your changes?
There was a problem hiding this comment.
It's actually related, otherwise you have to manually enable it in order for the requests to go through and be seen by the recorder.
The record mode should not disableNetConnect by default. For that behavior the other modes can be used.
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We try to do our best, but nock is maintained by volunteers and there is only so much we can do at a time. Thank you for your contributions. |
|
Any updates on this PR? I would like to be able to use mocks with recorder, for example I have a hard-to-reproduce request that I have managed to record and I want to write tests based on that request, changing small details of it. Now I either have to disable mocks to record new ones and use something else to mock the first request, which is not optimal. It would be nice if I could use the existing nock mock and record any subsequent requests for additional tests. |
|
You can install @guerrerocarlos’ fork for the time being |
|
I tried using the fork, but for some reason the recorder would not log any requests, although they we correctly passed through. Oh well, I can use Jest to mock the first request, but this would be a nice feature in Nock for sure. |
|
@guerrerocarlos are you still planning on seeing this through? If not, I'd still like to see this implemented and will work on this on my own fork. @gr2m I see that one of the failing tests is "recording turns off nock interception (backward compatibility behavior)" - do you have any context on what this means? Would this feature then be a breaking change? |
|
I don't think the feature would require a breaking change. If you'd like to send a new pull request based on the latest |
|
+1 for this feature... I'm trying to avoid implementing something similar on my own, as I've got the same need. |
|
@guerrerocarlos Any plans to finally merge this PR? |
|
This PR will need to be rebased on latest master before we can move forward. If anyone wants to give that a go and send a separate PR, we would be happy to review it |
|
For anyone looking, I'm working on this functionality for node-tdd in this pull request. @gr2m |
I took a stab at this in #2038 🙂 |
|
@gr2m @ollelauribostrom, rebased here: #2039 |
gr2m
added
Recorder
Reason for this PR
The record mode behavior does not meet the documentation:
Related Issue: #1115
This PR corrects it, by appending newly recorded nocks to existing ones.
Explanation of the code changes
Previously nock could wether intercept or record, but couldn't do both at the same time.
Now the record method is used a secondLayer during the intercept, effectively being able to do both at the same, so that when there are no interceptors for a requests, the recording mechanism takes it from there.