[rfc][icedtea-web] Kill processes in Integration Tests more cleanly
jvanek at redhat.com
Fri Apr 10 11:00:11 UTC 2015
On 04/09/2015 09:58 PM, Jie Kang wrote:
> This patch does four things:
> 1. Removes the usage of the softkiller : it no longer closes tabs properly and requires very old 3rd-party dependencies
> 2. Changes the signal sent from SIGINT to SIGUSR1 : mainly for firefox which doesn't handle any of the signals very well : https://bugzilla.mozilla.org/show_bug.cgi?id=336193
> Afaik the other browsers handle this signal by closing. I haven't been able to find any documentation on whether or not they close gracefully though.
> SIGTERM would be okay too
> 3. Removes the usage of p.destroy() : it may or may not forcibly kill the process depending on the implementation and with the signal sent above, shouldn't be necessary
> 4. Lowers the timeout time from 20 seconds to 10 seconds
> In general, it should make the suite go faster and remove issues that come from the browser and applet being destroyed without letting them clean up.
> I have run the reproducer suite on my machine (F21, openjdk 1.8) and there haven't been any new failing tests while these changes were applied. Technically, my timeout was at 5 seconds, not 10, but I think 10 is more reasonable given that some of the tests involve downloading from a remote source. To be honest though, I'd prefer it if each individual test was able to run in under 5 seconds...
> If others could take this patch, apply it and see how the reproducers fair for them, it'd be greatly appreciated.
Only two nits
Are you sure with timeout? I'm quite unsure about it... Well, lets try...
Please remove the whole build and sources and depndencies from configure related to softkiller.
You may do it in two changsets, as I would rather rereview the removal.
So ok to push for this one. And please prepare the removal.
I had not tested it, but Iknow you have beenstruggling with it for pretty lng time. So guess it
really helps. Also get rid of not standart killer would be probably useful.
More information about the distro-pkg-dev