Closed Bug 1091284 Opened 5 years ago Closed 4 years ago

Remove systemMemory, environment from automationutils


(Testing :: General, defect)

Not set


(firefox42 fixed)

Tracking Status
firefox42 --- fixed


(Reporter: jgriffin, Assigned: parkouss, Mentored)



User Story

Delete the systemMemory() and environment() methods from


(1 file)

It looks like these aren't actually used; the only call sites which use environment() use the copy in
User Story: (updated)
(In reply to Jonathan Griffin (:jgriffin) from comment #0)
> It looks like these aren't actually used; the only call sites which use
> environment() use the copy in

Whoops, that's wrong.  It is imported in a couple of places, at least:

Ideally we should move this to a mozbase package, but I'm not sure we have an appropriate one.  Do we need a new package to contain things like this?
Whiteboard: [good first bug][lang=python]
This probably belongs in the same place as the leak log functions, wherever that turns out to be (bug 1091274).
So, as far as I can understand from bug 1091274, it is suggested to create a mozrunner subclass inside moztest to put the leak logs functions in. So I suppose it should be the same here, putting environment() inside the same subclass.

I started to look at it, and it seems that it is pretty hard to inherit from mozrunner, as real runners are built on top of two distinct classes: DeviceRunner and GeckoRuntimeRunner. Plus it appears that sometimes the environment call does not fit into one or the other I think, like here for xpcshell:

As a starting point, I suggest that we just move the function inside moztest (maybe moztest.utils or moztest.runner). It would be still possible after that to create subclasses and call that function if desired, but for now it would help in the process of removing, and I think it make sense to have a public function that just populate env vars.

Guys, what do you think ?
Ok, so this was discussed a bit more on irc. :)

Actually :ted, :jgraham and :Ms2Ger agreed that a better place for that should be inside mozrunner. this makes sense to me also.

So I'd like to start with that: move the function somewhere in mozrunner as a start. We'll see later for some kind of better abstraction (like maybe inside a mozrunner subclass).

:jgriffin, does that makes sense to you ?
Flags: needinfo?(jgriffin)
Yes, that makes sense, thanks Julien.
Flags: needinfo?(jgriffin)
Bug 1091284 - Remove systemMemory, environment from automationutils. r?jgriffin
Attachment #8628411 - Flags: review?(jgriffin)
Comment on attachment 8628411 [details]
MozReview Request: Bug 1091284 - Remove systemMemory, environment from automationutils. r?jgriffin

It's a bit ugly to shove this into mozrunner, but it doesn't have a more obvious home, and in the interests of getting rid of automationutils, let's do it.  In the future, if we turn the harnesses into real Python packages, we may find another home for this.
Attachment #8628411 - Flags: review?(jgriffin) → review+
Yes, I fully agree.

I ran a first push try yesterday, covering probably more than necessary:

After that I changed my mind to slightly change the log interface. So I ran another try, but only on one mochitest test:

All went good, so I'm confident (double checked) and I will land that (ie what was reviewed).
Assignee: nobody → j.parkouss
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.