[Webkit-unassigned] [Bug 84446] [GTK] Build and run TestWebKitAPI WebKit2 unit tests

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=84446


Philippe Normand <pnormand at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #138083|review?                     |review-
               Flag|                            |




--- Comment #2 from Philippe Normand <pnormand at igalia.com>  2012-04-24 08:09:27 PST ---
(From update of attachment 138083)
View in context: https://bugs.webkit.org/attachment.cgi?id=138083&action=review

Looks good in general but I'm afraid I missed some things, maybe Martin can do another pass? This patch is quite big :). Anyway, some questions follow:

> Tools/ChangeLog:19
> +        * TestWebKitAPI/JavaScriptTest.cpp: Use
> +        JavaScriptCore/JSContextRef.h instead of
> +        JavaScriptCore/JavaScriptCore.h since it includes JSStringRefCF.h
> +        unconditionally.

Would it make sense to fix JavaScriptCore.h instead?

> Tools/Scripts/run-gtk-tests:83
> +        SkippedTest("TestWebKitAPI/WebKit2/TestNewFirstVisuallyNonEmptyLayout",
> +                    "Test times out"),
> +        SkippedTest("TestWebKitAPI/WebKit2/TestNewFirstVisuallyNonEmptyLayoutForImages",
> +                    "Test times out"),
> +        SkippedTest("TestWebKitAPI/WebKit2/TestWKConnectionTest",
> +                    "Test times out"),
> +        SkippedTest("TestWebKitAPI/WebKit2/TestRestoreSessionStateContainingFormData",
> +                    "Session State is not implemented in GTK+ port"),
> +        SkippedTest("TestWebKitAPI/WebKit2/TestSpacebarScrolling",
> +                    "Test fails")

Can these failures be tracked in bugzilla if possible?

> Tools/Scripts/run-gtk-tests:201
> +        self._test_env["TEST_WEBKIT_API_WEBKIT2_INJECTED_BUNDLE_PATH"] = os.path.abspath(os.path.join(self._get_build_directory(), "Libraries"))

Damn I forgot the "get build directory" is duplicated between this script and common.py. It'd be great to fix that at some point, in another patch.

> Tools/Scripts/run-gtk-tests:265
> +        tester_command = [test, "--gtest_throw_on_failure"]

Maybe that change should go in the other patch?

> Tools/TestWebKitAPI/GNUmakefile.am:61
> +testwebkitapi_wtf_tests_cppflags = \

Ditto

> Tools/TestWebKitAPI/GNUmakefile.am:69
> +testwebkitapi_wtf_tests_ldadd = \

Ditto

> Tools/TestWebKitAPI/GNUmakefile.am:74
> +testwebkitapi_wtf_tests_ldflags = \

Ditto

> Tools/TestWebKitAPI/GNUmakefile.am:172
> -Programs_TestWebKitAPI_WTF_TestCheckedArithmeticOperations_CPPFLAGS = $(testwebkitapi_tests_cppflags)
> -Programs_TestWebKitAPI_WTF_TestCheckedArithmeticOperations_LDADD = $(testwebkitapi_tests_ldadd)
> -Programs_TestWebKitAPI_WTF_TestCheckedArithmeticOperations_LDFLAGS = $(testwebkitapi_tests_ldflags)
> +Programs_TestWebKitAPI_WTF_TestCheckedArithmeticOperations_CPPFLAGS = $(testwebkitapi_wtf_tests_cppflags)
> +Programs_TestWebKitAPI_WTF_TestCheckedArithmeticOperations_LDADD = $(testwebkitapi_wtf_tests_ldadd)
> +Programs_TestWebKitAPI_WTF_TestCheckedArithmeticOperations_LDFLAGS = $(testwebkitapi_wtf_tests_ldflags)

Ok I'm tired of Ditto, you get it I guess :)

> Tools/TestWebKitAPI/gtk/PlatformUtilitiesGtk.cpp:65
> +    return g_filename_to_utf8(value, -1, 0, &bytesWritten, 0);

I find it a bit strange to use g_filename_to_utf8() here. Maybe some day we'll have env variables not dealing with file paths. Or maybe it's just this function that should be renamed :)

> Tools/TestWebKitAPI/gtk/main.cpp:37
> +    bool passed = TestWebKitAPI::TestsController::shared().run(argc, argv);
> +
> +    return passed ? EXIT_SUCCESS : EXIT_FAILURE;

This can be one line maybe?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.