Bug 222418 - [LayoutTests] Convert http/tests/navigation convert PHP to Python
Summary: [LayoutTests] Convert http/tests/navigation convert PHP to Python
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Gambrell
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-02-25 08:07 PST by Chris Gambrell
Modified: 2021-04-08 14:27 PDT (History)
7 users (show)

See Also:


Attachments
Patch (75.10 KB, patch)
2021-02-25 08:11 PST, Chris Gambrell
no flags Details | Formatted Diff | Diff
Patch (106.72 KB, patch)
2021-03-01 15:24 PST, Chris Gambrell
no flags Details | Formatted Diff | Diff
Patch (102.42 KB, patch)
2021-03-01 15:35 PST, Chris Gambrell
no flags Details | Formatted Diff | Diff
Patch (98.70 KB, patch)
2021-03-01 15:46 PST, Chris Gambrell
no flags Details | Formatted Diff | Diff
Patch (93.55 KB, patch)
2021-03-01 16:31 PST, Chris Gambrell
no flags Details | Formatted Diff | Diff
Patch (123.56 KB, patch)
2021-03-01 17:18 PST, Chris Gambrell
no flags Details | Formatted Diff | Diff
Patch (121.51 KB, patch)
2021-03-02 14:18 PST, Chris Gambrell
no flags Details | Formatted Diff | Diff
Patch (66.18 KB, patch)
2021-04-05 10:00 PDT, Chris Gambrell
no flags Details | Formatted Diff | Diff
Patch (55.04 KB, patch)
2021-04-05 11:03 PDT, Chris Gambrell
no flags Details | Formatted Diff | Diff
Patch (54.44 KB, patch)
2021-04-08 12:39 PDT, Chris Gambrell
jbedard: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Gambrell 2021-02-25 08:07:27 PST
Replacing PHP with equivalent Python CGI scripts
Comment 1 Radar WebKit Bug Importer 2021-02-25 08:07:44 PST
<rdar://problem/74744523>
Comment 2 Chris Gambrell 2021-02-25 08:11:55 PST
Created attachment 421527 [details]
Patch
Comment 3 Chris Gambrell 2021-03-01 15:24:22 PST
Created attachment 421876 [details]
Patch
Comment 4 Chris Gambrell 2021-03-01 15:35:18 PST
Created attachment 421879 [details]
Patch
Comment 5 Chris Gambrell 2021-03-01 15:46:00 PST
Created attachment 421880 [details]
Patch
Comment 6 Chris Gambrell 2021-03-01 16:31:43 PST
Created attachment 421885 [details]
Patch
Comment 7 Chris Gambrell 2021-03-01 17:18:33 PST
Created attachment 421891 [details]
Patch
Comment 8 Jonathan Bedard 2021-03-02 11:36:33 PST
Comment on attachment 421891 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=421891&action=review

> LayoutTests/http/tests/navigation/post-redirect-get-reload.py:15
> +else:

Can we do an early exit instead of an else here?

> LayoutTests/http/tests/navigation/resources/redirect-on-back-updates-history-item.py:27
> +    sys.stdout.write(

Can we do an early exit instead of else here?

> LayoutTests/http/tests/navigation/resources/redirect-on-reload-updates-history-item.py:63
> +    )

Nit: Extra whitespace here

> LayoutTests/http/tests/navigation/resources/randomredirects/0.py:10
> +    headers[headername[4:].lower().replace('_', '-')] = headervalue

This should be '5:'

I think this will be easier if you just check the environment for 'HTTP_IF_MODIFIED_SINCE' in line 12 and ditch this loop entirely.

> LayoutTests/http/tests/navigation/resources/randomredirects/1.py:10
> +    headers[headername[4:].lower().replace('_', '-')] = headervalue

Ditto.

> LayoutTests/http/tests/navigation/resources/randomredirects/2.py:10
> +    headers[headername[4:].lower().replace('_', '-')] = headervalue

Ditto.

> LayoutTests/http/tests/navigation/resources/randomredirects/3.py:10
> +    headers[headername[4:].lower().replace('_', '-')] = headervalue

Ditto.

> LayoutTests/http/tests/navigation/resources/randomredirects/4.py:10
> +    headers[headername[4:].lower().replace('_', '-')] = headervalue

Ditto.

> LayoutTests/http/tests/navigation/resources/randomredirects/5.py:10
> +    headers[headername[4:].lower().replace('_', '-')] = headervalue

Ditto.

> LayoutTests/http/tests/navigation/resources/randomredirects/6.py:10
> +    headers[headername[4:].lower().replace('_', '-')] = headervalue

Ditto.

> LayoutTests/http/tests/navigation/resources/randomredirects/7.py:10
> +    headers[headername[4:].lower().replace('_', '-')] = headervalue

Ditto.

> LayoutTests/http/tests/navigation/resources/randomredirects/8.py:10
> +    headers[headername[4:].lower().replace('_', '-')] = headervalue

Ditto.

