Remove mocket

[shadromani]

No description provided.

[oschwald]

Nice! I had a few comments.

[oschwald]

We should specify the method on all of the expect_request calls as the default is to allow all methods.

[shadromani]

Will you please elaborate how? The test has different responses depending on the specific ip and we can not return the default on all requests.

[oschwald]

I don't know if I completely understand the question, but I am referring to the HTTP method. If we don't set it explicitly to GET, any method will be allowed. To do so, I think you would add something like this to each of them:

Suggested change

	self.httpserver.expect_request("/geoip/v2.1/country/1.2.3.4").respond_with_json(
	self.httpserver.expect_request("/geoip/v2.1/country/1.2.3.4", method="GET").respond_with_json(

This would be equivalent to httpretty.GET in the previous version.

[shadromani]

I see, I misread your comment.

[oschwald]

It would be better to return a valid JSON response. I think if you use the header matchers, this will be easy to do with respond_with_json.

[oschwald]

The custom handler is a bit confusing. I think we could eliminate it by using headers or header_value_matcher to match the expected headers on expect_request. I don't think we need to test the path again.

[oschwald]

I don't think this is the correct usage of HeaderValueMatcher. I think you still set headers, but you would want to define a custom matcher for "User-Agent".

See the docs.

[oschwald]

I don't think this works either. If I change the values, the tests don't fail. I was expecting something like this:

        def user_agent_compare(actual: str, expected: str) -> bool:
            if actual is None:
                return False
            return actual.startswith(expected)

        self.httpserver.expect_request(
            "/geoip/v2.1/country/1.2.3.4",
            method="GET",
            headers={
                    "Accept": "application/json",
                    "Authorization": "Basic NDI6YWJjZGVmMTIzNDU2",
                    "User-Agent": "GeoIP2-Python-Client/"
            },
            header_value_matcher=HeaderValueMatcher(defaultdict(
                lambda: HeaderValueMatcher.default_header_value_matcher,
                {"User-Agent": user_agent_compare}
            )),
        ).respond_with_json(
            self.country,
            status=200,
            content_type=self._content_type("country"),
        )

Alternatively, you could modify the default matcher as directed in the docs.

[deronnax]

I'm curious, what's the rational behind the switch from mocket to http-server?

[oschwald]

We unfortunately had quite a few problem with it breaking, e.g., on new or old Python versions, on new aiohttp versions, on particular OSes, etc. If you search the pull requests for "mocket", you will see a subset of those.

We decided to use pytest-httpserver as it implements an actual HTTP server, which should be less prone to these sorts of issues.