RESOLVED FIXED 48488
[chromium] add a master-name flag to new-run-webkit-tests
https://bugs.webkit.org/show_bug.cgi?id=48488
Summary [chromium] add a master-name flag to new-run-webkit-tests
Ojan Vafai
Reported 2010-10-27 17:25:51 PDT
[chromium] add a master-name flag to new-run-webkit-tests
Attachments
Patch (2.28 KB, patch)
2010-10-27 17:27 PDT, Ojan Vafai
tony: review+
Ojan Vafai
Comment 1 2010-10-27 17:27:09 PDT
Dirk Pranke
Comment 2 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?
Tony Chang
Comment 3 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.
Ojan Vafai
Comment 4 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.
Ojan Vafai
Comment 5 2010-10-28 09:27:30 PDT
Note You need to log in before you can comment on or make changes to this bug.