Bug 226344

Summary: [run-webkit-tests] Move helper out of Port instance
Product: WebKit Reporter: Jonathan Bedard <jbedard>
Component: Tools / TestsAssignee: Jonathan Bedard <jbedard>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, dewei_zhu, ews-watchlist, glenn, slewis, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=226234
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Jonathan Bedard 2021-05-27 10:34:40 PDT 
Associating the helper process with Port instances doesn't make sense, since there should only ever be a single helper process, and that process should only be managed by the parent process of run-webkit-tests.
Comment 1 Radar WebKit Bug Importer 2021-05-27 10:34:59 PDT 
<rdar://problem/78575542>
Comment 2 Jonathan Bedard 2021-05-27 10:37:22 PDT 
Created attachment 429900 [details]
Patch
Comment 3 dewei_zhu 2021-05-27 15:52:15 PDT 
Comment on attachment 429900 [details]
Patch

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

> Tools/Scripts/webkitpy/port/base.py:963
>          it isn't, or it isn't available. If a port overrides start_helper()
>          it must override this routine as well."""

We may want to update the comment here because MacPort does not overwrite this routine.

> Tools/Scripts/webkitpy/port/base.py:964
> +        if Port.helper:

Do we want this to be 'self.helper', otherwise, MacPort.stop_helper() will do nothing.
Comment 4 Jonathan Bedard 2021-05-27 16:37:42 PDT 
Created attachment 429953 [details]
Patch
Comment 5 Jonathan Bedard 2021-05-27 16:39:55 PDT 
Created attachment 429954 [details]
Patch
Comment 6 Jonathan Bedard 2021-06-01 11:48:45 PDT 
Created attachment 430280 [details]
Patch
Comment 7 dewei_zhu 2021-06-01 15:20:25 PDT 
Comment on attachment 430280 [details]
Patch

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

r=me with comment.

> Tools/ChangeLog:12
> +        (Port.stop_helper): All classes should share the same code to stop the helper process.

Might worth mentioning we are doing this because popen object is not pickleable.
Comment 8 Jonathan Bedard 2021-06-01 15:42:54 PDT 
Created attachment 430300 [details]
Patch
Comment 9 EWS 2021-06-01 16:23:28 PDT 
Committed r278332 (238365@main): <https://commits.webkit.org/238365@main>

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