Bug 93758 - Rename LayoutTestController to TestRunner in DumpRenderTree
Summary: Rename LayoutTestController to TestRunner in DumpRenderTree
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: Ryosuke Niwa
URL:
Keywords:
Depends on: 88210 93899
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-10 18:05 PDT by Ryosuke Niwa
Modified: 2012-08-14 00:36 PDT (History)
11 users (show)

See Also:


Attachments
Patch (1.63 MB, patch)
2012-08-10 18:11 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Patch (590.03 KB, patch)
2012-08-13 22:57 PDT, Ryosuke Niwa
tony: review+
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-08-10 18:05:39 PDT
Rename LayoutTestController to TestRuner in DumpRenderTree
Comment 1 Ryosuke Niwa 2012-08-10 18:11:04 PDT
Created attachment 157847 [details]
Patch
Comment 2 Ryosuke Niwa 2012-08-10 18:12:38 PDT
One is one epic patch...
Comment 3 WebKit Review Bot 2012-08-10 18:16:36 PDT
Attachment 157847 [details] did not pass style-queue:

Tools/DumpRenderTree/qt/TestRunnerQt.cpp:147:  Should have a space between // and comment  [whitespace/comments] [4]
Tools/DumpRenderTree/qt/TestRunnerQt.cpp:322:  Should have a space between // and comment  [whitespace/comments] [4]
Tools/DumpRenderTree/qt/TestRunnerQt.cpp:329:  Should have a space between // and comment  [whitespace/comments] [4]
Tools/DumpRenderTree/qt/TestRunnerQt.cpp:336:  Should have a space between // and comment  [whitespace/comments] [4]
Tools/DumpRenderTree/qt/TestRunnerQt.cpp:352:  Should have a space between // and comment  [whitespace/comments] [4]
Tools/DumpRenderTree/qt/TestRunnerQt.cpp:358:  Should have a space between // and comment  [whitespace/comments] [4]
Tools/DumpRenderTree/qt/TestRunnerQt.cpp:364:  Should have a space between // and comment  [whitespace/comments] [4]
Tools/DumpRenderTree/qt/TestRunnerQt.cpp:515:  Should have a space between // and comment  [whitespace/comments] [4]
Tools/DumpRenderTree/qt/TestRunnerQt.h:37:  Alphabetical sorting problem.  [build/include_order] [4]
Tools/DumpRenderTree/qt/TestRunnerQt.h:52:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Tools/DumpRenderTree/qt/TestRunnerQt.h:134:  Missing space inside { }.  [whitespace/braces] [5]
Tools/DumpRenderTree/qt/TestRunnerQt.h:147:  Missing space inside { }.  [whitespace/braces] [5]
Tools/DumpRenderTree/qt/TestRunnerQt.h:154:  The parameter name "type" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/qt/TestRunnerQt.h:155:  The parameter name "enable" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/qt/TestRunnerQt.h:156:  The parameter name "enable" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/qt/TestRunnerQt.h:163:  The parameter name "enable" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/qt/TestRunnerQt.h:164:  The parameter name "enabled" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/qt/TestRunnerQt.h:166:  The parameter name "enable" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/qt/TestRunnerQt.h:167:  The parameter name "locale" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/qt/TestRunnerQt.h:169:  The parameter name "isKey" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/qt/TestRunnerQt.h:170:  The parameter name "isFirst" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/qt/TestRunnerQt.h:173:  The parameter name "enable" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/qt/TestRunnerQt.h:174:  The parameter name "enable" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/qt/TestRunnerQt.h:176:  The parameter name "mode" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/qt/TestRunnerQt.h:177:  The parameter name "enable" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/qt/TestRunnerQt.h:178:  The parameter name "enable" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/qt/TestRunnerQt.h:181:  The parameter name "string" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/qt/TestRunnerQt.h:197:  The parameter name "quota" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/qt/TestRunnerQt.h:204:  The parameter name "enable" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/qt/TestRunnerQt.h:211:  The parameter name "enabled" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/qt/TestRunnerQt.h:218:  Missing space inside { }.  [whitespace/braces] [5]
Tools/DumpRenderTree/qt/TestRunnerQt.h:222:  Missing space inside { }.  [whitespace/braces] [5]
Tools/DumpRenderTree/qt/TestRunnerQt.h:243:  Missing space inside { }.  [whitespace/braces] [5]
Tools/DumpRenderTree/qt/TestRunnerQt.h:254:  The parameter name "element" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/TestRunner.cpp:35:  Alphabetical sorting problem.  [build/include_order] [4]
Tools/DumpRenderTree/TestRunner.cpp:659:  Use 0 instead of NULL.  [readability/null] [5]
Tools/DumpRenderTree/TestRunner.cpp:1027:  Use 0 instead of NULL.  [readability/null] [5]
Tools/DumpRenderTree/TestRunner.cpp:1042:  Use 0 instead of NULL.  [readability/null] [5]
Tools/DumpRenderTree/TestRunner.cpp:1151:  Use 0 instead of NULL.  [readability/null] [5]
Tools/DumpRenderTree/TestRunner.cpp:1234:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/TestRunner.cpp:1234:  Use 0 instead of NULL.  [readability/null] [5]
Tools/DumpRenderTree/TestRunner.cpp:1235:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/TestRunner.cpp:1235:  Use 0 instead of NULL.  [readability/null] [5]
Tools/DumpRenderTree/TestRunner.cpp:1236:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/TestRunner.cpp:1236:  Use 0 instead of NULL.  [readability/null] [5]
Tools/DumpRenderTree/TestRunner.cpp:1246:  Use 0 instead of NULL.  [readability/null] [5]
Tools/DumpRenderTree/TestRunner.cpp:1814:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Tools/DumpRenderTree/TestRunner.h:46:  The parameter name "context" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/TestRunner.h:89:  The parameter name "acceptsEditing" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/TestRunner.h:93:  The parameter name "quota" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/TestRunner.h:98:  The parameter name "quota" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/TestRunner.h:101:  The parameter name "iconDatabaseEnabled" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/TestRunner.h:116:  The parameter name "cycles" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/TestRunner.h:121:  The parameter name "mode" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/TestRunner.h:123:  The parameter name "enable" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/TestRunner.h:124:  The parameter name "enable" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/TestRunner.h:247:  The parameter name "waitToDump" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/TestRunner.h:260:  The parameter name "windowIsKey" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/TestRunner.h:263:  The parameter name "alwaysAcceptCookies" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/TestRunner.h:323:  The parameter name "locale" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/TestRunner.h:329:  The parameter name "serialize" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/TestRunner.h:341:  The parameter name "context" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/TestRunner.h:426:  One space before end of linFailed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/DumpRenderTree/D..." exit_code: 1
e comments  [whitespace/comments] [5]
Tools/DumpRenderTree/win/TestRunnerBlackWin.cpp:45:  Alphabetical sorting problem.  [build/include_order] [4]
Tools/DumpRenderTree/win/TestRunnerBlackWin.cpp:54:  Use 'using namespace std;' instead of 'using std::string;'.  [build/using_std] [4]
Tools/DumpRenderTree/win/TestRunnerBlackWin.cpp:55:  Use 'using namespace std;' instead of 'using std::wstring;'.  [build/using_std] [4]
Tools/DumpRenderTree/win/TestRunnerBlackWin.cpp:275:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Tools/DumpRenderTree/win/TestRunnerBlackWin.cpp:626:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/win/TestRunnerBlackWin.cpp:684:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/DumpRenderTreeTestRunner.cpp:1586:  One line control clauses should not use braces.  [whitespace/braces] [4]
Tools/DumpRenderTree/gtk/TestRunnerBlackGtk.cpp:44:  Streams are highly discouraged.  [readability/streams] [3]
Tools/DumpRenderTree/gtk/TestRunnerBlackGtk.cpp:46:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 72 in 60 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Kent Tamura 2012-08-10 18:22:37 PDT
1.63MB patch is too large :-<
You don't need to do everything at once. You can rename file names without renaming class names and instance names.  You can rename one by one for each of ports.
Comment 5 Build Bot 2012-08-10 18:40:46 PDT
Comment on attachment 157847 [details]
Patch

