Closed Bug 1194076 Opened 7 years ago Closed 7 years ago

replace ffprocess.py code with psutil API calls

Categories

(Testing :: Talos, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: parkouss, Assigned: parkouss)

References

Details

Attachments

(1 file)

ffprocess.py will be obsolete when Talos will require psutil. We should rewrite the functionalities using psutil.
now we can tackle this !
Attached patch 1194076.patchSplinter Review
So, this should do the job, and a bit more.

I have slightly changed the way we check if the browser is alive. This is now handled in talos_process.run_browser, so this ensure that after this call the browser was killed - or an exception is raised otherwise.

This allow to remove some things, mainly the big try/except we had in the ttest._run_test method.
Assignee: nobody → j.parkouss
Status: NEW → ASSIGNED
Attachment #8656433 - Flags: review?(jmaher)
Comment on attachment 8656433 [details] [diff] [review]
1194076.patch

Review of attachment 8656433 [details] [diff] [review]:
-----------------------------------------------------------------

only 1 small nit

::: talos/talos_process.py
@@ +31,5 @@
> +                self.process.wait(3)
> +            except psutil.TimeoutExpired:
> +                self.process.kill()
> +                # will raise TimeoutExpired if unable to kill
> +                self.process.wait(3)

shouldn't we wait, then try to kill again?
Attachment #8656433 - Flags: review?(jmaher) → review+
oh, the logic is:

try to kill nicely, wait three seconds max. If that fail, try to kill not nicely, wait again three seconds max. If that fail, an error is raised.

I took that from the psutil example https://pythonhosted.org/psutil/#psutil.wait_procs, I think we are good.
So far, pretty good!

We had one failure with damp. Browser crash, see http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jmaher@mozilla.com-288c117b2be8/try-linux/try_ubuntu32_hw_test-g2-e10s-bm105-tests1-linux-build17.txt.gz.

BTW this mean that our minidump is working! Nice to be sure of that. :)

So I will retrigger the failed job a few more times to investigate that issue. But anyway, this is unlikely related to that patch, so we are good to land it.
So I retried 5 times each g2 job for e10s on all platforms, no other issue so far.

So, landed that patch in https://hg.mozilla.org/build/talos/rev/3fa2c76a3fb7.

Maybe we could investigate this intermittent by re-triggering more here, or from another bug.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.