Closed
Bug 1147129
Opened 9 years ago
Closed 9 years ago
Upgrade mochitest from optparse to argparse
Categories
(Testing :: Mochitest, defect)
Tracking
(firefox40 fixed)
RESOLVED
FIXED
mozilla40
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: ahal, Assigned: ahal)
References
Details
Attachments
(1 file, 1 obsolete file)
We no longer have any python 2.6 dependencies, so let's upgrade to argparse. It's easy to do and will make it easier to do future work around option handling (namely using configman if we decide to do that).
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=49c1cb163d61
Assignee | ||
Comment 2•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=74d61d9755f2
Assignee | ||
Comment 3•9 years ago
|
||
/r/6659 - Bug 1147129 - upgrade mochitest from optparse to argparse and move android cli to mochitest_options.py, r=chmanchester Pull down this commit: hg pull -r 74d61d9755f21dfb01f74accbeae6e6fba67f60e https://reviewboard-hg.mozilla.org/gecko/
Assignee | ||
Comment 4•9 years ago
|
||
Comment on attachment 8588595 [details] MozReview Request: bz://1147129/ahal /r/6659 - Bug 1147129 - upgrade mochitest from optparse to argparse and move android cli to mochitest_options.py, r=chmanchester Pull down this commit: hg pull -r 74d61d9755f21dfb01f74accbeae6e6fba67f60e https://reviewboard-hg.mozilla.org/gecko/
Attachment #8588595 -
Flags: review?(cmanchester)
Assignee | ||
Comment 5•9 years ago
|
||
Comment on attachment 8588595 [details] MozReview Request: bz://1147129/ahal Here's a passing try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=47c127009e76 Geoff, this patch moves the android command line option handling over to mochitest_options.py alongside the desktop and b2g options, but otherwise shouldn't have any effect on how they get run. Is this ok with you? I'll likely be refactoring that file into a subdirectory + config files later on.
Attachment #8588595 -
Flags: feedback?(gbrown)
Comment 6•9 years ago
|
||
Comment on attachment 8588595 [details]
MozReview Request: bz://1147129/ahal
I have no objection -- it looks good to me.
Attachment #8588595 -
Flags: feedback?(gbrown) → feedback+
Updated•9 years ago
|
Attachment #8588595 -
Flags: review?(cmanchester) → review+
Comment 7•9 years ago
|
||
Comment on attachment 8588595 [details] MozReview Request: bz://1147129/ahal https://reviewboard.mozilla.org/r/6657/#review5545 Ship It! ::: testing/mochitest/mochitest_options.py (Diff revision 1) > - defaults["httpPort"] = DEFAULT_PORTS['http'] > - defaults["sslPort"] = DEFAULT_PORTS['https'] > defaults["logFile"] = "mochitest.log" > defaults["autorun"] = True > defaults["closeWhenDone"] = True > - defaults["testPath"] = "" Any particular reason to move some of these defaults and not others? Can we just make them all inline or set them all here? ::: testing/mochitest/runtests.py (Diff revision 1) > - options, args = parser.parse_args() > + options = parser.parse_args() Nit: Here and elsewhere we could call the result of this method call "args" ::: testing/mochitest/mochitest_options.py (Diff revision 1) > + options.app = __file__ Moving this changes the value which looks suspicious, but I guess it's just a throwaway value, right?
Assignee | ||
Comment 8•9 years ago
|
||
https://reviewboard.mozilla.org/r/6657/#review5551 > Any particular reason to move some of these defaults and not others? Can we just make them all inline or set them all here? Ideally they'd all be inlined, but the ones left over are needed to override the default of an option defined by the super class. > Moving this changes the value which looks suspicious, but I guess it's just a throwaway value, right? Yeah, it's getting set back to the proper value at the bottom of that function. Good eye though, I didn't notice that. > Nit: Here and elsewhere we could call the result of this method call "args" I agree, but the problem is that that options dict then gets passed around literally everywhere in mochitest. So renaming it to args here either means we'd have to s/options/args everywhere else, or we'd be inconsistent with the rest of the harness. Personally I think leaving it as "options" for now is easiest.
Assignee | ||
Comment 9•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/0ab6e1161f94
Comment 10•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/0ab6e1161f94
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Assignee | ||
Comment 11•9 years ago
|
||
Attachment #8588595 -
Attachment is obsolete: true
Attachment #8619866 -
Flags: review+
Assignee | ||
Comment 12•9 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•