Bug 111021

Summary: Add --additional-drt-flag option to run-perf-tests to make it easy to test runtime options
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: New BugsAssignee: Eric Seidel (no email) <eric>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dpranke, rniwa, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 106127    
Attachments:
Description Flags
Patch
none
Patch for landing none

Description Eric Seidel (no email) 2013-02-27 15:47:42 PST
Add --additional-drt-flag option to run-perf-tests to make it easy to test runtime options
Comment 1 Eric Seidel (no email) 2013-02-27 15:48:29 PST
Created attachment 190621 [details]
Patch
Comment 2 Dirk Pranke 2013-02-27 15:50:40 PST
Comment on attachment 190621 [details]
Patch

looks fine. is there any way to test this (and is it worth it)?
Comment 3 Eric Seidel (no email) 2013-02-27 15:51:05 PST
We still can't meaningfully run most of the perf tests in the threaded parser as they use document.write instead of urls to load, but this at least plumbs the option.
Comment 4 Dirk Pranke 2013-02-27 15:54:02 PST
I was more wondering if there's some way to stub out / mock some of the calls just to check that the flag is making it through to the driver.
Comment 5 Eric Seidel (no email) 2013-02-27 15:55:16 PST
Created attachment 190623 [details]
Patch for landing
Comment 6 Eric Seidel (no email) 2013-02-27 15:56:28 PST
Good question.  Added a very basic test.  I didn't see any other tests using a mock driver for run-perf-tests.  This option depends on the driver reading the option, rather than pushing it to the driver, making using a mock driver much less useful anyway.  We could use a driver with a MockExecutive and that could work, but I decided this was sufficient for now.  If you disagree I can revisit.
Comment 7 Dirk Pranke 2013-02-27 16:01:02 PST
Ah. I probably wouldn't have bothered with that test, although it is perhaps better than nothing. It looks like it wouldn't be too hard to add something to perftestsrunner_integrationtest.py that does get all the way through to a mocked-out driver, but I don't know if it's really worth it or important since, as you say, the plumbing was already pretty much there.
Comment 8 Eric Seidel (no email) 2013-02-27 16:05:50 PST
I don't think adding flags to DRT is particularly common.  I think it should be. :)  But I think the threaded parser is the first to do this for a feature in a long time.  Next time this comes up we can make this support even more awesome.
Comment 9 Dirk Pranke 2013-02-27 16:13:49 PST
If we do start adding more flags to DRT, we should probably at least make --help work :).
Comment 10 WebKit Review Bot 2013-02-27 17:35:41 PST
Comment on attachment 190623 [details]
Patch for landing

Clearing flags on attachment: 190623

Committed r144253: <http://trac.webkit.org/changeset/144253>
Comment 11 WebKit Review Bot 2013-02-27 17:35:45 PST
All reviewed patches have been landed.  Closing bug.