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

Type-check test_cookies.py #677

Merged
merged 1 commit into from
Dec 23, 2019
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 11 additions & 9 deletions tests/client/test_cookies.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,12 @@ async def send(
elif request.url.path.startswith("/set_cookie"):
headers = {"set-cookie": "example-name=example-value"}
return Response(200, headers=headers, request=request)
else:
raise NotImplementedError # pragma: no cover


@pytest.mark.asyncio
async def test_set_cookie():
async def test_set_cookie() -> None:
"""
Send a request including a cookie.
"""
Expand All @@ -40,7 +42,7 @@ async def test_set_cookie():


@pytest.mark.asyncio
async def test_set_cookie_with_cookiejar():
async def test_set_cookie_with_cookiejar() -> None:
"""
Send a request including a cookie, using a `CookieJar` instance.
"""
Expand All @@ -63,7 +65,7 @@ async def test_set_cookie_with_cookiejar():
discard=True,
comment=None,
comment_url=None,
rest={"HttpOnly": None},
rest={"HttpOnly": ""},
rfc2109=False,
)
cookies.set_cookie(cookie)
Expand All @@ -76,7 +78,7 @@ async def test_set_cookie_with_cookiejar():


@pytest.mark.asyncio
async def test_setting_client_cookies_to_cookiejar():
async def test_setting_client_cookies_to_cookiejar() -> None:
"""
Send a request including a cookie, using a `CookieJar` instance.
"""
Expand All @@ -99,21 +101,21 @@ async def test_setting_client_cookies_to_cookiejar():
discard=True,
comment=None,
comment_url=None,
rest={"HttpOnly": None},
rest={"HttpOnly": ""},
rfc2109=False,
)
cookies.set_cookie(cookie)

client = Client(dispatch=MockDispatch())
client.cookies = cookies
client.cookies = cookies # type: ignore
Copy link
Member Author

@florimondmanca florimondmanca Dec 21, 2019

Choose a reason for hiding this comment

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

mypy doesn't yet support setters that accept arguments of a different type than what is returned by the getter. The official recommandation is to # type: ignore for now. See: python/mypy#3004

Copy link
Member

Choose a reason for hiding this comment

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

Hrm. Well, it is a strange usage. Got me thinking that maybe we just shouldn’t be doing that in the first place...

Instead, we could enforce this style:

client.cookies = httpx.Cookies(...)

That’s possibly less surprising than setting the cookies to a dict, but having it return a Cookies instance.

We would probably want to hard check the type.

Obvs same would apply to the other setters too.

Thoughts?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, sure, that would sound good to me. :-)

Copy link
Member Author

Choose a reason for hiding this comment

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

-> #678

response = await client.get(url)

assert response.status_code == 200
assert response.json() == {"cookies": "example-name=example-value"}


@pytest.mark.asyncio
async def test_set_cookie_with_cookies_model():
async def test_set_cookie_with_cookies_model() -> None:
"""
Send a request including a cookie, using a `Cookies` instance.
"""
Expand All @@ -130,7 +132,7 @@ async def test_set_cookie_with_cookies_model():


@pytest.mark.asyncio
async def test_get_cookie():
async def test_get_cookie() -> None:
url = "http://example.org/set_cookie"

client = Client(dispatch=MockDispatch())
Expand All @@ -142,7 +144,7 @@ async def test_get_cookie():


@pytest.mark.asyncio
async def test_cookie_persistence():
async def test_cookie_persistence() -> None:
"""
Ensure that Client instances persist cookies between requests.
"""
Expand Down