222935 – [LayoutTests] Convert http/tests/cache convert PHP to Python

RESOLVED FIXED 222935

[LayoutTests] Convert http/tests/cache convert PHP to Python

https://bugs.webkit.org/show_bug.cgi?id=222935

Summary [LayoutTests] Convert http/tests/cache convert PHP to Python

Chris Gambrell

Reported 2021-03-08 12:53:25 PST

Replacing PHP with equivalent Python CGI scripts

Attachments
Patch (102.52 KB, patch) 2021-03-10 19:48 PST, Chris Gambrell	no flags	Details Formatted Diff Diff
Patch (99.93 KB, patch) 2021-03-10 19:58 PST, Chris Gambrell	no flags	Details Formatted Diff Diff
Patch (96.48 KB, patch) 2021-03-11 08:23 PST, Chris Gambrell	no flags	Details Formatted Diff Diff
Patch (113.04 KB, patch) 2021-03-11 15:08 PST, Chris Gambrell	no flags	Details Formatted Diff Diff
Patch (109.92 KB, patch) 2021-03-11 15:21 PST, Chris Gambrell	no flags	Details Formatted Diff Diff
Patch (109.90 KB, patch) 2021-03-12 09:13 PST, Chris Gambrell	no flags	Details Formatted Diff Diff
Patch (109.99 KB, patch) 2021-03-15 11:28 PDT, Chris Gambrell	no flags	Details Formatted Diff Diff
Patch (111.08 KB, patch) 2021-03-16 11:44 PDT, Chris Gambrell	no flags	Details Formatted Diff Diff
Patch (10.40 KB, patch) 2021-03-24 08:56 PDT, Chris Gambrell	no flags	Details Formatted Diff Diff
Patch (10.40 KB, patch) 2021-03-24 09:11 PDT, Chris Gambrell	no flags	Details Formatted Diff Diff
Patch (9.33 KB, patch) 2021-03-24 10:14 PDT, Chris Gambrell	no flags	Details Formatted Diff Diff
Patch (10.30 KB, patch) 2021-03-31 18:34 PDT, Chris Gambrell	no flags	Details Formatted Diff Diff
Patch (10.31 KB, patch) 2021-04-01 14:33 PDT, Chris Gambrell	jbedard: review+	Details Formatted Diff Diff
Show Obsolete (12) View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2021-03-08 12:54:37 PST

<rdar://problem/75183314>

Chris Gambrell

Comment 2 2021-03-10 19:48:42 PST

Created attachment 422893 [details] Patch

Chris Gambrell

Comment 3 2021-03-10 19:58:26 PST

Created attachment 422895 [details] Patch

Chris Gambrell

Comment 4 2021-03-11 08:23:58 PST

Created attachment 422933 [details] Patch

Chris Gambrell

Comment 5 2021-03-11 15:08:15 PST

Created attachment 422977 [details] Patch

Chris Gambrell

Comment 6 2021-03-11 15:21:03 PST

Created attachment 422981 [details] Patch

Chris Gambrell

Comment 7 2021-03-12 09:13:58 PST

Created attachment 423056 [details] Patch

Chris Gambrell

Comment 8 2021-03-15 11:28:01 PDT

Created attachment 423204 [details] Patch

Jonathan Bedard

Comment 9 2021-03-15 15:13:58 PDT

