Closed Bug 1091274 Opened 5 years ago Closed 4 years ago
Move leak log functions out of automationutils and into mozbase
We should move all the leak log related methods from automationutils into a mozbase module, probably mozrunner. Any other opinions?
Bug 1087474 and related discussion might suggest moztest as a candidate.
(In reply to Chris Manchester [:chmanchester] from comment #1) > Bug 1087474 and related discussion might suggest moztest as a candidate. Hmm, moztest is currently just a result container; this (and the stuff discussed in bug 1087474) are fairly orthogonal to that. Maybe we need a new module, ala mozautomation?
We had discussed making a Mozrunner subclass, maybe "TestRunner" and putting it in moztest as a place to hang common harness functionality off of.
Yeah, I think originally moztest was going to be the integration layer for many of the other mozbase modules into something resembling a bare bones test harness. The only reason it contains results is because that's what Mihnea started with, then priorities changed and/or he ran out of time and we never pursued it since. If we *really* wanted results to live in their own module, I'd vote we make mozresults.. but I think that is unnecessary.
Depends on: 1087474
Hm, not sure if I should ask you for a feedback here Jonathan or someone else - feel free to redirect the feedback. :) As suggested, this is a first attempt in leak log function integration into mozrunner subclasses.
Well more exacly this add a new method "process_leak_log" on BaseRunner, so it should be available on all runner subclasses. Tell me what you think about it, and how you would like to see it improved if needed - I know for example that this may fail due to a get_default_logger() that can return None, but I'm not sure how to get around this now. Anyway, feedback will be much appreciated!
Comment on attachment 8628505 [details] [diff] [review] 1091274.patch Eh, we are going in a different way! I asked :jgraham to look at this, and he found it looked like mozcrash. So, we are going to create a new mozleak package and use that (ala mozcrash). Steps for the new proposal: 1. create mozleak and move the functions into mozleak 2. use that, removing old stuff around. 3. create an appropriate structured logger to report leaks in a standardized way. It seems like :ted agreed on this. So I hope we will stay on this road (hehe)! Well it seems like the best approach we found until now, and I would be surprised if this time somebody complained about it. ;) So 1 and 2 may be handled with this bug (I think), and 3 should be a followup bug.
Assignee: nobody → j.parkouss
Status: NEW → ASSIGNED
Bug 1091274 - Move leak log functions out of automationutils and into mozbase. r?jgriffin
Attachment #8629219 - Flags: review?(jgriffin)
Hm, I added 'mozleak' into a bunch of files that I suppose are required to take the new mozleak package into account. It is based on a dxr "moztest" search.
Comment on attachment 8629219 [details] MozReview Request: Bug 1091274 - Move leak log functions out of automationutils and into mozbase. r?jgriffin https://reviewboard.mozilla.org/r/12537/#review11141 Looks good; we definitely should have a try run here first.
Attachment #8629219 - Flags: review?(jgriffin) → review+
Some fails because of undefined variable in reftest - fixed and re pushed: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0049826035c5
carrying r+ from the previous review done by :jgriffin. the try push was good. Tree is closed, so I will ask for checkin-needed.
You need to log in before you can comment on or make changes to this bug.