Bug 40058 – testapi.c depends on the Core Foundation.

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Jędrzej Nowacki]

JSC C API tests could be compile only on Mac. It means that the API is not tested on other platforms and it could not be easily fixed or developed by people not using Mac.

I strongly believe that the dependency should be removed.

[Caio Marcelo de Oliveira Filho]

Created attachment 60308 [details]
Separate CF parts of testapi, compile support for Qt environment

This bootstraps support for Qt (and possible other platforms that have interest) in compiling the testapi.c. The build system bits were taken from jsc.pro.

[Caio Marcelo de Oliveira Filho]

Created attachment 60309 [details]
Qt version of the assert function that depended on CF

The WithCharacters API functions of JSStringRef.h work with 16-bit unicode characters. To test this representation the testapi.c used CFString and its helper functions. This patch implements a Qt version using QString.

Since it depends on the previous patch, I didn't mark for reviewing yet, but feel free to point out whatever problems you find out or whether there's a better way to do it.

[Caio Marcelo de Oliveira Filho]

Created attachment 60310 [details]
Tests for JSStringCreateWithCharacters

Depends on previous patches.

JSStringCreateWithCharacters() tests depended on CF specific functions. This commit adds tests that depend on a more generic function that both CF and Qt implement. This function creates the JSStringRef using CFString or QString as helpers to get the 16-bit buffer and then feed JSStringCreateWithCharacters(). It's similar in spirit to the assert of the previous patch.

Note: I couldn't test the CF version, so would be glad if someone with CF available could test it.

[Caio Marcelo de Oliveira Filho]

I added the three patches here since they are related to the same issue and depend on each other (2nd depends on 1st and 3rd on 2nd).

[WebKit Review Bot]

Attachment 60308 [details] did not build on win:
Build output: http://webkit-commit-queue.appspot.com/results/3345403

[Oliver Hunt]

Comment on attachment 60308 [details]
Separate CF parts of testapi, compile support for Qt environment

r=me

[Caio Marcelo de Oliveira Filho]

How can I get more details from the win build error at

http://webkit-commit-queue.appspot.com/results/3345403

[Caio Marcelo de Oliveira Filho]

Created attachment 61406 [details]
Separate CF parts of testapi, compile support for Qt environment, take 2

Make the function testJSStringRefCF() static. This was generating a compiler error in the CF code path (I tested now on MacOSX but probably it's the win EWS bot error).

[WebKit Commit Bot]

Comment on attachment 61406 [details]
Separate CF parts of testapi, compile support for Qt environment, take 2

Clearing flags on attachment: 61406

Committed r63262: <http://trac.webkit.org/changeset/63262>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Caio Marcelo de Oliveira Filho]

Created attachment 61462 [details]
Implement the assertEqualsAsCharactersPtr() for Qt

Create a Qt version of this assertion, to complement the existing CF code. This enables more of the testapi when compiling in the Qt platform.

Note: should this and the other patch go to bugs of their own?

[Simon Fraser (smfr)]

r63262 broke windows builds.

[Caio Marcelo de Oliveira Filho]

Created attachment 61470 [details]
Separate CF parts of testapi, compile support for Qt environment, take 3

The original commit was rolled out because it broke Win compilation (bug 42229). In VCProj, config.h wasn't available in the testapi utility compilation. Sorry folks.

The patch is the same as before, but adds the necessary bits for the Win build system (at least what I thought was necessary). Any suggestions on how to do it better is appreciated, since I'm not familiar with VCProj neiter have a win system available.

I'm marking this for review? but not cq? so the bots can check this patch and we can see if win passes. I will also remove the r? from the other patch to avoid confusion.

[Caio Marcelo de Oliveira Filho]

Created attachment 61506 [details]
Separate CF parts of testapi, compile support for Qt environment, take 4

Updating the patch for the win EWS bot. This time I just added the correct include directory in the compilation. Let's see if the bot can compile it this time.

[WebKit Review Bot]

Attachment 61506 [details] did not build on win:
Build output: http://webkit-commit-queue.appspot.com/results/3497297

[Adam Roben (:aroben)]

Comment on attachment 61506 [details]
Separate CF parts of testapi, compile support for Qt environment, take 4

> -		AdditionalIncludeDirectories=""$(ProjectDir)\..\..\API";"$(WebKitOutputDir)\include\WebCore\ForwardingHeaders";"$(WebKitOutputDir)\include\JavaScriptCore";"$(WebKitOutputDir)\include\private\JavaScriptCore";"$(WebKitOutputDir)\include";"$(WebKitOutputDir)\include\private";"$(WebKitLibrariesDir)\include";"$(WebKitLibrariesDir)\include\private""
> +		AdditionalIncludeDirectories=""$(ProjectDir)\..\..\API";../../;"$(WebKitOutputDir)\include\WebCore\ForwardingHeaders";"$(WebKitOutputDir)\include\JavaScriptCore";"$(WebKitOutputDir)\include\private\JavaScriptCore";"$(WebKitOutputDir)\include";"$(WebKitOutputDir)\include\private";"$(WebKitLibrariesDir)\include";"$(WebKitLibrariesDir)\include\private""

Please use backslashes and make the path relative to ProjectDir: $(ProjectDir)\..\..

[Caio Marcelo de Oliveira Filho]

Created attachment 61517 [details]
Separate CF parts of testapi, compile support for Qt environment, take 5

Following Adam Roben suggestion.

[WebKit Review Bot]

Attachment 61517 [details] did not build on win:
Build output: http://webkit-commit-queue.appspot.com/results/3529111

[chris fleizach]

Comment on attachment 61517 [details]
Separate CF parts of testapi, compile support for Qt environment, take 5

please fix so it builds on win