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

test: add cookie tests #142

Merged
merged 1 commit into from
Aug 5, 2020
Merged

Conversation

rwoll
Copy link
Member

@rwoll rwoll commented Aug 4, 2020

No description provided.

@coveralls
Copy link

coveralls commented Aug 4, 2020

Pull Request Test Coverage Report for Build 195514391

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 89.703%

Totals Coverage Status
Change from base Build 195321786: 0.1%
Covered Lines: 3624
Relevant Lines: 4040

💛 - Coveralls

@rwoll rwoll marked this pull request as ready for review August 5, 2020 00:33
async def test_should_roundtrip_cookie(context, page, server):
await page.goto(server.EMPTY_PAGE)
# @see https://en.wikipedia.org/wiki/Year_2038_problem
date = int(datetime.datetime(2038, 1, 1).timestamp() * 1000)
Copy link
Member Author

Choose a reason for hiding this comment

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

I manually checked this matches the date from the JS version, but someone should double check since dates are always fun!

Comment on lines +93 to +107
page_1 = await context.newPage()
await page_1.goto(server.PREFIX + "/setcookie.html")
##
page_2 = await context.newPage()
await page_2.goto(server.EMPTY_PAGE)
cookies_2 = await context.cookies()
assert len(cookies_2) == 1
assert ",".join(list(map(lambda c: c["value"], cookies_2))) == "value"
##
context_b = await browser.newContext()
page_3 = await context_b.newPage()
await page_3.goto(server.EMPTY_PAGE)
cookies_3 = await context_b.cookies()
assert cookies_3 == []
await context_b.close()
Copy link
Member Author

Choose a reason for hiding this comment

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

The javascript version used scoping; I've manually appended subscripts to (hopefully) achieve the same thing. The few tests with ## should be double checked by someone else to ensure the logic is preserved. 👀

cookie = []

def handler(request):
cookie.extend(request.requestHeaders.getRawHeaders("cookie") or [])
Copy link
Member Author

Choose a reason for hiding this comment

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

In the JS version, cookie was simply assigned to. In Python, that didn't work so I used a mutable list.

@rwoll
Copy link
Member Author

rwoll commented Aug 5, 2020

@mxschmitt and @pavelfeldman: This is now ready for review. Like the other PRs, this is almost a 1:1 port from the JS. I've left a few comments where there should be some extra eyes in the cases that the 1:1 port had to be modified slightly.

@pavelfeldman pavelfeldman merged commit 49b691a into microsoft:master Aug 5, 2020
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

4 participants