Skip to content

Conversation

@OldSneerJaw
Copy link
Contributor

@OldSneerJaw OldSneerJaw commented May 11, 2021

Addresses #171

Improvements:

  • Adds the missing url parameter to the register_uri and request methods.
  • Renames the path parameters of delete, get, head, options, patch, post, and put methods to url to match the name of the corresponding parameter in requests_mock.adapter.Adapter.register_uri.
  • Changes the types of various method and url parameters to support the requests_mock.adapter.ANY "wildcard" value.
  • Introduces the AnyMatcher type for the requests_mock.adapter.ANY matcher (thanks, @pcorpet!).
  • Adds the request_headers and complete_qs request matching parameters to MockerCore's registration methods (e.g. register_uri, request, get`, etc.).
  • Adds the response_list parameter to MockerCore's registration methods so that it can be specified as a positional argument as in the documentation's examples.
  • Ensures that Adapter's register_uri method supports the same request matching parameters as MockerCore's register_uri.

For #171

Modifications to `MockerCore`:
- Adds the missing `url` parameter to the `register_uri` and `request` methods.
- Renames the `path` parameters of `delete`, `get`, `head`, `options`, `patch`, `post`, and `put` methods to `url` to match the name of the corresponding parameter in `requests_mock.adapter.Adapter.register_uri`.
- Changes the types of various `method` and `url` parameters to support the `requests_mock.adapter.ANY` "wildcard" value.
@jamielennox
Copy link
Owner

@pcorpet @palfrey

self,
method: str,
method: Union[str, Any],
url: Union[str, Pattern[str], Any],
Copy link
Owner

Choose a reason for hiding this comment

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

what's the point of union-ing with Any, does it give any hints over just using Any?

Copy link

Choose a reason for hiding this comment

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

AFAIK, no. Well, some tooling may go "hey, you might want to use str, but you can use Any", but mypy I don't think will care.

def get(
self,
path: Union[str, Pattern[str]],
url: Union[str, Pattern[str], Any],
Copy link
Owner

Choose a reason for hiding this comment

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

good catch.

@pcorpet
Copy link
Contributor

pcorpet commented May 11, 2021

I think the best move is to strongly type ANY as a NewType and reference the type everywhere.

In adapters.pyi:

AnyMatcher = typing.NewType('AnyMatcher', object)

ANY: AnyMatcher = ...

and then in mocker.pyi:

def get(
  self,
  url: Union[str, Pattern[str], AnyMatcher]

@OldSneerJaw OldSneerJaw changed the title Improve MockerCore type stubs Improve response matching type stubs May 11, 2021
@OldSneerJaw
Copy link
Contributor Author

I've pushed a new commit that includes @pcorpet's suggestion and adds some request matching parameters that were missing. There are also several response-related parameters missing (e.g. reason, cookies, json), plus the existing text parameter's type annotation doesn't currently support dynamic invocation. But I elected to treat those as out of scope of this pull request.

@OldSneerJaw OldSneerJaw changed the title Improve response matching type stubs Improve request matching type stubs May 11, 2021
For #171

Improvements:
- Introduces the `AnyMatcher` type for the `requests_mock.adapter.ANY` matcher.
- Adds the `request_headers` and `complete_qs` request matching parameters to `MockerCore`'s registration methods (e.g. `register_uri`, `request`, get`, etc.).
- Adds the `response_list` parameter to `MockerCore`'s registration methods so that it can be specified as a positional argument as in the [documentation](https://requests-mock.readthedocs.io/en/latest/response.html#response-lists)'s examples.
- Ensures that `Adapter`'s `register_uri` method supports the same request matching parameters as `MockerCore`'s `register_uri`.
@OldSneerJaw
Copy link
Contributor Author

@jamielennox: Any chance you might review and release this change soon? Let me know if you've got further questions or concerns I can help with.

@jamielennox
Copy link
Owner

Ah, sorry - i have been busy and haven't checked in until i got the notification.

Seems good to me, only thing i'd like (might not be possible) is the equivalent of a get(url, **, arg=) i as overly pedantic in making sure you couldn't just have a long list of positional arguments and it reads to me like the typing suggests that works.

I can release this as 1.9.3 as it's a minor, non-functional update.

@jamielennox jamielennox merged commit 1f4c3ae into jamielennox:master May 28, 2021
@OldSneerJaw OldSneerJaw deleted the mocker-type-stub branch May 28, 2021 02:55
@jamielennox
Copy link
Owner

released as 1.9.3 - thanks for the help and sorry it took a little while

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.

4 participants