Closed
Bug 1194076
Opened 7 years ago
Closed 7 years ago
replace ffprocess.py code with psutil API calls
Categories
(Testing :: Talos, defect)
Testing
Talos
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: parkouss, Assigned: parkouss)
References
Details
Attachments
(1 file)
28.21 KB,
patch
|
jmaher
:
review+
|
Details | Diff | Splinter Review |
ffprocess.py will be obsolete when Talos will require psutil. We should rewrite the functionalities using psutil.
Comment 1•7 years ago
|
||
now we can tackle this !
Assignee | ||
Comment 2•7 years ago
|
||
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.
Comment 3•7 years ago
|
||
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+
Assignee | ||
Comment 4•7 years ago
|
||
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.
Assignee | ||
Comment 5•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=288c117b2be8
Assignee | ||
Comment 6•7 years ago
|
||
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.
Assignee | ||
Comment 7•7 years ago
|
||
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.
Description
•