Closed
Bug 1185761
Opened 9 years ago
Closed 9 years ago
Ability to override special case that keeps browser open if test length is 1 and the test is mochitest-plain
Categories
(Testing :: Mochitest, defect)
Testing
Mochitest
Tracking
(firefox43 fixed)
RESOLVED
FIXED
mozilla43
Tracking | Status | |
---|---|---|
firefox43 | --- | fixed |
People
(Reporter: erahm, Assigned: ahal)
References
Details
Attachments
(1 file)
Use case: I want to hunt down a regression w/ a plain mochitest. Problem: browser stays open after running the test which makes automating the regression hunt impossible. Proposed solution: remove the special case [1], we already have a '--keep-open' flag. [1] https://hg.mozilla.org/mozilla-central/annotate/e7e69cc8c07b/testing/mochitest/mach_commands.py#l347
Assignee | ||
Comment 1•9 years ago
|
||
Personally +1. But I was previously told that special case was important to keep, so you might get some push back. The alternative is to make all flavors have that behaviour, and then provide another option to override it, but that's more complicated than I'd like.
Comment 2•9 years ago
|
||
You can automate the regression hunt by passing the opposite of the keep-open flag. I don't know what that's called after all the renaming. People who write mochitest-plain tests want to be able to reload the page after changing the test file in order to quickly iterate on a test. Notably bz argued in favour of keeping this as-is. I don't write m-plain very often if at all, but you should probably check in with him when he's back from vacation.
Assignee | ||
Comment 3•9 years ago
|
||
There is no longer an opposite flag to --keep-open. I was trying to simplify the command line, and having both --auto-close and --keep-open seemed unnecessarily complicated. But yes, the current situation where you can't override --keep-open in this special case is bad. There are two options here: 1) Remove the special case like :erahm proposes, and people can use --keep-open as needed 2) Re-add --auto-close I don't like 2) because it isn't clear to anyone when you'd need to use it. In fact, it's a no-op unless you run a single test with --flavor plain. That is not good ux.
Comment 5•9 years ago
|
||
Yeah, you can't fix this bug as stated, that will break bz's use case (don't do that). Maybe we could make --keep-open take an optional parameter so you could pass --keep-open=false?
Comment 6•9 years ago
|
||
Can someone explain to me why Boris can't just pass --keep-open?
Comment 7•9 years ago
|
||
Because we don't have any way to make that the default, and that's what he does every time.
Assignee | ||
Comment 8•9 years ago
|
||
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #5) > Yeah, you can't fix this bug as stated, that will break bz's use case (don't > do that). Maybe we could make --keep-open take an optional parameter so you > could pass --keep-open=false? I didn't think this was possible, but looking into it, it is with the 'const' keyword. Seems like a good enough compromise.
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
Assignee | ||
Comment 9•9 years ago
|
||
Bug 1185761 - [mochitest] Allow boolean values to --keep-open for overriding the default, r=ted Most of the time the default is to close the browser after tests run. And that can be overridden with --keep-open. But in some corner cases, the default is to leave the browser open after tests have run. In these cases, it is not currently possible to override. This patch allows the syntax --keep-open or --keep-open=false, the latter overrides the edge case default.
Attachment #8644953 -
Flags: review?(ted)
Comment 10•9 years ago
|
||
Comment on attachment 8644953 [details] MozReview Request: Bug 1185761 - [mochitest] Allow boolean values to --keep-open for overriding the default, r=ted https://reviewboard.mozilla.org/r/15347/#review14929 ::: testing/mochitest/mach_commands.py:342 (Diff revision 1) > # XXX why is this such a special case? It'd be good to document why this is a special case instead of the confused comment here.
Attachment #8644953 -
Flags: review?(ted)
Comment 11•9 years ago
|
||
Comment on attachment 8644953 [details] MozReview Request: Bug 1185761 - [mochitest] Allow boolean values to --keep-open for overriding the default, r=ted https://reviewboard.mozilla.org/r/15347/#review14931 Ship It!
Attachment #8644953 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
Summary: Remove special case that keeps browser open if test length is 1 and the test is mochitest-plain → Ability to override special case that keeps browser open if test length is 1 and the test is mochitest-plain
Comment 13•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/992b47960b9e
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox43:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in
before you can comment on or make changes to this bug.
Description
•