Attachment 157847 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13464946
Comment 6 Gyuyoung Kim 2012-08-10 19:05:52 PDT
Comment on attachment 157847 [details]
Patch

Attachment 157847 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/13477385
Comment 7 Ryosuke Niwa 2012-08-10 19:11:19 PDT
(In reply to comment #4)
> 1.63MB patch is too large :-<
> You don't need to do everything at once. You can rename file names without renaming class names and instance names.  You can rename one by one for each of ports.

Okay. Let's do file names first then. Unfortunately, splitting the patch per port isn't easy because class names, etc... are shared among all ports.
Comment 8 Kent Tamura 2012-08-10 19:19:06 PDT
(In reply to comment #7)
> (In reply to comment #4)
> > 1.63MB patch is too large :-<
> > You don't need to do everything at once. You can rename file names without renaming class names and instance names.  You can rename one by one for each of ports.
> 
> Unfortunately, splitting the patch per port isn't easy because class names, etc... are shared among all ports.

Ah, It's right.
I think  DRT/Chromium doesn't depend on other DRT files and you can handle Chromium port separately.
Comment 9 Gyuyoung Kim 2012-08-13 00:36:50 PDT
Comment on attachment 157847 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=157847&action=review

> Tools/ChangeLog:41
> +        * DumpRenderTree/efl/TestRunnerBlackEfl.cpp: Copied from Tools/DumpRenderTree/efl/LayoutTestControllerEfl.cpp.

It looks TestRunnerEfl.cpp is correct, isn't it? When I change this to TestRunnerEfl.cpp, there is no build break. :)

> Tools/ChangeLog:45
> +        * DumpRenderTree/gtk/TestRunnerBlackGtk.cpp: Copied from Tools/DumpRenderTree/gtk/LayoutTestControllerGtk.cpp.

ditto?
Comment 10 Ryosuke Niwa 2012-08-13 22:57:48 PDT
Created attachment 158220 [details]
Patch
Comment 11 Tony Chang 2012-08-14 00:09:19 PDT
Comment on attachment 158220 [details]
Patch

rs=me
Comment 12 Ryosuke Niwa 2012-08-14 00:36:48 PDT
Committed r125516: <http://trac.webkit.org/changeset/125516>