Comment on attachment 423204 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423204&action=review Biggest note is that I don't think our Windows bots have f strings available, although we can let EWS check that assumption > LayoutTests/http/tests/cache/post-redirect-get.php:-4 > -// This test loads an uncacheable main resource and a cacheable image subresource. We should keep this comment around > LayoutTests/http/tests/cache/post-redirect-get.py:12 > +sys.path.insert(0, http_root) No reason to do this if we aren't using any shared code > LayoutTests/http/tests/cache/post-redirect-get.py:27 > + f'Expires: {expires}\r\n' f strings are introduced in Python 3.6....I think our Windows bots are using 3.5 > LayoutTests/http/tests/cache/post-redirect-get.py:40 > + '<html>\r\n' \r\n isn't going to be harmful here, but is there a reason we're doing \r\n instead of just \n? > LayoutTests/http/tests/cache/post-with-cached-subresources.py:18 > +sys.path.insert(0, http_root) We don't need this since we aren't importing any shared code > LayoutTests/http/tests/cache/reload-main-resource.py:9 > +sys.path.insert(0, http_root) We don't need this since we aren't importing any shared code > LayoutTests/http/tests/cache/disk-cache/resources/redirect-chain.py:24 > + f'Location: redirect-chain.py?length={length}&uniqueId={unique_id}&random={randint(0, sys.maxsize)}\r\n\r\n' I think we need to avoid f strings because of Windows for the moment > LayoutTests/http/tests/cache/disk-cache/speculative-validation/resources/validation-request-frame.py:12 > + f'Set-Cookie: {cookie}\r\n\r\n' I think we should avoid f string for the sake of our Windows bots > LayoutTests/http/tests/cache/resources/cacheable-iframe.py:10 > + f'Cache-Control: public, max-age={max_age}\r\n' Ditto on the f strings > LayoutTests/http/tests/cache/resources/load-and-check-referer.py:7 > +def contentType(path): Existing function is fine as is, but might consider doing something like this: { 'html': 'text/html', 'manifest': 'text/cache-manifest', ... }.get(path.split('.')[-1], 'text/plain') > LayoutTests/http/tests/cache/resources/partitioned-cache-echo-state.py:9 > +sys.path.insert(0, http_root) We aren't using any shared code, so we don't need to edit the Python path > LayoutTests/http/tests/cache/resources/post-image-to-verify.py:1 > +#!/usr/bin/env python3 Where is the file we're replacing? > LayoutTests/http/tests/cache/resources/reload-main-resource-iframe.py:10 > +sys.path.insert(0, http_root) We aren't using any shared code, so we don't need to edit the Python path > LayoutTests/http/tests/cache/resources/stylesheet304.php:-21 > -?> Doesn't look live we've replaced this file. Is it no longer in use? > LayoutTests/http/tests/resources/touch-temp-file.py:1 > +#!/usr/bin/env python3 I don't see the file this is replacing. > LayoutTests/http/tests/resources/touch-temp-file.py:10 > +sys.path.insert(0, http_root) We aren't using any shared code, so we don't need to edit the Python path > LayoutTests/http/tests/resources/write-temp-file.py:1 > +#!/usr/bin/env python3 I don't see the file we're replacing. > LayoutTests/http/tests/resources/write-temp-file.py:10 > +sys.path.insert(0, http_root) We aren't using any shared code, so we don't need to edit the Python path

Chris Gambrell

Comment 10 2021-03-16 11:44:58 PDT

Created attachment 423370 [details] Patch

Chris Gambrell

Comment 11 2021-03-16 11:51:01 PDT

Comment on attachment 423370 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423370&action=review > LayoutTests/http/tests/cache/post-redirect-get.py:46 > + '<html>\n' Fixed \r\n to \n > LayoutTests/http/tests/cache/resources/load-and-check-referer.py:7 > +def contentType(path): Converted func to your dictionary approach. Got errors when running test including images. Will look closer into changing to a dictionary approach the second pass with this directory with the more challenging conversions. > LayoutTests/http/tests/cache/resources/post-image-to-verify.py:1 > +#!/usr/bin/env python3 Removing PHP equivalent. > LayoutTests/http/tests/cache/resources/stylesheet304.php:-1 > -<?php No need to replace. No longer in use > LayoutTests/http/tests/resources/touch-temp-file.py:1 > +#!/usr/bin/env python3 Still used by multiple .php files. This directory/tests are reference throughout http/tests so this will be the last directory for full conversion. > LayoutTests/http/tests/resources/write-temp-file.py:1 > +#!/usr/bin/env python3 Still used by multiple .php files. This directory/tests are reference throughout http/tests so this will be the last directory for full conversion.

