Closed
Bug 1194076
Opened 10 years ago
Closed 10 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•10 years ago
|
||
now we can tackle this !
Assignee | ||
Comment 2•10 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•10 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•10 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•10 years ago
|
||
Assignee | ||
Comment 6•10 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•10 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: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•