Bug 44155 – [Qt] Add WebKitTestRunner's build files

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Zoltan Horvath]

Add WebKitTestRunner's files and modify WK2 build files to build WebKitTestRunner.

[Zoltan Horvath]

Created attachment 64675 [details]
first patch

Please comment.

[Kenneth Rohde Christiansen]

Comment on attachment 64675 [details]
first patch

WebKitTools/WebKitTestRunner/qt/TestControllerQt.cpp:32
 +  static void addQTDirToPATH()
huh? AddQTDirToPATH? QuickTime ?

WebKitTools/WebKitTestRunner/qt/main.cpp:33
 +      }
Why this?

Is this working? Don't you need the InjectBundle to work?

[Zoltan Horvath]

Created attachment 64686 [details]
second patch

(In reply to comment #2)
> (From update of attachment 64675 [details])
> WebKitTools/WebKitTestRunner/qt/TestControllerQt.cpp:32
>  +  static void addQTDirToPATH()
> huh? AddQTDirToPATH? QuickTime ?

Sry, I've uploaded a wrong patch with some unrelated change like this.
 
> WebKitTools/WebKitTestRunner/qt/main.cpp:33
>  +      }
> Why this?

It is copy pasted code from win implementation. I don't see reason for this, so I removed brackets now.

> Is this working? Don't you need the InjectBundle to work?

It is building and making the binary, but isn't working. 
We need InjectedBundle implementation, yes.

[Balazs Kelemen]

Created attachment 69466 [details]
third patch

[WebKit Review Bot]

Attachment 69466 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKitTools/WebKitTestRunner/qt/PlatformWebViewQt.cpp:27:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebKit2/UIProcess/API/C/WebKit2.h:53:  More than one command on the same line in if  [whitespace/parens] [4]
WebKitTools/WebKitTestRunner/qt/main.cpp:27:  Found other header before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebKitTools/WebKitTestRunner/qt/TestControllerQt.cpp:27:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 4 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Early Warning System Bot]

Attachment 69466 [details] did not build on qt:
Build output: http://queues.webkit.org/results/4229042

[Balazs Kelemen]

Created attachment 69941 [details]
Patch

[WebKit Review Bot]

Attachment 69941 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit2/UIProcess/API/C/WebKit2.h:53:  More than one command on the same line in if  [whitespace/parens] [4]
Total errors found: 1 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Balazs Kelemen]

Created attachment 69943 [details]
Patch

Forgot to add the new files in the previous.

[WebKit Review Bot]

Attachment 69943 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKitTools/WebKitTestRunner/qt/PlatformWebViewQt.cpp:27:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebKit2/UIProcess/API/C/WebKit2.h:53:  More than one command on the same line in if  [whitespace/parens] [4]
WebKitTools/WebKitTestRunner/qt/main.cpp:27:  Found other header before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebKitTools/WebKitTestRunner/qt/TestControllerQt.cpp:27:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 4 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Balazs Kelemen]

Created attachment 69950 [details]
Patch

Removed accidentally added implementation that need to be in a follow up patch.

[WebKit Review Bot]

Attachment 69950 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKitTools/WebKitTestRunner/qt/PlatformWebViewQt.cpp:27:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebKit2/UIProcess/API/C/WebKit2.h:53:  More than one command on the same line in if  [whitespace/parens] [4]
WebKitTools/WebKitTestRunner/qt/main.cpp:27:  Found other header before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebKitTools/WebKitTestRunner/qt/TestControllerQt.cpp:27:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 4 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Kenneth Rohde Christiansen]

Comment on attachment 69950 [details]
Patch

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

> WebKit2/UIProcess/API/C/WebKit2.h:53
> -#if !__APPLE__ || __OBJC__
> +#if (defined(__APPLE__) && __APPLE__) || (defined(__OBJC__) && __OBJC__)

The first seems to be checking for NOT __APPLE__

> WebKitTools/WebKitTestRunner/qt/PlatformWebViewQt.cpp:35
> +    // Implement
> +    notImplemented();

This comment seems a bit useless

> WebKitTools/WebKitTestRunner/qt/TestControllerQt.cpp:41
> +}
> +void TestController::platformInitialize()

needs newline between those two

[Balazs Kelemen]

Created attachment 69954 [details]
Patch

Corrected the issues noticed by Kenneth.

[WebKit Review Bot]

Attachment 69954 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKitTools/WebKitTestRunner/qt/PlatformWebViewQt.cpp:27:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebKitTools/WebKitTestRunner/qt/main.cpp:27:  Found other header before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebKitTools/WebKitTestRunner/qt/TestControllerQt.cpp:27:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 3 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Early Warning System Bot]

Attachment 69954 [details] did not build on qt:
Build output: http://queues.webkit.org/results/4255030

[Balazs Kelemen]

Oops. This depends on 45393. Instead of set the dependency I am going to upload the patch without actually switching on the build of WebKitTestRunner. This way we can move forward and switch it on when the build will be stable.

[Balazs Kelemen]

Created attachment 69993 [details]
Patch

Same as the last one without actually turn on the build of WTR.

[WebKit Review Bot]

Attachment 69993 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKitTools/WebKitTestRunner/qt/PlatformWebViewQt.cpp:27:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebKitTools/WebKitTestRunner/qt/main.cpp:27:  Found other header before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebKitTools/WebKitTestRunner/qt/TestControllerQt.cpp:27:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 3 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Kenneth Rohde Christiansen]

Comment on attachment 69993 [details]
Patch

When landing please make sure to stay around to make sure this does break any other port

[Andras Becsi]

Comment on attachment 69993 [details]
Patch

Clearing flags on attachment: 69993

Committed r69244: <http://trac.webkit.org/changeset/69244>

[Andras Becsi]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

http://trac.webkit.org/changeset/69244 might have broken Chromium Win Release

[WebKit Review Bot]

http://trac.webkit.org/changeset/69245 might have broken Chromium Win Release