Chris Gambrell

Comment 12 2021-03-16 11:52:24 PDT

(In reply to Chris Gambrell from comment #11) > Comment on attachment 423370 [details] > Patch > Waiting to convert the f strings back to regular strings with the .format modifier since previous patch passed EWS. If the conversion isn't needed, will leave it with f strings. If conversion is needed, I will push another patch.

Jonathan Bedard

Comment 13 2021-03-16 15:18:09 PDT

(In reply to Chris Gambrell from comment #12) > (In reply to Chris Gambrell from comment #11) > > Comment on attachment 423370 [details] > > Patch > > > > Waiting to convert the f strings back to regular strings with the .format > modifier since previous patch passed EWS. If the conversion isn't needed, > will leave it with f strings. If conversion is needed, I will push another > patch. Looks like I was wrong about the version of Python our Windows bots are running. The are running 3.6, we can use f-strings.

EWS

Comment 14 2021-03-17 11:56:09 PDT

Committed r274574: <https://commits.webkit.org/r274574> All reviewed patches have been landed. Closing bug and clearing flags on attachment 423370 [details].

Chris Gambrell

Comment 15 2021-03-24 08:46:06 PDT

Reopening to complete conversion of directory

Chris Gambrell

Comment 16 2021-03-24 08:56:55 PDT

Created attachment 424137 [details] Patch

Chris Gambrell

Comment 17 2021-03-24 09:11:09 PDT

Created attachment 424139 [details] Patch

Chris Gambrell

Comment 18 2021-03-24 10:14:43 PDT

Created attachment 424151 [details] Patch

Jonathan Bedard

Comment 19 2021-03-31 13:25:47 PDT

Comment on attachment 424151 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424151&action=review > LayoutTests/http/tests/cache/resources/cache-control-redirect.py:10 > +code = query.get('code', [None])[-1] Should replace None with 302 and drop the if statement on line 15 > LayoutTests/http/tests/cache/resources/x-frame-options.py:1 > +#!/usr/bin/env python3 I don't see the file we are replacing

Chris Gambrell

Comment 20 2021-03-31 18:34:25 PDT

Created attachment 424856 [details] Patch

Chris Gambrell

Comment 21 2021-03-31 18:34:55 PDT

(In reply to Jonathan Bedard from comment #19) > Comment on attachment 424151 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=424151&action=review > > > LayoutTests/http/tests/cache/resources/cache-control-redirect.py:10 > > +code = query.get('code', [None])[-1] > > Should replace None with 302 and drop the if statement on line 15 > > > LayoutTests/http/tests/cache/resources/x-frame-options.py:1 > > +#!/usr/bin/env python3 > > I don't see the file we are replacing Fixed in Comment 20

Chris Gambrell

Comment 22 2021-04-01 14:33:27 PDT

Created attachment 424949 [details] Patch

Chris Gambrell

Comment 23 2021-04-01 14:37:22 PDT

Comment on attachment 424949 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424949&action=review > LayoutTests/ChangeLog:13 > + * http/tests/cache/resources/x-frame-options.py: Added. PHP counterpart not removed because of an issue with it and apply-patch. When r+'d I will remove locally and land.

Jonathan Bedard

Comment 24 2021-04-01 15:39:08 PDT

Comment on attachment 424949 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424949&action=review > LayoutTests/http/tests/cache/resources/x-frame-options.py:15 > +one_year = 12 * 31 * 24 * 60 * 60 Super weird way to represent a year, we usually use 365 instead of 12 * 31

Chris Gambrell

Comment 25 2021-04-01 18:07:37 PDT

Committed r275399 (236063@main): <https://commits.webkit.org/236063@main>

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version WebKit Nightly Build

Hardware Unspecified

OS Unspecified

Product WebKit

Component Tools / Tests

Assignee

Chris Gambrell

Reported

2021-03-08 12:53 PST

Modified

2021-04-01 18:07 PDT History

CC List

6 users Show

URL

Keywords InRadar

Depends on

Blocks