It looks like these aren't actually used; the only call sites which use environment() use the copy in automation.py.in.

(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 automation.py.in. Whoops, that's wrong. It is imported in a couple of places, at least: http://dxr.mozilla.org/mozilla-central/source/testing/mochitest/runtests.py#37 http://dxr.mozilla.org/mozilla-central/source/layout/tools/reftest/runreftest.py#28 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?

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: https://dxr.mozilla.org/mozilla-central/source/testing/mochitest/runtests.py#408 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 automationutils.py, 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 ?

Yes, that makes sense, thanks Julien.

Bug 1091284 - Remove systemMemory, environment from automationutils. r?jgriffin

Comment on attachment 8628411 [details] MozReview Request: Bug 1091284 - Remove systemMemory, environment from automationutils. r?jgriffin https://reviewboard.mozilla.org/r/12373/#review10933 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.

Yes, I fully agree. I ran a first push try yesterday, covering probably more than necessary: https://treeherder.mozilla.org/#/jobs?repo=try&revision=fd0a4a9f7836 After that I changed my mind to slightly change the log interface. So I ran another try, but only on one mochitest test: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d1b55dd302c4 All went good, so I'm confident (double checked) and I will land that (ie what was reviewed).