Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mbeliaev/recorder #545

Merged
merged 19 commits into from Sep 1, 2022
Merged

Conversation

beliaev-maksim
Copy link
Collaborator

@beliaev-maksim beliaev-maksim commented Apr 14, 2022

propose beta feature for 0.22.0 to record responses to files using man in the middle

there are still steps to polish within this PR (types, docs, style, etc), but before doing this I need to confirm the idea.
General workflow looks like:

import requests
from responses import _recorder


def another():
    rsp = requests.get("https://httpstat.us/500")
    rsp = requests.get("https://httpstat.us/202")

@_recorder.record(file_path="out.yml")
def test_recorder():
    rsp = requests.get("https://httpstat.us/404")
    rsp = requests.get("https://httpstat.us/201")
    another()

which will product following out.yml file:

responses:
- response:
    auto_calculate_content_length: false
    body: ''
    content_type: text/plain
    headers: null
    method: GET
    status: 404
    url: https://httpstat.us/404
- response:
    auto_calculate_content_length: false
    body: ''
    content_type: text/plain
    headers: null
    method: GET
    status: 201
    url: https://httpstat.us/201
- response:
    auto_calculate_content_length: false
    body: ''
    content_type: text/plain
    headers: null
    method: GET
    status: 500
    url: https://httpstat.us/500
- response:
    auto_calculate_content_length: false
    body: ''
    content_type: text/plain
    headers: null
    method: GET
    status: 202
    url: https://httpstat.us/202

later I am going to extend the functionality and add possibility to load these yaml files

@codecov
Copy link

codecov bot commented Apr 14, 2022

Codecov Report

Merging #545 (f7f937e) into master (d49f829) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #545   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            7         9    +2     
  Lines         2644      2730   +86     
=========================================
+ Hits          2644      2730   +86     
Impacted Files Coverage Δ
responses/_recorder.py 100.00% <100.00%> (ø)
responses/tests/test_recorder.py 100.00% <100.00%> (ø)
responses/matchers.py 100.00% <0.00%> (ø)
responses/tests/test_matchers.py 100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

responses/registries.py Outdated Show resolved Hide resolved
responses/_recorder.py Outdated Show resolved Hide resolved
@beliaev-maksim
Copy link
Collaborator Author

@markstory
I removed redundant code by inheriting from RequestMock
also, switched to toml format and added type hints in the code

if we are fine with the idea of implementation, then I will add tests to increase coverage

so, following code

import requests
from responses import _recorder


def another():
    rsp = requests.get("https://httpstat.us/500")
    rsp = requests.get("https://httpstat.us/202")

@_recorder.record(file_path="out.toml")
def test_recorder():
    rsp = requests.get("https://httpstat.us/404")
    rsp = requests.get("https://httpbin.org/status/wrong")
    another()

will product next output in out.toml

[[responses]]

[responses.response]
method = "GET"
url = "https://httpstat.us/404"
body = "404 Not Found"
status = 404
content_type = "text/plain"
auto_calculate_content_length = false
[[responses]]

[responses.response]
method = "GET"
url = "https://httpbin.org/status/wrong"
body = "Invalid status code"
status = 400
content_type = "text/plain"
auto_calculate_content_length = false
[[responses]]

[responses.response]
method = "GET"
url = "https://httpstat.us/500"
body = "500 Internal Server Error"
status = 500
content_type = "text/plain"
auto_calculate_content_length = false
[[responses]]

[responses.response]
method = "GET"
url = "https://httpstat.us/202"
body = "202 Accepted"
status = 202
content_type = "text/plain"
auto_calculate_content_length = false

@@ -73,6 +79,31 @@ def replace(self, response: "BaseResponse") -> "BaseResponse":
self.registered[index] = response
return response

def _dump(self, destination: "io.IOBase") -> None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should dumping to a file be a separate thing? That would let userland code or us in the future change the serialization format that is being used to save response fixtures.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to create it in the registry because this will allow to dump existing tests to the file
Eg
If user ran responses.add() multiple times, then user can run responses.get_registry()._dump(*args)

That will allow safe (compared to brand new recording) transfer to the new method if required

In the future we can provide serializer argument, but at the moment I am a bit reluctant to support other serializers than one we decide

Also, user can also overwrite this method after inheriting

Or what are the other options to introduce this method?
As a separate function?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't dumping be on the Recorder()? I'm concerned that recording feels like it is half in the Recorder and half in the registry right now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@markstory
done!

this could be done even as a separate function. Then it will be possible to easily dump existing registries into the file

responses/registries.py Outdated Show resolved Hide resolved
@beliaev-maksim
Copy link
Collaborator Author

@markstory
can you please review?

@beliaev-maksim
Copy link
Collaborator Author

@markstory
Can you check pls?

@markstory markstory merged commit f431d49 into getsentry:master Sep 1, 2022
@beliaev-maksim beliaev-maksim deleted the mbeliaev/recorder branch September 1, 2022 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants