619410 - stopping conditions should be improved for mozmill 1.5.2

Status: RESOLVED FIXED

(Testing Graveyard :: Mozmill, defect)

Description

[Jeff Hammel]

The general stopping framework has been improved as a side-effect of
the restructuring have to do with manifests:

https://github.com/mozautomation/mozmill/commit/5a58949f9628d7ebbcbe64ede2674c2100a8ea0a

This should be backported to 1.5.2 to improve its robustness as well.

Comment 1

[Jeff Hammel]

Attachment: patch to improve things

I'm not convinced this is 100%, even ignoring the SIGINT vs SIGKILL in killableprocess.  Its a good port from master, but the control flow has changed a lot

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

The patch doesn't help for bug 615662, but fixes the problem reported on bug 619110.

Comment 3

[Jeff Hammel]

I'm not sure where our logic fails in 1.5.2 for stopping.  As a hacky fix, I would suggest putting in a SIGKILL if the process hasn't shut down (which *may*? fix bug 615662).  The control flow is much simpler on master: https://github.com/mozautomation/mozmill/blob/master/mozmill/mozmill/__init__.py#L347 I'm not sure if this is an issue there.  If it is, we should make mozrunner.cleanup more robust.

Comment 4

[Heather Arthur [:harth]]

Comment on attachment 497969 [details] [diff] [review]
patch to improve things

Definitely looks like an improvement, thanks.

Comment 5

[Jeff Hammel]

:ctalbert, :whimboo: okay to mozmill-1.5.2+ so I can land?

Comment 6

[cmtalbert]

(In reply to comment #5)
> :ctalbert, :whimboo: okay to mozmill-1.5.2+ so I can land?

yes, of course.

Comment 7

[Jeff Hammel]

pushed: https://github.com/mozautomation/mozmill/commit/81a9fe2fe1203dc120b760df6a6a6be792a0e145