> LayoutTests/http/tests/navigation/resources/randomredirects/9.py:10
> +    headers[headername[4:].lower().replace('_', '-')] = headervalue

Ditto.
Comment 9 Chris Gambrell 2021-03-02 14:18:07 PST
Created attachment 421997 [details]
Patch
Comment 10 EWS 2021-03-02 18:20:56 PST
Committed r273781: <https://commits.webkit.org/r273781>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 421997 [details].
Comment 11 Chris Gambrell 2021-04-05 09:51:40 PDT
Reopening for second pass at .php files
Comment 12 Chris Gambrell 2021-04-05 10:00:18 PDT
Created attachment 425165 [details]
Patch
Comment 13 Chris Gambrell 2021-04-05 11:03:37 PDT
Created attachment 425174 [details]
Patch
Comment 14 Chris Gambrell 2021-04-05 11:07:18 PDT
Comment on attachment 425174 [details]
Patch

There were issues with apply-patch and the files
* LayoutTests/http/tests/navigation/no-referrer-reset.html
* LayoutTests/http/tests/navigation/no-referrer-same-window.html
* LayoutTests/http/tests/navigation/no-referrer-subframe.html
* LayoutTests/http/tests/navigation/no-referrer-target-blank.html

which contains the new py files
* LayoutTests/http/tests/navigation/resources/no-referrer-reset-helper.py
* LayoutTests/http/tests/navigation/resources/no-referrer-same-window-helper.py
* LayoutTests/http/tests/navigation/resources/no-referrer-helper.py

Reverted the tests for now back to the original with the .php versions of the scripts but also included the .py files for review. macwk1, macwk2, and iOS all pass locally on my machine with the new py versions. 

Once r+ I will change the tests back to the .py versions and remove the .php files to keep apply-patch happy.
Comment 15 Jonathan Bedard 2021-04-08 11:27:18 PDT
Comment on attachment 425174 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=425174&action=review

> LayoutTests/http/tests/navigation/resources/check-ping.py:10
> +sys.path.insert(0, http_root)

I don't see us using this.

> LayoutTests/http/tests/navigation/resources/delete-ping.py:9
> +sys.path.insert(0, http_root)

I don't see us using this.

> LayoutTests/http/tests/navigation/resources/no-referrer-helper.py:1
> +#!/usr/bin/env python3

Are we still using http/tests/navigation/resources/no-referrer-helper.php?

> LayoutTests/http/tests/navigation/resources/no-referrer-reset-helper.py:1
> +#!/usr/bin/env python3

Are we still using http/tests/navigation/resources/no-referrer-reset-helper.php?

> LayoutTests/http/tests/navigation/resources/no-referrer-same-window-helper.py:1
> +#!/usr/bin/env python3

Are we still using http/tests/navigation/resources/no-referrer-same-window-helper.php?

> LayoutTests/http/tests/navigation/resources/ping_file_path.py:10
> +sys.path.insert(0, http_root)

I don't see us using this.

> LayoutTests/http/tests/navigation/resources/redirected-post-request-contents.py:40
> +if content is not None and content == 'true':

Can just do "content == 'true'"
Comment 16 Chris Gambrell 2021-04-08 12:39:20 PDT
Created attachment 425532 [details]
Patch
Comment 17 Chris Gambrell 2021-04-08 12:40:23 PDT
(In reply to Jonathan Bedard from comment #15)
> Comment on attachment 425174 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=425174&action=review
> 
> > LayoutTests/http/tests/navigation/resources/check-ping.py:10
> > +sys.path.insert(0, http_root)
> 
> I don't see us using this.
> 
> > LayoutTests/http/tests/navigation/resources/delete-ping.py:9
> > +sys.path.insert(0, http_root)
> 
> I don't see us using this.
> 
> > LayoutTests/http/tests/navigation/resources/no-referrer-helper.py:1
> > +#!/usr/bin/env python3
> 
> Are we still using http/tests/navigation/resources/no-referrer-helper.php?
> 
> > LayoutTests/http/tests/navigation/resources/no-referrer-reset-helper.py:1
> > +#!/usr/bin/env python3
> 
> Are we still using
> http/tests/navigation/resources/no-referrer-reset-helper.php?
> 
> > LayoutTests/http/tests/navigation/resources/no-referrer-same-window-helper.py:1
> > +#!/usr/bin/env python3
> 
> Are we still using
> http/tests/navigation/resources/no-referrer-same-window-helper.php?
> 
> > LayoutTests/http/tests/navigation/resources/ping_file_path.py:10
> > +sys.path.insert(0, http_root)
> 
> I don't see us using this.
> 
> > LayoutTests/http/tests/navigation/resources/redirected-post-request-contents.py:40
> > +if content is not None and content == 'true':
> 
> Can just do "content == 'true'"

See comment 14 for why some PHP have not yet been removed
Comment 18 Chris Gambrell 2021-04-08 14:27:48 PDT
Committed r275677 (236313@main): <https://commits.webkit.org/236313@main>