Bug 40058 - testapi.c depends on the Core Foundation.
Summary: testapi.c depends on the Core Foundation.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Enhancement
Assignee: Nobody
URL:
Keywords:
Depends on: 42229
Blocks: 31863
  Show dependency treegraph
 
Reported: 2010-06-02 06:57 PDT by Jędrzej Nowacki
Modified: 2010-09-22 00:22 PDT (History)
9 users (show)

See Also:


Attachments
Separate CF parts of testapi, compile support for Qt environment (16.42 KB, patch)
2010-07-01 16:27 PDT, Caio Marcelo de Oliveira Filho
oliver: review+
Details | Formatted Diff | Diff
Qt version of the assert function that depended on CF (7.98 KB, patch)
2010-07-01 16:31 PDT, Caio Marcelo de Oliveira Filho
no flags Details | Formatted Diff | Diff
Tests for JSStringCreateWithCharacters (6.08 KB, patch)
2010-07-01 16:35 PDT, Caio Marcelo de Oliveira Filho
no flags Details | Formatted Diff | Diff
Separate CF parts of testapi, compile support for Qt environment, take 2 (16.41 KB, patch)
2010-07-13 12:44 PDT, Caio Marcelo de Oliveira Filho
no flags Details | Formatted Diff | Diff
Implement the assertEqualsAsCharactersPtr() for Qt (7.84 KB, patch)
2010-07-13 20:42 PDT, Caio Marcelo de Oliveira Filho
no flags Details | Formatted Diff | Diff
Separate CF parts of testapi, compile support for Qt environment, take 3 (18.65 KB, patch)
2010-07-13 22:26 PDT, Caio Marcelo de Oliveira Filho
no flags Details | Formatted Diff | Diff
Separate CF parts of testapi, compile support for Qt environment, take 4 (18.06 KB, patch)
2010-07-14 05:16 PDT, Caio Marcelo de Oliveira Filho
no flags Details | Formatted Diff | Diff
Separate CF parts of testapi, compile support for Qt environment, take 5 (18.08 KB, patch)
2010-07-14 07:41 PDT, Caio Marcelo de Oliveira Filho
cfleizach: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jędrzej Nowacki 2010-06-02 06:57:33 PDT
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.
Comment 1 Caio Marcelo de Oliveira Filho 2010-07-01 16:27:23 PDT
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.
Comment 2 Caio Marcelo de Oliveira Filho 2010-07-01 16:31:24 PDT
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.
Comment 3 Caio Marcelo de Oliveira Filho 2010-07-01 16:35:34 PDT
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.
Comment 4 Caio Marcelo de Oliveira Filho 2010-07-01 16:40:34 PDT
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).
Comment 5 WebKit Review Bot 2010-07-02 02:59:57 PDT
Attachment 60308 [details] did not build on win:
Build output: http://webkit-commit-queue.appspot.com/results/3345403
Comment 6 Oliver Hunt 2010-07-02 14:36:20 PDT
Comment on attachment 60308 [details]
Separate CF parts of testapi, compile support for Qt environment

r=me
Comment 7 Caio Marcelo de Oliveira Filho 2010-07-06 08:26:27 PDT
How can I get more details from the win build error at

http://webkit-commit-queue.appspot.com/results/3345403
Comment 8 Caio Marcelo de Oliveira Filho 2010-07-13 12:44:16 PDT
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).
Comment 9 WebKit Commit Bot 2010-07-13 17:00:15 PDT
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>
Comment 10 WebKit Commit Bot 2010-07-13 17:00:21 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 Caio Marcelo de Oliveira Filho 2010-07-13 20:42:44 PDT
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?
Comment 12 Simon Fraser (smfr) 2010-07-13 21:08:22 PDT
r63262 broke windows builds.
Comment 13 Caio Marcelo de Oliveira Filho 2010-07-13 22:26:16 PDT
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.
Comment 14 Caio Marcelo de Oliveira Filho 2010-07-14 05:16:34 PDT
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.
Comment 15 WebKit Review Bot 2010-07-14 06:31:24 PDT
Attachment 61506 [details] did not build on win:
Build output: http://webkit-commit-queue.appspot.com/results/3497297
Comment 16 Adam Roben (:aroben) 2010-07-14 07:20:16 PDT
Comment on attachment 61506 [details]
Separate CF parts of testapi, compile support for Qt environment, take 4

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

Please use backslashes and make the path relative to ProjectDir: $(ProjectDir)\..\..
Comment 17 Caio Marcelo de Oliveira Filho 2010-07-14 07:41:45 PDT
Created attachment 61517 [details]
Separate CF parts of testapi, compile support for Qt environment, take 5

Following Adam Roben suggestion.
Comment 18 WebKit Review Bot 2010-07-14 09:22:52 PDT
Attachment 61517 [details] did not build on win:
Build output: http://webkit-commit-queue.appspot.com/results/3529111
Comment 19 chris fleizach 2010-09-22 00:22:37 PDT
Comment on attachment 61517 [details]
Separate CF parts of testapi, compile support for Qt environment, take 5

please fix so it builds on win