Bug 111021 - Add --additional-drt-flag option to run-perf-tests to make it easy to test runtime options
Summary: Add --additional-drt-flag option to run-perf-tests to make it easy to test ru...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eric Seidel (no email)
URL:
Keywords:
Depends on:
Blocks: 106127
  Show dependency treegraph
 
Reported: 2013-02-27 15:47 PST by Eric Seidel (no email)
Modified: 2013-02-27 17:35 PST (History)
4 users (show)

See Also:


Attachments
Patch (1.86 KB, patch)
2013-02-27 15:48 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Patch for landing (3.15 KB, patch)
2013-02-27 15:55 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.