Closed Bug 1185761 Opened 5 years ago Closed 5 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)

defect
Not set

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
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.
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.
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.
Duplicate of this bug: 1191865
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?
Can someone explain to me why Boris can't just pass --keep-open?
Because we don't have any way to make that the default, and that's what he does every time.
(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
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 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 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+
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
https://hg.mozilla.org/mozilla-central/rev/992b47960b9e
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in before you can comment on or make changes to this bug.