--wait-for-browser arguments increase by one every time mozregression runs the next build
Categories
(Testing :: mozregression, defect)
Tracking
(firefox84 fixed)
| Tracking | Status | |
|---|---|---|
| firefox84 | --- | fixed |
People
(Reporter: kats, Assigned: Kwan)
Details
Attachments
(4 files)
See attached mozregression output. The arguments to firefox include an extra --wait-for-browser on each invocation
|
||
Comment 1•4 years ago
|
||
The severity field is not set for this bug.
:wlach, could you have a look please?
For more information, please visit auto_nag documentation.
|
||
Updated•4 years ago
|
|
||
Comment 2•4 years ago
|
||
|
Assignee | |
Comment 3•4 years ago
|
||
This one took me a little while to reproduce, since it's a) limited to Windows and b) limited to when you actually pass in an --arg, and I hadn't initially noticed the URL in the log.
So the issue is that on Windows mozrunner always adds --wait-for-browser to the cmd args. Now maybe mozrunner should only do that if it isn't already there, but it also seems like we shouldn't be allowing external (to mozregression) code to alter our data, so I went with passing a copy of cmdargs to mozrunner rather than the original object.
|
|
||
Comment 4•4 years ago
|
||
I'm a bit suspicious that mozregression should be worrying about this. Two things:
- Yes, mozrunner should check if this is already there.
- mozrunner shouldn't be modifying this variable at all IMO (if it wants to add something to what's passed in, it should be working against its own copy)
Looking at searchfox, I think we could probably fix this with a fairly straightforward patch to mozrunner: https://searchfox.org/mozilla-central/rev/353f334d13359f7cb879b3cb38a94294231c2aa8/testing/mozbase/mozrunner/mozrunner/base/browser.py#35
:rstewart wdyt?
|
||
Comment 5•4 years ago
|
||
Really not my area of expertise :)
If it were me I would do a if '--wait-for-browser' not in self.cmdargs: self.cmdargs.append('--wait-for-browser') thing, but I don't have enough context to be sure.
|
|
||
Comment 6•4 years ago
|
||
(In reply to Ricky Stewart from comment #5)
Really not my area of expertise :)
If it were me I would do a
if '--wait-for-browser' not in self.cmdargs: self.cmdargs.append('--wait-for-browser')thing, but I don't have enough context to be sure.
Hmm, :ahal do you have opinions here? See comment 4 and comment 5.
|
||
Comment 7•4 years ago
|
||
Wfm! No one's particularly involved in mozbase these days, to the point where if tests don't break you probably won't get any opposition :). I agree it seems like something to fix in mozrunner rather than mozregression.
|
|
Assignee | |
Comment 9•4 years ago
|
||
And make a copy of the passed in cmdargs so we don't modify the calling
program's copy.
|
|
Assignee | |
Comment 10•4 years ago
|
||
|
|
Assignee | |
Comment 11•4 years ago
|
||
(In reply to William Lachance (:wlach) (use needinfo!) from comment #8)
:Kwan would you care to give this a try?
Sure!
Try run (pre-black), tests seem to be okay (except for bug 1675631, thanks for fixing that ahal)
|
||
Comment 12•4 years ago
|
||
Pushed by btara@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/39c82171f295 [mozrunner] Only add --wait-for-browser to cmdargs if it's absent. r=ahal https://hg.mozilla.org/integration/autoland/rev/99163fcc949d [mozrunner] Bump version to 8.0.3. r=ahal
|
||
Comment 13•4 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/39c82171f295
https://hg.mozilla.org/mozilla-central/rev/99163fcc949d
Description
•