Bug 223891 – [LayoutTests] Convert http/tests/cookies convert PHP to Python

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Chris Gambrell]

Replacing PHP with equivalent Python CGI scripts

[Radar WebKit Bug Importer]

<rdar://problem/75965634>

[Chris Gambrell]

Created attachment 424563 [details]
Patch

[Jonathan Bedard]

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?

[Chris Gambrell]

Created attachment 424673 [details]
Patch

[Chris Gambrell]

Created attachment 424679 [details]
Patch

[Jonathan Bedard]

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=`

[Chris Gambrell]

Committed r275315 (235996@main): <https://commits.webkit.org/235996@main>

[Chris Gambrell]

(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

[Chris Gambrell]

Reopening for the final PHP files that need to be converted

[Chris Gambrell]

Created attachment 425529 [details]
Patch

[Jonathan Bedard]

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.

[Chris Gambrell]

Created attachment 425554 [details]
Patch

[EWS]

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].