Converting all php in xmlhttprequest to Python to gauge the difficulty of doing this across all layout tests.
<rdar://problem/73630008>
Created attachment 418472 [details] Patch
Created attachment 418562 [details] Patch
Created attachment 418599 [details] Patch
(In reply to Jonathan Bedard from comment #4) > Created attachment 418599 [details] > Patch The test failures caused by this change are because Mojave does not have Python 3. We’re about to update our Mac EWS bots, though.
Created attachment 418761 [details] Patch
Created attachment 418889 [details] Patch
Created attachment 418926 [details] Patch
Created attachment 419015 [details] Patch
Created attachment 419220 [details] Patch
Comment on attachment 419220 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=419220&action=review Non-reviewer review: I'll point out that we have Python-using copies of most of these in wpt/xhr today, as both are ultimately derived from the same written-in-PHP-from-Opera-Software test suite. So starting with XHR might not be the most valuable place to start! That said, this mostly looks good. > LayoutTests/http/tests/xmlhttprequest/null-auth.py:9 > +sys.stdout.write('Content-Type: text/html') > +sys.stdout.write('\r\n\r\n') > + > +print('<p>Test that null values in XHR login/password parameters are treated correctly.</p>') > +print('<p>No auth tokens should be sent with this request.</p>') I find the combination of sys.stdout.write and print somewhat surprising, even though I understand _why_ it's doing it (to ensure headers are terminated with CRLF, and not caring about the body of the response). Equally, it feels like in many cases the print could be of a triple-quoted multi-line string, rather than like this. Not sure how difficult/worthwhile making such changes is, though? > LayoutTests/http/tests/xmlhttprequest/resources/multipart-post-echo.py:16 > +for key in sorted(form.keys()): That this is sorting them is a notable change from the original PHP, and causes changed expectations. I can't recall quite how precisely this is specified, but it would be nice to avoid changing the order? (Especially when CPython 3.6+ preserves order with dicts, so the iteration order isn't at the mercy of the dict construction any more.) > LayoutTests/http/tests/local/formdata/send-form-data-constructed-from-form-using-open-panel-expected.txt:5 > -Response: submitted=true&checkbox=value2&radio=value3&text=value4&file=test.txt:text/plain:Hello > +Response: checkbox=value2&radio=value3&submitted=true&text=value4&file=test.txt:text/plain:Hello e.g., this change
Comment on attachment 419220 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=419220&action=review >> LayoutTests/http/tests/xmlhttprequest/null-auth.py:9 >> +print('<p>No auth tokens should be sent with this request.</p>') > > I find the combination of sys.stdout.write and print somewhat surprising, even though I understand _why_ it's doing it (to ensure headers are terminated with CRLF, and not caring about the body of the response). Equally, it feels like in many cases the print could be of a triple-quoted multi-line string, rather than like this. Not sure how difficult/worthwhile making such changes is, though? I'll upload a change that makes an attempt to standardize this...I think it's really going to be a question about style preferences. >> LayoutTests/http/tests/xmlhttprequest/resources/multipart-post-echo.py:16 >> +for key in sorted(form.keys()): > > That this is sorting them is a notable change from the original PHP, and causes changed expectations. I can't recall quite how precisely this is specified, but it would be nice to avoid changing the order? (Especially when CPython 3.6+ preserves order with dicts, so the iteration order isn't at the mercy of the dict construction any more.) Turns out the cgi module pretty much always sorts this (ie, key in sorted(form.keys()), key in form.keys(), key in form are all sorted) I'd sort of like to keep it sorted(form.keys()), so future contributors will immediately know how this is organized.
Created attachment 419442 [details] Patch
Created attachment 419453 [details] Patch
(In reply to Jonathan Bedard from comment #14) > Created attachment 419453 [details] > Patch Not a big fan of how the multi-line strings end up looking....
Comment on attachment 419442 [details] Patch Why not perl? r=me
(In reply to Alex Christensen from comment #16) > Comment on attachment 419442 [details] > Patch > > Why not perl? > r=me Because there are many folks who want to replace our perl with Python :)
Created attachment 419608 [details] Patch
Committed r272548: <https://commits.webkit.org/r272548> All reviewed patches have been landed. Closing bug and clearing flags on attachment 419608 [details].
Reopening to attach new patch.
Created attachment 419752 [details] Patch
Committed r272609: <https://commits.webkit.org/r272609> All reviewed patches have been landed. Closing bug and clearing flags on attachment 419752 [details].