Summary: | [LayoutTests] Convert http/tests/cookies convert PHP to Python | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Gambrell <cgambrell> | ||||||||||||
Component: | Tools / Tests | Assignee: | Chris Gambrell <cgambrell> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | ap, ews-watchlist, hi, jbedard, ryanhaddad, webkit-bug-importer | ||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=220749 | ||||||||||||||
Attachments: |
|
Description
Chris Gambrell
2021-03-29 11:56:38 PDT
Created attachment 424563 [details]
Patch
Comment on attachment 424563 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424563&action=review > LayoutTests/http/tests/cookies/resources/cookie-utility.py:18 > + expires= datetime.utcnow() - timedelta(seconds=86400) Nit: Need space before = > LayoutTests/http/tests/cookies/resources/cookie-utility.py:29 > + sys.exit(0) With the 'else' block, we don't need sys.exit(0) > LayoutTests/http/tests/cookies/resources/cookie-utility.py:30 > +elif query_function == 'deleteCookies': For readability, I would put empty lines before each elif. > LayoutTests/http/tests/cookies/resources/set-cookie-and-serve.py:18 > +with open(os.path.join('/'.join(__file__.split('/')[0:-1]), destination), 'r') as file: Instead of using ".split", you should use "os.path.dirname". > LayoutTests/http/tests/cookies/resources/set-cookie-on-redirect.py:17 > +elif step == 2: Nit: Put empty lines before the elif for readability > LayoutTests/http/tests/cookies/resources/set-cookie-on-redirect.py:18 > + expires= datetime.utcnow() + timedelta(seconds=86400) Nit: Put space before = > LayoutTests/http/tests/cookies/same-site/resources/fetch-after-top-level-cross-origin-redirect.py:7 > +cookies = {} Not required, although I see us repeating this 7 line code a lot. Do you think if may be worth it to standardize this in LayoutTests/http/tests/resources/portabilityLayer.py? Created attachment 424673 [details]
Patch
Created attachment 424679 [details]
Patch
Comment on attachment 424679 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424679&action=review > LayoutTests/http/tests/cookies/resources/cookie-utility.py:36 > + expires= datetime.utcnow() + timedelta(seconds=86400) Might need to search through the whole patch for cases where you don't have a space in front of the = in `expires=` Committed r275315 (235996@main): <https://commits.webkit.org/235996@main> (In reply to Jonathan Bedard from comment #6) > Comment on attachment 424679 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=424679&action=review > > > LayoutTests/http/tests/cookies/resources/cookie-utility.py:36 > > + expires= datetime.utcnow() + timedelta(seconds=86400) > > Might need to search through the whole patch for cases where you don't have > a space in front of the = in `expires=` Quite a few instances- fixed and committed Reopening for the final PHP files that need to be converted Created attachment 425529 [details]
Patch
Comment on attachment 425529 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=425529&action=review > LayoutTests/http/tests/cookies/multiple-redirect-and-set-cookie.py:52 > +</html>'''.format(redirect_url(1))) Style checker might complain, but we need to organize this string to retain the indent, this code is hard to read without indenting the function contents. The multi-line sys.stdout that you do for headers is probably the right technique. Created attachment 425554 [details]
Patch
Committed r275762 (236340@main): <https://commits.webkit.org/236340@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 425554 [details]. |