Closed Bug 1100419 Opened 10 years ago Closed 9 years ago

./mach mochitest should take --timeout parameter to force lower timeout

Categories

(Testing :: Mochitest, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla36

People

(Reporter: Gijs, Assigned: Gijs)

Details

Attachments

(1 file, 1 obsolete file)

On my high-end machine and an opt build, I don't want to be waiting 1 minute when doing test runs to see which of N tests time out / fail, every time one does. :-)
Attachment #8523931 - Flags: review?(ted)
Comment on attachment 8523931 [details] [diff] [review]
add --timeout switch to mochitets,

There's trivial and then there's unwarranted optimism...
Attachment #8523931 - Flags: review?(ted)
Attachment #8524157 - Flags: review?(ted)
Attachment #8523931 - Attachment is obsolete: true
Comment on attachment 8524157 [details] [diff] [review]
add --timeout switch to mochitets,

Review of attachment 8524157 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/mochitest/mach_commands.py
@@ +569,5 @@
>              "If you have run ./mach package beforehand, you can specify 'dist' to " \
>              "run tests against the distribution bundle's binary.");
>      func = app_override(func)
>  
> +    timeout = CommandArgument('--timeout', default=-1, type=int,

Why not just default this to None so you don't have to tweak runtests.py? (I realize you would not be able to use type=int for that.)

@@ +570,5 @@
>              "run tests against the distribution bundle's binary.");
>      func = app_override(func)
>  
> +    timeout = CommandArgument('--timeout', default=-1, type=int,
> +        help='The per-test timeout time in seconds (default: 60 seconds)');

"timeout time" seems redundant.
Attachment #8524157 - Flags: review?(ted) → review+
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #4)
> Comment on attachment 8524157 [details] [diff] [review]
> add --timeout switch to mochitets,
> 
> Review of attachment 8524157 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: testing/mochitest/mach_commands.py
> @@ +569,5 @@
> >              "If you have run ./mach package beforehand, you can specify 'dist' to " \
> >              "run tests against the distribution bundle's binary.");
> >      func = app_override(func)
> >  
> > +    timeout = CommandArgument('--timeout', default=-1, type=int,
> 
> Why not just default this to None so you don't have to tweak runtests.py? (I
> realize you would not be able to use type=int for that.)

Oh, good idea. However, then don't I need to coerce the argument at some later point if it *was* provided? Or does python do that for me?

> @@ +570,5 @@
> >              "run tests against the distribution bundle's binary.");
> >      func = app_override(func)
> >  
> > +    timeout = CommandArgument('--timeout', default=-1, type=int,
> > +        help='The per-test timeout time in seconds (default: 60 seconds)');
> 
> "timeout time" seems redundant.

Fair!
Flags: needinfo?(ted)
Yeah, fair point, you'd need to do something like 'int(options.timeout) if options.timeout is not None else None', which is maybe uglier (although constrained to one place).
Flags: needinfo?(ted)
using None and not touching runtests.py \o/

remote:   https://hg.mozilla.org/integration/fx-team/rev/09034b8710cb
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/6e82407a7c65
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.