[Webkit-unassigned] [Bug 69409] [GTK] [WebKit2] Make adding another unit test easier

[bugzilla-daemon at webkit.org]

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





--- Comment #2 from Carlos Garcia Campos <cgarcia at igalia.com>  2011-10-05 08:21:01 PST ---
(From update of attachment 109767)
View in context: https://bugs.webkit.org/attachment.cgi?id=109767&action=review

I like the idea of making test easier to write, I have some comments, though.

> GNUmakefile.am:211
> +include Source/WebKit2/UIProcess/API/gtk/tests/GNUmakefile.am

Why adding a new makefile for the tests?

> Source/WebKit2/UIProcess/API/gtk/tests/GNUmakefile.am:4
> +TEST_PROGS += \
> +	Programs/WebKit2APITests/TestWebKitWebContext \
> +	Programs/WebKit2APITests/TestWebKitWebView \
> +	Programs/WebKit2APITests/TestWebKitWebLoaderClient

Renaming the path and test names would break the bots, see bug https://bugs.webkit.org/show_bug.cgi?id=68992. so I would leave current names unless there's an important reason to rename them.

> Source/WebKit2/UIProcess/API/gtk/tests/GNUmakefile.am:35
> +noinst_LTLIBRARIES += Libraries/libWebKit2APITestCore.la

This library should probably be in WebCore or Tools, so that it can be used for wk1 tests too.

> Source/WebKit2/UIProcess/API/gtk/tests/GNUmakefile.am:38
> +	Source/WebKit2/UIProcess/API/gtk/tests/LoadTrackingTest.cpp \
> +	Source/WebKit2/UIProcess/API/gtk/tests/LoadTrackingTest.h \

These files are specific to the loading test, I think there should be only common classes in the library, since all tests will link ot it.

> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitWebLoaderClient.cpp:27
> +#include "config.h"
> +
> +#include "LoadTrackingTest.h"
> +#include <gtk/gtk.h>
> +#include <libsoup/soup.h>
> +#include <wtf/text/CString.h>
> +

I don't mind using C++ for unit tests, but I'm not sure about using internal API in the tests. I think tests should only use public gtk+ api.

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