[webkit-reviews] review granted: [Bug 84008] Add public page loading performance tests using web-page-replay : [Attachment 145188] Addressed Dirk's comments.

[bugzilla-daemon at webkit.org]

Dirk Pranke <dpranke at chromium.org> has granted Ryosuke Niwa
<rniwa at webkit.org>'s request for review:
Bug 84008: Add public page loading performance tests using web-page-replay
https://bugs.webkit.org/show_bug.cgi?id=84008

Attachment 145188: Addressed Dirk's comments.
https://bugs.webkit.org/attachment.cgi?id=145188&action=review

------- Additional Comments from Dirk Pranke <dpranke at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=145188&action=review


> Tools/Scripts/webkitpy/layout_tests/port/driver.py:134
> +

Turns out I don't want this ... see below (and sorry!)

> Tools/Scripts/webkitpy/layout_tests/port/webkit.py:533
> +	       command = driver_input.test_name

Okay, I see the problem here, which is that driver_input.test_name isn't
actually a test_name, it's a full URL. DriverInputs aren't supposed to be full
URLs, they're supposed to be test names :). 

However, it would be lame if that kept you from using the driver, so we need
some other way of distinguishing that this is a URL and not a test name. I'm
not sure what the best way to do that is yet, and I don't want to block you on
this ... Can you add a #FIXME for now, flip this clause so that it's tested
first, and go back to what you were doing before? e.g.:

# FIXME: perf tests can pass in full URLs, not just test names ...
if driver_input.test_name.startswith('http://') or
driver_input.test_name.startswith('https://'):
   command = driver_input.test_name
else:
   ...

sorry for the back-and-forth on this.

Everything else looks fine. Thanks!