Bug 48488 - [chromium] add a master-name flag to new-run-webkit-tests
Summary: [chromium] add a master-name flag to new-run-webkit-tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Ojan Vafai
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-10-27 17:25 PDT by Ojan Vafai
Modified: 2010-10-28 09:27 PDT (History)
4 users (show)

See Also:


Attachments
Patch (2.28 KB, patch)
2010-10-27 17:27 PDT, Ojan Vafai
tony: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ojan Vafai 2010-10-27 17:25:51 PDT
[chromium] add a master-name flag to new-run-webkit-tests
Comment 1 Ojan Vafai 2010-10-27 17:27:09 PDT
Created attachment 72120 [details]
Patch
Comment 2 Dirk Pranke 2010-10-27 17:33:28 PDT
Comment on attachment 72120 [details]
Patch

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

LGTM with the minor comment about the FIXME (I don't feel strongly about it one way or another).

> WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:958
> +

Is there a reason you didn't just add

if not self._options.test_results_server:
    raise AssertionError()

or something here instead of adding a FIXME?
Comment 3 Tony Chang 2010-10-27 18:03:14 PDT
Comment on attachment 72120 [details]
Patch

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

>> WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:958
>> +
> 
> Is there a reason you didn't just add
> 
> if not self._options.test_results_server:
>     raise AssertionError()
> 
> or something here instead of adding a FIXME?

I assume we need to add the flags to the bots, which is a server change?  I guess we could do that first.  Once we require it, we can unit tests this requirement.

Seems fine for now.
Comment 4 Ojan Vafai 2010-10-28 07:14:41 PDT
(In reply to comment #3)
> >> WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:958
> > 
> > Is there a reason you didn't just add
> > 
> > if not self._options.test_results_server:
> >     raise AssertionError()
> > 
> > or something here instead of adding a FIXME?
> 
> I assume we need to add the flags to the bots, which is a server change?  I guess we could do that first.  Once we require it, we can unit tests this requirement.

Right. The bots need to add the flag. But in order to add the flag to the bots, the script needs to allow the flag in the first place.
Comment 5 Ojan Vafai 2010-10-28 09:27:30 PDT
Committed r70782: <http://trac.webkit.org/changeset/70782>