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

Eric Seidel (no email)
Reported 2013-02-27 15:47:42 PST
Add --additional-drt-flag option to run-perf-tests to make it easy to test runtime options
Attachments
Patch (1.86 KB, patch)
2013-02-27 15:48 PST, Eric Seidel (no email) 		no flags
DetailsFormatted DiffDiff
Patch for landing (3.15 KB, patch)
2013-02-27 15:55 PST, Eric Seidel (no email) 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1 2013-02-27 15:48:29 PST
Created attachment 190621 [details] Patch
Dirk Pranke
Comment 2 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)?
Eric Seidel (no email)
Comment 3 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.
Dirk Pranke
Comment 4 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.
Eric Seidel (no email)
Comment 5 2013-02-27 15:55:16 PST
Created attachment 190623 [details] Patch for landing
Eric Seidel (no email)
Comment 6 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.
Dirk Pranke
Comment 7 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.
Eric Seidel (no email)
Comment 8 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.
Dirk Pranke
Comment 9 2013-02-27 16:13:49 PST
If we do start adding more flags to DRT, we should probably at least make --help work :).
WebKit Review Bot
Comment 10 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>
WebKit Review Bot
Comment 11 2013-02-27 17:35:45 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.