Bug 105248 - Consider removing --pause-before-testing option
Summary: Consider removing --pause-before-testing option
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords:
Depends on:
Blocks: 77037
  Show dependency treegraph
 
Reported: 2012-12-17 21:10 PST by Ryosuke Niwa
Modified: 2012-12-18 10:56 PST (History)
5 users (show)

See Also:


Attachments
Cleanup (13.10 KB, patch)
2012-12-17 23:01 PST, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Also remove Driver.start (14.16 KB, patch)
2012-12-17 23:10 PST, Ryosuke Niwa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2012-12-17 21:10:38 PST
Now that we've added --profile option to run-perf-tests, do we still need --pause-before-testing? We added this option to help us attach profiler but it adds a lot of crafts to various parts in webkitpy.
Comment 1 Eric Seidel (no email) 2012-12-17 22:29:33 PST
Completely up to you.  :)
Comment 2 Ryosuke Niwa 2012-12-17 22:35:21 PST
(In reply to comment #1)
> Completely up to you.  :)

Alright. Let's get rid of it. I'm planning to make a serious of improvements to run-perf-tests and not having to support it will considerably simplify the patch.
Comment 3 Eric Seidel (no email) 2012-12-17 22:48:15 PST
I would caution you that --profile is not currently designed to handle every possible iprofiler, etc. configuration, so if others are using --pause-before-testing for more exciting profiling, it may still have its uses.
Comment 4 Ryosuke Niwa 2012-12-17 22:53:57 PST
(In reply to comment #3)
> I would caution you that --profile is not currently designed to handle every possible iprofiler, etc. configuration, so if others are using --pause-before-testing for more exciting profiling, it may still have its uses.

I guess we can always restore it if someone complaints.
Comment 5 Ryosuke Niwa 2012-12-17 23:01:14 PST
Created attachment 179882 [details]
Cleanup
Comment 6 Eric Seidel (no email) 2012-12-17 23:04:04 PST
Comment on attachment 179882 [details]
Cleanup

I believe you can also remove Driver.start() as dpranke told me once it only existed for this option to perf-tests.
Comment 7 Ryosuke Niwa 2012-12-17 23:07:58 PST
(In reply to comment #6)
> (From update of attachment 179882 [details])
> I believe you can also remove Driver.start() as dpranke told me once it only existed for this option to perf-tests.

Will do.
Comment 8 Ryosuke Niwa 2012-12-17 23:10:06 PST
Created attachment 179883 [details]
Also remove Driver.start
Comment 9 Eric Seidel (no email) 2012-12-17 23:15:41 PST
Comment on attachment 179883 [details]
Also remove Driver.start

This looks fine to me, but you should probably also have dpranke take a look, and probably give folks at least until tomorrow to give comments (since you announced it on webkit-dev). :)
Comment 10 Ryosuke Niwa 2012-12-18 10:56:48 PST
Comment on attachment 179883 [details]
Also remove Driver.start

Clearing flags on attachment: 179883

Committed r138042: <http://trac.webkit.org/changeset/138042>
Comment 11 Ryosuke Niwa 2012-12-18 10:56:50 PST
All reviewed patches have been landed.  Closing bug.