Bug 169878 – webkitpy: Work around simctl launch returning dead processes

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Jonathan Bedard]

Sometimes, simctl launch will return the PID of a dead process.  Check that the process we return when launching an app is actually running.

[Jonathan Bedard]

Created attachment 304932 [details]
Patch

[Jonathan Bedard]

Created attachment 304935 [details]
Patch

[Daniel Bates]

Comment on attachment 304935 [details]
Patch

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

> Tools/ChangeLog:7
> +

Please explain the purpose of this change and the issue we are working around.

> Tools/ChangeLog:17
> +        (SimulatedDevice.launch_app): Check that the process being returned is active, use a timeout instead
> +        of attempts.

We tend to wrap ChangeLog lines at ~100 characters.

> Tools/Scripts/webkitpy/xcode/simulated_device.py:181
> +            if match and self.poll(int(match.group('pid'))) is None:

Please file a radar on CoreSimulator (if one does not already exist) to have "simctl launch" returns us a valid process id and add a comment to above this line that explains the reason we need to query the OS to see if the pid returned by "simctl launch_app" is valid as well as references the radar URL.

> Tools/Scripts/webkitpy/xcode/simulated_device.py:195
> +    def poll(self, pid):
> +        try:
> +            os.kill(pid, 0)
> +        except OSError as err:
> +            assert err.errno == errno.ESRCH
> +            return 1
> +        return None

I know that you are motivated to share this code with SimulatorProcess.Popen.poll(). This does not seem like the appropriate place for this function. I wish we could find a more appropriate place to put this.

[Jonathan Bedard]

Created attachment 304944 [details]
Patch

[Jonathan Bedard]

(In reply to comment #3)
> Comment on attachment 304935 [details]
> Patch
>
> > Tools/Scripts/webkitpy/xcode/simulated_device.py:195
> > +    def poll(self, pid):
> > +        try:
> > +            os.kill(pid, 0)
> > +        except OSError as err:
> > +            assert err.errno == errno.ESRCH
> > +            return 1
> > +        return None
> 
> I know that you are motivated to share this code with
> SimulatorProcess.Popen.poll(). This does not seem like the appropriate place
> for this function. I wish we could find a more appropriate place to put this.

I believe this is the appropriate place for this.  SimulatorProcess receives a device as an argument, it is this device which should define the behavior of poll.  The other option would be to define poll in the port class, but other Darwin ports either do not need a poll function or would define their respective polls differently.

[WebKit Commit Bot]

Comment on attachment 304944 [details]
Patch

Clearing flags on attachment: 304944

Committed r214192: <http://trac.webkit.org/changeset/214192>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.