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

Request/response mocking for testing purposes #109

Open
amotl opened this issue Apr 21, 2024 · 6 comments
Open

Request/response mocking for testing purposes #109

amotl opened this issue Apr 21, 2024 · 6 comments

Comments

@amotl
Copy link

amotl commented Apr 21, 2024

Dear @Ousret,

we just tried to use the responses package together with niquests, and obviously it does not work out of the box. Maybe there is an easy solution?

With kind regards,
Andreas.

@amotl
Copy link
Author

amotl commented Apr 21, 2024

Ah, right. I am now using initialization code like this in pytest's conftest.py. It perfectly works around the problem.

import pytest


@pytest.fixture(scope="session", autouse=True)
def niquests_patch_all():
    from sys import modules

    import niquests
    import urllib3

    # Amalgamate the module namespace to make all modules aiming
    # to use `requests`, in fact use `niquests` instead.
    modules["requests"] = niquests
    modules["requests.adapters"] = niquests.adapters
    modules["requests.sessions"] = niquests.sessions
    modules["requests.exceptions"] = niquests.exceptions
    modules["requests.packages.urllib3"] = urllib3

-- grafana-toolbox/grafana-wtf#130 (review)

Maybe the documentation can be improved, by providing corresponding guidance?

@Ousret
Copy link

Ousret commented Apr 21, 2024

Hello,

Maybe you've missed it, but we did write about this https://niquests.readthedocs.io/en/latest/dev/migrate.html#maintainer-migration

regards,

@amotl
Copy link
Author

amotl commented Apr 21, 2024

Thank you very much for the reminder, I knew we had this on the radar just recently.

Apparently, I haven't been able to find it back, because the page lacks corresponding keywords, and response has a typo, should be called responses.

When iterating on the documentation next time, may I suggest to also add keywords like "patch" and/or "monkeypatch" to the Migration Guide documentation section?

I may be coming from a different school of thought, more thinking in terms of integration instead of migration. In this case, I approached the documentation in a way to find about about integration capabilities with the responses package.

In the same spirit, I think the page Recommended Packages and Extensions could also use the keyword "integration" somewhere on it, to make it easier to discover using documentation search, when using a different kind of jargon.

Personally, I would expect all those items to be presented on a single page. I guess I would finally have discovered it by using "pytest" as a search term, instead of complaining here right away. However, I thought that Niquests, being an excellent package already, also needs excellent documentation, even in those nit-picky areas, so I wanted to add my two cents, in order to convey our perspectives on how we approached the documentation.

In this case, corresponding improvements primarily should serve people like us, who are writing software tests on a daily basis, sometimes for libraries only rarely maintained. In such situations, any kinds of helper tools are always well appreciated, and, in this case, would make migration to Niquests easier, if it would transparently integrate with tests suites better, without actually changing any code.

@amotl
Copy link
Author

amotl commented Apr 22, 2024

As you can see, we are really strong proponents for adding this monkeypatching code snippet to the niquests package itself, so it becomes more re-usable for 3rd party applications, which use the niquests package directly, but also transitively.

If you are not comfortable adding this snippet the main module namespace, maybe you could consider adding it to niquests.testing, like how others are doing it, in order to support their downstream libraries and applications by providing corresponding testing helpers to them?

On the other hand, you already mentioned on another occasion that you are not comfortable adding this snippet to the code base. So, if we haven't been able to convince you up until now, we will surely stop bothering you about it ;].

@Ousret
Copy link

Ousret commented Apr 23, 2024

maybe you could consider adding it to niquests.testing, like how others are doing it, in order to support their downstream libraries and applications by providing corresponding testing helpers to them?

I'll think about it. And propose a implementation that would suit all of us. Will keep you posted.

regards,

edit: trying to see how "responses" can help to avoid complex patching utilities see getsentry/responses#710

@Ousret
Copy link

Ousret commented Apr 29, 2024

update: "responses" agreed to review a PR for Niquests optional support. I will propose a PR at my early.

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

No branches or pull requests

2 participants