Bug 221940 - [LayoutTests] Convert http/tests/eventsource convert PHP to Python
Summary: [LayoutTests] Convert http/tests/eventsource 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: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-02-15 17:27 PST by Chris Gambrell
Modified: 2021-02-16 13:46 PST (History)
6 users (show)

See Also:


Attachments
Patch (33.35 KB, patch)
2021-02-15 17:45 PST, Chris Gambrell
no flags Details | Formatted Diff | Diff
Patch (33.40 KB, patch)
2021-02-16 10:27 PST, Chris Gambrell
no flags 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-15 17:27:21 PST
Replacing PHP with equivalent Python CGI scripts
Comment 1 Radar WebKit Bug Importer 2021-02-15 17:27:45 PST
<rdar://problem/74372782>
Comment 2 Chris Gambrell 2021-02-15 17:45:13 PST
Created attachment 420407 [details]
Patch
Comment 3 Jonathan Bedard 2021-02-16 09:36:46 PST
Comment on attachment 420407 [details]
Patch

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

> LayoutTests/http/tests/eventsource/resources/es-cors-basic.py:8
> +request_method = os.environ.get('REQUEST_METHOD')

I think we should do the request_method bit first.

> LayoutTests/http/tests/eventsource/resources/es-cors-basic.py:11
> +    sys.exit('Got unexpected preflight request')

Python's sys.exit() prints to stderr, PHP's die prints to stdout. So we really want this to be:

sys.stdout.write('Content-Type: text/html\r\n\r\n')
sys.stdout.write('Got unexpected preflight request\n')
sys.exit(0)

> LayoutTests/http/tests/eventsource/resources/es-cors-basic.py:24
> +

If the count is 0 or 1, we aren't ending the header

> LayoutTests/http/tests/eventsource/resources/es-cors-credentials.py:9
> +    sys.exit('Got unexpected preflight request')

Ditto on the sys.exit() vs die()

> LayoutTests/http/tests/eventsource/resources/es-cors-credentials.py:17
> +returned_from_header = False

Unused variable

> LayoutTests/http/tests/eventsource/resources/es-eof.py:11
> +    'id: {}\n'

I think we want to use named variables in this string, gets to be a bit tough to follow

> LayoutTests/http/tests/eventsource/resources/infinite-event-stream.py:15
> +    data = {"time": curDate}

Nit: Should use single quotes

> LayoutTests/http/tests/eventsource/resources/status-codes.py:18
> +elif status_code == 301 or status_code == 302 or status_code == 303 or status_code == 307:

Should probably do: status_code in [301, 302, 303, 307] instead
Comment 4 Chris Gambrell 2021-02-16 10:27:02 PST
Created attachment 420494 [details]
Patch
Comment 5 Chris Gambrell 2021-02-16 10:30:11 PST
(In reply to Jonathan Bedard from comment #3)
> Comment on attachment 420407 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=420407&action=review
> 
> > LayoutTests/http/tests/eventsource/resources/es-cors-basic.py:8
> > +request_method = os.environ.get('REQUEST_METHOD')
> 
> I think we should do the request_method bit first.
> 
> > LayoutTests/http/tests/eventsource/resources/es-cors-basic.py:11
> > +    sys.exit('Got unexpected preflight request')
> 
> Python's sys.exit() prints to stderr, PHP's die prints to stdout. So we
> really want this to be:
> 
> sys.stdout.write('Content-Type: text/html\r\n\r\n')
> sys.stdout.write('Got unexpected preflight request\n')
> sys.exit(0)
> 
> > LayoutTests/http/tests/eventsource/resources/es-cors-basic.py:24
> > +
> 
> If the count is 0 or 1, we aren't ending the header
> 
> > LayoutTests/http/tests/eventsource/resources/es-cors-credentials.py:9
> > +    sys.exit('Got unexpected preflight request')
> 
> Ditto on the sys.exit() vs die()
> 
> > LayoutTests/http/tests/eventsource/resources/es-cors-credentials.py:17
> > +returned_from_header = False
> 
> Unused variable
> 
> > LayoutTests/http/tests/eventsource/resources/es-eof.py:11
> > +    'id: {}\n'
> 
> I think we want to use named variables in this string, gets to be a bit
> tough to follow
> 
> > LayoutTests/http/tests/eventsource/resources/infinite-event-stream.py:15
> > +    data = {"time": curDate}
> 
> Nit: Should use single quotes
> 
> > LayoutTests/http/tests/eventsource/resources/status-codes.py:18
> > +elif status_code == 301 or status_code == 302 or status_code == 303 or status_code == 307:
> 
> Should probably do: status_code in [301, 302, 303, 307] instead

Fixed all of these in comment 4
Comment 6 EWS 2021-02-16 13:46:07 PST
Committed r272926: <https://commits.webkit.org/r272926>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 420494 [details].