Closed Bug 993320 Opened 7 years ago Closed 7 years ago

[mozilla-runner] Allow environment to be entirely overridden or have variables set/cleared

Categories

(Testing Graveyard :: JSMarionette, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
1.4 S6 (25apr)

People

(Reporter: asuth, Assigned: asuth)

Details

(Whiteboard: [c= p=0.5])

Attachments

(1 file)

Currently the only way to control the environment of the app launched by mozrunner is to alter the current environment before spawning the subprocess.  It would be better to support specific arguments, especially in a case like the Gaia JS Marionette tests where the code specifying the arguments is already in a subprocess of where mozrunner runs and so can't do that hack itself.
Kevin, asking you for review on this since you are listed as the only maintainer other than :lightsofapollo on https://www.npmjs.org/package/mozilla-runner and I am using that as a proxy for ownership/peerage since the repo itself lacks explicit guidance there and https://wiki.mozilla.org/Modules/FirefoxOS does not explicitly call the repo out.
Attachment #8403142 - Flags: review?(kgrandon)
Summary: [mozrunner] Allow environment to be entirely overridden or have variables set/cleared → [mozilla-runner] Allow environment to be entirely overridden or have variables set/cleared
Huh, so, as a note, the repo's package.json has version 0.2.0 in it, but npm claims 0.3.0 is the most recent version.  It looks like maybe this was a glitch related to merging the commits for bug 930948 without having that bug update package.json and whoever realized that forgot to commit their correction to the repo.

Accordingly I'm updating my pull request to jump the version from 0.2.0 to 0.3.1.
Comment on attachment 8403142 [details] [review]
allow explicit env / overridden env values

So I don't believe I am any kind of peer/owner of this code, and I have not landed patches in master yet. Your code seems reasonable though, and due to the circumstances of James being on PTO for a while - I feel it should be reasonable that someone should be able to review this code (it's also not part of the build so I'm a bit more comfortable in this case).

I left some suggestions on github mainly regarding style. Nothing I would really block the review on though.
Attachment #8403142 - Flags: review?(kgrandon) → review+
Thanks for the super-fast review!  I've either made the requested changes or commented on why I think my current choices make sense (but am open to change things if you disagree with my rationale/revisions.)  I've rebased and pushed.

needinfo for: I lack the ability to merge the revised patch or to publish a new version on npm.  Assuming you are okay with the (revised) patch, can you merge it and push a new version to npm?
Flags: needinfo?(kgrandon)
Landed: https://github.com/mozilla-b2g/mozilla-runner/commit/edc9cb4ba04a8140868b82af223501f259cc4040

I have also made you a collaborator of the project.
Flags: needinfo?(kgrandon)
Slight fubar in collaborator adding, but I've fixed it and added the proper permissions group to this repo.
And a version 0.4.0 is published: https://github.com/mozilla-b2g/mozilla-runner/commit/9d0f8e8509f073c44e6659256cf2e99ce6bf9049

https://www.npmjs.org/package/mozilla-runner
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [c= p=0.5]
Target Milestone: --- → 1.4 S6 (25apr)
Product: Testing → Testing Graveyard
You need to log in before you can comment on or make changes to this bug.