Bug 50045 – new-run-webkit-tests: add a --dry-run / -n flag

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Dirk Pranke]

new-run-webkit-tests: add a --dry-run / -n flag

[Dirk Pranke]

Created attachment 74811 [details]
Patch

[Tony Chang]

Comment on attachment 74811 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=74811&action=review

> WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:612
>          try:
> -            message_broker.run_message_loop()
> +            if not self._options.dry_run:
> +                message_broker.run_message_loop()

I would put the 'if' outside try/except since the try/except doesn't seem to apply if it's a dry run.

[Dirk Pranke]

Created attachment 74817 [details]
review feedback, fix default value for --dry-run

[Dirk Pranke]

(In reply to comment #2)
> (From update of attachment 74811 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=74811&action=review
> 
> > WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:612
> >          try:
> > -            message_broker.run_message_loop()
> > +            if not self._options.dry_run:
> > +                message_broker.run_message_loop()
> 
> I would put the 'if' outside try/except since the try/except doesn't seem to apply if it's a dry run.

Done. I was attempting to follow the "skip as little code as possible" rule with --dry-run, which sort of conflicts with the "wrap as little code as possible in the try block" rule. I think the precedence here is a bit of a toss-up, but I've done it your way.

Also, fixed the default value for --dry-run which was causing all of the tests I didn't add to fail ;)

[Dirk Pranke]

Comment on attachment 74817 [details]
review feedback, fix default value for --dry-run

Clearing flags on attachment: 74817

Committed r73080: <http://trac.webkit.org/changeset/73080>

[Dirk Pranke]

All reviewed patches have been landed.  Closing bug.