Closed Bug 1194076 Opened 10 years ago Closed 10 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: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: