Bug 68509 - [EFL] Implementation of Unit Tests framework for the WebKit-Efl port
Summary: [EFL] Implementation of Unit Tests framework for the WebKit-Efl port
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: All Linux
: P2 Enhancement
Assignee: Krzysztof Czech
URL:
Keywords:
Depends on: 90136
Blocks: 68510 86092 87251 90525
  Show dependency treegraph
 
Reported: 2011-09-21 01:29 PDT by Krzysztof Czech
Modified: 2012-07-04 00:58 PDT (History)
13 users (show)

See Also:


Attachments
Patch (20.24 KB, patch)
2011-09-21 01:30 PDT, Krzysztof Czech
rakuco: review-
Details | Formatted Diff | Diff
Patch (20.85 KB, patch)
2011-10-05 02:39 PDT, Krzysztof Czech
no flags Details | Formatted Diff | Diff
Patch (20.84 KB, patch)
2011-10-05 07:12 PDT, Krzysztof Czech
no flags Details | Formatted Diff | Diff
Patch (20.84 KB, patch)
2011-10-19 01:28 PDT, Krzysztof Czech
no flags Details | Formatted Diff | Diff
Corrected patch with build scripts and implementation (23.60 KB, patch)
2012-01-13 08:28 PST, Krzysztof Czech
gyuyoung.kim: commit-queue-
Details | Formatted Diff | Diff
Corrected patch with build scripts and implementation (23.70 KB, patch)
2012-01-16 06:05 PST, Krzysztof Czech
gyuyoung.kim: commit-queue-
Details | Formatted Diff | Diff
Corrected patch with build scripts and implementation (23.85 KB, patch)
2012-01-16 07:03 PST, Krzysztof Czech
no flags Details | Formatted Diff | Diff
Corrected patch with build scripts and implementation (23.61 KB, patch)
2012-01-17 00:25 PST, Krzysztof Czech
no flags Details | Formatted Diff | Diff
Corrected patch with build scripts and implementation (24.06 KB, patch)
2012-05-14 06:41 PDT, Krzysztof Czech
no flags Details | Formatted Diff | Diff
Corrected patch with build scripts and implementation (24.08 KB, patch)
2012-05-18 00:49 PDT, Krzysztof Czech
no flags Details | Formatted Diff | Diff
Corrected patch with build scripts and implementation (24.33 KB, patch)
2012-06-06 06:59 PDT, Krzysztof Czech
gyuyoung.kim: commit-queue-
Details | Formatted Diff | Diff
Corrected patch with build scripts and implementation (22.85 KB, patch)
2012-06-13 03:29 PDT, Krzysztof Czech
gyuyoung.kim: commit-queue-
Details | Formatted Diff | Diff
Corrected patch with build scripts and implementation - correcting build problems (22.90 KB, patch)
2012-06-14 03:51 PDT, Krzysztof Czech
no flags Details | Formatted Diff | Diff
Corrected patch with build scripts and implementation - updating (23.38 KB, patch)
2012-06-19 08:24 PDT, Krzysztof Czech
g.czajkowski: commit-queue-
Details | Formatted Diff | Diff
Corrected patch with build scripts and implementation (22.98 KB, patch)
2012-06-21 02:23 PDT, Krzysztof Czech
no flags Details | Formatted Diff | Diff
Corrected patch with build scripts and implementation (22.82 KB, patch)
2012-06-21 02:50 PDT, Krzysztof Czech
cshu: review+
Details | Formatted Diff | Diff
Corrected patch with build scripts and implementation - updating (22.81 KB, patch)
2012-06-27 07:48 PDT, Krzysztof Czech
no flags Details | Formatted Diff | Diff
Corrected patch with build scripts and implementation - updating (22.70 KB, patch)
2012-06-29 08:12 PDT, Krzysztof Czech
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Krzysztof Czech 2011-09-21 01:29:06 PDT
Unit Tests for the WebKit-Efl port are based on the gtest library.
There's also been added a simple framework for running tests that require instance of a webview and example of unit tests.
Comment 1 Krzysztof Czech 2011-09-21 01:30:48 PDT
Created attachment 108121 [details]
Patch
Comment 2 Gyuyoung Kim 2011-09-21 01:47:29 PDT
Comment on attachment 108121 [details]
Patch

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

Could you let me know how to run? and what is the result ?

> Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestView.cpp:40
> +EFLTestEcoreEvas::EFLTestEcoreEvas(const char *engine_name, int viewport_x, int viewport_y, int viewport_w, int viewport_h, const char *extra_options)

Move '*' type to data type.

> Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestView.cpp:76
> +EFLTestView::EFLTestView(Evas *evas, const char* url)

ditto.

> Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestView.cpp:98
> +EFLTestView::EFLTestView(Evas *evas, EwkViewType type, const char* url, int width, int height)

ditto.

> Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestView.h:30
> +    EFLTestEcoreEvas(const char *engine_name, int viewport_x, int viewport_y, int viewport_w, int viewport_h, const char *extra_options);

ditto.

In addition, do not use _ for parameter name. We are trying to use WebKit coding style except for public APIs

> Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestView.h:37
> +    Ecore_Evas *m_ecore_evas;

ditto.
Comment 3 Krzysztof Czech 2011-09-21 02:00:08 PDT
You have to applied all the patches first, build WebKit again and run ./Tools/Scripts/run-efl-tests. I added two simple unit tests to demonstrate what is all about. The result you will see after run it run-efl-tests script.
Comment 4 Krzysztof Czech 2011-09-21 02:04:49 PDT
I meant all patches by 68509, 68510, 68511
Comment 5 Raphael Kubo da Costa (:rakuco) 2011-09-21 07:10:05 PDT
Comment on attachment 108121 [details]
Patch

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

The patch looks OK overall (even though I've never used gtest).

However, there are coding style issues that need to be fixed (I see m_fooBar and m_foo_bar as member variables, there are the issues Gyuyoung pointed out). I would also prefer to use as many "native" C++ and WebKit types as possible, so you have bool instead of Eina_Bool where possible, and you use Strings instead of char*'s whose memory you need to manage manually. Speaking of memory management, you could also use OwnPtrs for the Evas_Objects and other pointers so you also don't need to manage their memory by hand.

> Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestLauncher.cpp:32
> +Eina_Bool EFLTestLauncher::init()

Shouldn't you have to call the respective _shutdown() functions as well?

> Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestView.cpp:73
> +    m_url = createTestUrl(EFLUnitTests::Config::defaultTestPage);

Why the double assignment?

> Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestView.cpp:84
> +    m_url = createTestUrl(url);

Why the double assignment?

> Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestView.cpp:95
> +    m_url = createTestUrl(url);

Why the double assignment?

> Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestView.cpp:106
> +    m_url = createTestUrl(url);

Why the double assignment?

> Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestView.h:61
> +protected:

Small nitpick: an extra empty line before this keyword would be nice

> Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestView.h:63
> +private:

Ditto.
Comment 6 Krzysztof Czech 2011-09-22 01:35:35 PDT
(In reply to comment #5)
> (From update of attachment 108121 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=108121&action=review
> 
> The patch looks OK overall (even though I've never used gtest).
> 
> However, there are coding style issues that need to be fixed (I see m_fooBar and m_foo_bar as member variables, there are the issues Gyuyoung pointed out). I would also prefer to use as many "native" C++ and WebKit types as possible, so you have bool instead of Eina_Bool where possible, and you use Strings instead of char*'s whose memory you need to manage manually. Speaking of memory management, you could also use OwnPtrs for the Evas_Objects and other pointers so you also don't need to manage their memory by hand.

I think it might be a problem using WebKit's OwnPtrs in unit tests context.
Tests are linked against WebKit library. Those internals like OwnPtr are not publicly exported. Other way I am not sure if is a good idea to mix Evas_Object's with smart pointers. How do you think ?
> 
> > Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestLauncher.cpp:32
> > +Eina_Bool EFLTestLauncher::init()
> 
> Shouldn't you have to call the respective _shutdown() functions as well?
> 
> > Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestView.cpp:73
> > +    m_url = createTestUrl(EFLUnitTests::Config::defaultTestPage);
> 
> Why the double assignment?
You mean one in initialization list and one above. In case something goes wrong with the above initialization m_url points at null. Other way is good to have default initialization.
> 
> > Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestView.cpp:84
> > +    m_url = createTestUrl(url);
> 
> Why the double assignment?
> 
> > Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestView.cpp:95
> > +    m_url = createTestUrl(url);
> 
> Why the double assignment?
> 
> > Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestView.cpp:106
> > +    m_url = createTestUrl(url);
> 
> Why the double assignment?
> 
> > Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestView.h:61
> > +protected:
> 
> Small nitpick: an extra empty line before this keyword would be nice
> 
> > Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestView.h:63
> > +private:
> 
> Ditto.
Comment 7 Krzysztof Czech 2011-09-26 07:34:44 PDT
> prefer to use as many "native" C++ and WebKit types as possible, so you have
> bool instead of Eina_Bool where possible, and you use Strings instead of char*

I wondering if including c++ string is really necessary, in this particular context having char* is OK, since only one array is declared on heap and is cleaned when test goes out of the scope.
Comment 8 Krzysztof Czech 2011-10-05 02:39:46 PDT
Created attachment 109766 [details]
Patch
Comment 9 Krzysztof Czech 2011-10-05 06:58:19 PDT
Comment on attachment 109766 [details]
Patch

>diff --git a/Source/WebKit/efl/ChangeLog b/Source/WebKit/efl/ChangeLog
>index 69c60a2..c6336c7 100755
>--- a/Source/WebKit/efl/ChangeLog
>+++ b/Source/WebKit/efl/ChangeLog
>@@ -1,3 +1,47 @@
>+2011-09-21  Krzysztof Czech  <k.czech@samsung.com>
>+
>+        Implementation of Unit Tests framework for the WebKit-Efl port.
>+        https://bugs.webkit.org/show_bug.cgi?id=XXXXX
>+
>+        Unit Tests for the WebKit-Efl port are based on the gtest library.
>+        There's also been added a simple framework for running tests that require instance of a webview
>+        and example of unit tests.
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * tests/default_test_page.html: Added.
>+        * tests/src/EFLTestsRun.cpp: Added.
>+        (main):
>+        * tests/src/EwkEditableTests.cpp: Added.
>+        (editableSetTestCallback):
>+        (TEST):
>+        * tests/src/EwkUriTests.cpp: Added.
>+        (ewkUriSetTestCallback):
>+        (TEST):
>+        * tests/src/UnitTestLauncher/EFLTestConfig.h: Added.
>+        * tests/src/UnitTestLauncher/EFLTestLauncher.cpp: Added.
>+        (EFLUnitTests::EFLTestLauncher::init):
>+        (EFLUnitTests::EFLTestLauncher::startTest):
>+        (EFLUnitTests::EFLTestLauncher::endTest):
>+        (EFLUnitTests::EFLTestLauncher::createTest):
>+        (EFLUnitTests::EFLTestLauncher::runTest):
>+        * tests/src/UnitTestLauncher/EFLTestLauncher.h: Added.
>+        * tests/src/UnitTestLauncher/EFLTestView.cpp: Added.
>+        (EFLUnitTests::EFLTestEcoreEvas::EFLTestEcoreEvas):
>+        (EFLUnitTests::EFLTestEcoreEvas::~EFLTestEcoreEvas):
>+        (EFLUnitTests::EFLTestEcoreEvas::getEvas):
>+        (EFLUnitTests::EFLTestEcoreEvas::show):
>+        (EFLUnitTests::EFLTestView::EFLTestView):
>+        (EFLUnitTests::EFLTestView::~EFLTestView):
>+        (EFLUnitTests::EFLTestView::createTestUrl):
>+        (EFLUnitTests::EFLTestView::init):
>+        (EFLUnitTests::EFLTestView::show):
>+        (EFLUnitTests::EFLTestView::getMainFrame):
>+        (EFLUnitTests::EFLTestView::getEvas):
>+        (EFLUnitTests::EFLTestView::bindEvents):
>+        * tests/src/UnitTestLauncher/EFLTestView.h: Added.
>+        (EFLUnitTests::EFLTestView::getWebView):
>+
> 2011-09-17  Mihai Parparita  <mihaip@chromium.org>
> 
>         FrameLoaderClient BackForwardList-related methods are unsued
>diff --git a/Source/WebKit/efl/tests/default_test_page.html b/Source/WebKit/efl/tests/default_test_page.html
>new file mode 100644
>index 0000000..edd81e7
>--- /dev/null
>+++ b/Source/WebKit/efl/tests/default_test_page.html
>@@ -0,0 +1,6 @@
>+<HTML>
>+<BODY>
>+<H2 align="center">EFL Unit Tests</H2>
>+<H2 align="center">Default Testing Web Page</H2>
>+</BODY>
>+</HTML>
>diff --git a/Source/WebKit/efl/tests/src/EFLTestsRun.cpp b/Source/WebKit/efl/tests/src/EFLTestsRun.cpp
>new file mode 100644
>index 0000000..a22a61c
>--- /dev/null
>+++ b/Source/WebKit/efl/tests/src/EFLTestsRun.cpp
>@@ -0,0 +1,30 @@
>+/*
>+    Copyright (C) 2011 Samsung Electronics
>+
>+    This library is free software; you can redistribute it and/or
>+    modify it under the terms of the GNU Lesser General Public
>+    License as published by the Free Software Foundation; either
>+    version 2.1 of the License, or (at your option) any later version.
>+
>+    This library is distributed in the hope that it will be useful,
>+    but WITHOUT ANY WARRANTY; without even the implied warranty of
>+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>+    Lesser General Public License for more details.
>+
>+    You should have received a copy of the GNU Lesser General Public License
>+    along with this library; if not, write to the Free Software Foundation,
>+    Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>+*/
>+
>+#include <EFLTestLauncher.h>
>+#include <gtest/gtest.h>
>+#include <stdio.h>
>+
>+using namespace EFLUnitTests;
>+
>+int main(int argc, char** argv)
>+{
>+    atexit(EFLTestLauncher::shutdownAll);
>+    ::testing::InitGoogleTest(&argc, argv);
>+    return RUN_ALL_TESTS();
>+}
>diff --git a/Source/WebKit/efl/tests/src/EwkEditableTests.cpp b/Source/WebKit/efl/tests/src/EwkEditableTests.cpp
>new file mode 100644
>index 0000000..e866026
>--- /dev/null
>+++ b/Source/WebKit/efl/tests/src/EwkEditableTests.cpp
>@@ -0,0 +1,35 @@
>+/*
>+    Copyright (C) 2011 Samsung Electronics
>+
>+    This library is free software; you can redistribute it and/or
>+    modify it under the terms of the GNU Lesser General Public
>+    License as published by the Free Software Foundation; either
>+    version 2.1 of the License, or (at your option) any later version.
>+
>+    This library is distributed in the hope that it will be useful,
>+    but WITHOUT ANY WARRANTY; without even the implied warranty of
>+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>+    Lesser General Public License for more details.
>+
>+    You should have received a copy of the GNU Lesser General Public License
>+    along with this library; if not, write to the Free Software Foundation,
>+    Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>+*/
>+
>+#include "EFLTestLauncher.h"
>+#include "EWebKit.h"
>+#include <gtest/gtest.h>
>+
>+using namespace EFLUnitTests;
>+
>+void editableSetTestCallback(void* eventInfo, Evas_Object* o, void* data)
>+{
>+    EXPECT_EQ(EINA_TRUE, ewk_view_editable_set(o, EINA_FALSE));
>+    EXPECT_EQ(EINA_FALSE, ewk_view_editable_get(o));
>+    END_TEST();
>+}
>+
>+TEST(EwkEditableTests, EditableSetTest)
>+{
>+    RUN_TEST(editableSetTestCallback, "load,finished", 0);
>+}
>diff --git a/Source/WebKit/efl/tests/src/EwkUriTests.cpp b/Source/WebKit/efl/tests/src/EwkUriTests.cpp
>new file mode 100644
>index 0000000..eccd060
>--- /dev/null
>+++ b/Source/WebKit/efl/tests/src/EwkUriTests.cpp
>@@ -0,0 +1,34 @@
>+/*
>+    Copyright (C) 2011 Samsung Electronics
>+
>+    This library is free software; you can redistribute it and/or
>+    modify it under the terms of the GNU Lesser General Public
>+    License as published by the Free Software Foundation; either
>+    version 2.1 of the License, or (at your option) any later version.
>+
>+    This library is distributed in the hope that it will be useful,
>+    but WITHOUT ANY WARRANTY; without even the implied warranty of
>+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>+    Lesser General Public License for more details.
>+
>+    You should have received a copy of the GNU Lesser General Public License
>+    along with this library; if not, write to the Free Software Foundation,
>+    Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>+*/
>+
>+#include "EFLTestLauncher.h"
>+#include "EWebKit.h"
>+#include <gtest/gtest.h>
>+
>+using namespace EFLUnitTests;
>+
>+void ewkUriSetTestCallback(void* eventInfo, Evas_Object* o, void* data)
>+{
>+    EXPECT_STREQ("http://www.webkit.org/", ewk_view_uri_get(o));
>+    END_TEST();
>+}
>+
>+TEST(EwkUriTests, EwkUriSetTest)
>+{
>+    RUN_TEST("http://www.webkit.org", ewkUriSetTestCallback, "load,finished", 0);
>+}
>diff --git a/Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestConfig.h b/Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestConfig.h
>new file mode 100644
>index 0000000..ddcfcb6
>--- /dev/null
>+++ b/Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestConfig.h
>@@ -0,0 +1,31 @@
>+/*
>+    Copyright (C) 2011 Samsung Electronics
>+
>+    This library is free software; you can redistribute it and/or
>+    modify it under the terms of the GNU Lesser General Public
>+    License as published by the Free Software Foundation; either
>+    version 2.1 of the License, or (at your option) any later version.
>+
>+    This library is distributed in the hope that it will be useful,
>+    but WITHOUT ANY WARRANTY; without even the implied warranty of
>+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>+    Lesser General Public License for more details.
>+
>+    You should have received a copy of the GNU Lesser General Public License
>+    along with this library; if not, write to the Free Software Foundation,
>+    Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>+*/
>+
>+#ifndef EFLTestConfig_h
>+#define EFLTestConfig_h
>+
>+namespace EFLUnitTests {
>+namespace Config {
>+static const int defaultViewWidth = 600;
>+static const int defaultViewHeight = 800;
>+static const char* defaultThemePath = DEFAULT_THEME_PATH"/default.edj";
>+static const char* defaultTestPage = "file://"DEFAULT_TEST_PAGE_DIR"/default_test_page.html";
>+}
>+}
>+
>+#endif
>diff --git a/Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestLauncher.cpp b/Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestLauncher.cpp
>new file mode 100644
>index 0000000..0b46e18
>--- /dev/null
>+++ b/Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestLauncher.cpp
>@@ -0,0 +1,102 @@
>+/*
>+    Copyright (C) 2011 Samsung Electronics
>+
>+    This library is free software; you can redistribute it and/or
>+    modify it under the terms of the GNU Lesser General Public
>+    License as published by the Free Software Foundation; either
>+    version 2.1 of the License, or (at your option) any later version.
>+
>+    This library is distributed in the hope that it will be useful,
>+    but WITHOUT ANY WARRANTY; without even the implied warranty of
>+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>+    Lesser General Public License for more details.
>+
>+    You should have received a copy of the GNU Lesser General Public License
>+    along with this library; if not, write to the Free Software Foundation,
>+    Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>+*/
>+
>+#include "config.h"
>+#include "EFLTestLauncher.h"
>+
>+#include "EFLTestConfig.h"
>+#include "EFLTestView.h"
>+#include "EWebKit.h"
>+
>+#include <Ecore.h>
>+#include <Edje.h>
>+#include <Eina.h>
>+
>+namespace EFLUnitTests {
>+
>+bool EFLTestLauncher::init()
>+{
>+    if (!ecore_evas_init())
>+        return false;
>+
>+    if (!edje_init()) {
>+        ecore_evas_shutdown();
>+        return false;
>+    }
>+    ewk_init();
>+    return true;
>+}
>+
>+void EFLTestLauncher::shutdown()
>+{
>+    ecore_evas_shutdown();
>+    edje_shutdown();
>+    ewk_shutdown();
>+}
>+
>+void EFLTestLauncher::shutdownAll()
>+{
>+    int count = 0;
>+
>+    while ((count = ecore_evas_shutdown()) > 0) { }
>+    while ((count = edje_shutdown()) > 0) { }
>+    while ((count = ewk_shutdown()) > 0) { }
>+}
>+
>+void EFLTestLauncher::startTest()
>+{
>+    ecore_main_loop_begin();
>+}
>+
>+void EFLTestLauncher::endTest()
>+{
>+    ecore_main_loop_quit();
>+}
>+
>+bool EFLTestLauncher::createTest(const char* url, void (*event_callback)(void*, Evas_Object*, void*), const char* event_name, void* event_data)
>+{
>+    EFL_INIT_RET();
>+
>+    EFLTestEcoreEvas evas;
>+    if (!evas.getEvas())
>+        return false;
>+    evas.show();
>+
>+    EFLTestView view(evas.getEvas(), url);
>+    if (!view.init())
>+        return false;
>+
>+    view.bindEvents(event_callback, event_name, event_data);
>+    view.show();
>+
>+    START_TEST();
>+
>+    return true;
>+}
>+
>+bool EFLTestLauncher::runTest(void (*event_callback)(void*, Evas_Object*, void*), const char* event_name, void* event_data)
>+{
>+    return createTest(EFLUnitTests::Config::defaultTestPage, event_callback, event_name, event_data);
>+}
>+
>+bool EFLTestLauncher::runTest(const char* url, void (*event_callback)(void*, Evas_Object*, void*), const char* event_name, void* event_data)
>+{
>+    return createTest(url, event_callback, event_name, event_data);
>+}
>+
>+}
>diff --git a/Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestLauncher.h b/Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestLauncher.h
>new file mode 100644
>index 0000000..746f95d
>--- /dev/null
>+++ b/Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestLauncher.h
>@@ -0,0 +1,88 @@
>+/*
>+    Copyright (C) 2011 Samsung Electronics
>+
>+    This library is free software; you can redistribute it and/or
>+    modify it under the terms of the GNU Lesser General Public
>+    License as published by the Free Software Foundation; either
>+    version 2.1 of the License, or (at your option) any later version.
>+
>+    This library is distributed in the hope that it will be useful,
>+    but WITHOUT ANY WARRANTY; without even the implied warranty of
>+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>+    Lesser General Public License for more details.
>+
>+    You should have received a copy of the GNU Lesser General Public License
>+    along with this library; if not, write to the Free Software Foundation,
>+    Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>+*/
>+
>+#ifndef EFLTestLauncher_h
>+#define EFLTestLauncher_h
>+
>+#include <Evas.h>
>+
>+#ifdef GTEST_TEST_FRAMEWORK
>+#include <gtest/gtest.h>
>+#endif
>+
>+#ifdef GTEST_TEST_FRAMEWORK
>+    #define RUN_TEST(args...)   \
>+        do {    \
>+            ASSERT_EQ(true, EFLTestLauncher::runTest(args));    \
>+        } while (0)
>+#else
>+    #define RUN_TEST(args...)   \
>+    do {    \
>+        EFLTestLauncher::runTest(args); \
>+    } while (0)
>+#endif
>+
>+#define START_TEST()    \
>+    do {    \
>+        EFLTestLauncher::startTest();   \
>+    } while (0)
>+
>+#define END_TEST()    \
>+    do {    \
>+        EFLTestLauncher::endTest(); \
>+    } while (0)
>+
>+#define EFL_INIT_RET()  \
>+    do {    \
>+        if (!EFLTestLauncher::init())   \
>+            return false;    \
>+    } while (0)
>+
>+#define EFL_INIT()  \
>+    do {    \
>+        EFLTestLauncher::init();    \
>+    } while (0)
>+
>+#define EFL_SHUTDOWN()  \
>+    do {    \
>+        EFLTestLauncher::shutdown();    \
>+    } while (0)
>+
>+#define EFL_SHUTDOWN_ALL()  \
>+    do {    \
>+        EFLTestLauncher::shutdownAll(); \
>+    } while (0)
>+
>+namespace EFLUnitTests {
>+
>+class EFLTestLauncher {
>+    static bool createTest(const char* url, void (*event_callback)(void*, Evas_Object*, void*), const char* event_name, void* event_data);
>+public:
>+    static bool init();
>+    static void shutdown();
>+    static void shutdownAll();
>+    static void startTest();
>+    static void endTest();
>+
>+    static bool runTest(const char* url, void (*event_callback)(void*, Evas_Object*, void*), const char* event_name, void* event_data);
>+    static bool runTest(void (*event_callback)(void*, Evas_Object*, void*), const char* event_name, void* event_data);
>+};
>+
>+}
>+
>+#endif
>diff --git a/Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestView.cpp b/Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestView.cpp
>new file mode 100644
>index 0000000..8053b3a
>--- /dev/null
>+++ b/Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestView.cpp
>@@ -0,0 +1,177 @@
>+/*
>+    Copyright (C) 2011 Samsung Electronics
>+
>+    This library is free software; you can redistribute it and/or
>+    modify it under the terms of the GNU Lesser General Public
>+    License as published by the Free Software Foundation; either
>+    version 2.1 of the License, or (at your option) any later version.
>+
>+    This library is distributed in the hope that it will be useful,
>+    but WITHOUT ANY WARRANTY; without even the implied warranty of
>+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>+    Lesser General Public License for more details.
>+
>+    You should have received a copy of the GNU Lesser General Public License
>+    along with this library; if not, write to the Free Software Foundation,
>+    Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>+*/
>+
>+#include "config.h"
>+#include "EFLTestView.h"
>+
>+#include "EFLTestConfig.h"
>+#include "EWebKit.h"
>+
>+#include <Ecore.h>
>+#include <Ecore_File.h>
>+#include <Edje.h>
>+#include <Eina.h>
>+
>+#include <string.h>
>+
>+namespace EFLUnitTests {
>+
>+EFLTestEcoreEvas::EFLTestEcoreEvas()
>+    : m_ecoreEvas(0)
>+{
>+    m_ecoreEvas = ecore_evas_new(0, 0, 0, EFLUnitTests::Config::defaultViewWidth, EFLUnitTests::Config::defaultViewHeight, 0);
>+}
>+
>+EFLTestEcoreEvas::EFLTestEcoreEvas(const char* engine_name, int viewport_x, int viewport_y, int viewport_w, int viewport_h, const char* extra_options)
>+    : m_ecoreEvas(0)
>+{
>+    m_ecoreEvas = ecore_evas_new(engine_name, viewport_x, viewport_y, viewport_w, viewport_h, extra_options);
>+}
>+
>+EFLTestEcoreEvas::~EFLTestEcoreEvas()
>+{
>+    if (m_ecoreEvas)
>+        ecore_evas_free(m_ecoreEvas);
>+}
>+
>+Evas* EFLTestEcoreEvas::getEvas()
>+{
>+    if (m_ecoreEvas)
>+        return ecore_evas_get(m_ecoreEvas);
>+    return 0;
>+}
>+
>+void EFLTestEcoreEvas::show()
>+{
>+    if (m_ecoreEvas)
>+        ecore_evas_show(m_ecoreEvas);
>+}
>+
>+EFLTestView::EFLTestView(Evas* evas)
>+    : m_webView(0)
>+    , m_evas(evas)
>+    , m_url(0)
>+    , m_defaultViewType(TiledView)
>+    , m_width(EFLUnitTests::Config::defaultViewWidth)
>+    , m_height(EFLUnitTests::Config::defaultViewHeight)
>+{
>+    m_url = createTestUrl(EFLUnitTests::Config::defaultTestPage);
>+}
>+
>+EFLTestView::EFLTestView(Evas* evas, const char* url)
>+    : m_webView(0)
>+    , m_evas(evas)
>+    , m_url(0)
>+    , m_defaultViewType(TiledView)
>+    , m_width(EFLUnitTests::Config::defaultViewWidth)
>+    , m_height(EFLUnitTests::Config::defaultViewHeight)
>+{
>+    m_url = createTestUrl(url);
>+}
>+
>+EFLTestView::EFLTestView(Evas* evas, EwkViewType type, const char* url)
>+    : m_webView(0)
>+    , m_evas(evas)
>+    , m_url(0)
>+    , m_defaultViewType(type)
>+    , m_width(EFLUnitTests::Config::defaultViewWidth)
>+    , m_height(EFLUnitTests::Config::defaultViewHeight)
>+{
>+    m_url = createTestUrl(url);
>+}
>+
>+EFLTestView::EFLTestView(Evas* evas, EwkViewType type, const char* url, int width, int height)
>+    : m_webView(0)
>+    , m_evas(evas)
>+    , m_url(0)
>+    , m_defaultViewType(type)
>+    , m_width(width)
>+    , m_height(height)
>+{
>+    m_url = createTestUrl(url);
>+}
>+
>+EFLTestView::~EFLTestView()
>+{
>+    if (m_webView)
>+        evas_object_del(m_webView);
>+    free(m_url);
>+}
>+
>+char* EFLTestView::createTestUrl(const char* url)
>+{
>+    if (url)
>+        return strdup(url);
>+    return 0;
>+}
>+
>+bool EFLTestView::init()
>+{
>+    if (!m_evas || !m_url)
>+        return false;
>+
>+    switch (m_defaultViewType) {
>+    case SingleView:
>+        m_webView = ewk_view_single_add(m_evas);
>+        break;
>+
>+    case TiledView:
>+        m_webView = ewk_view_tiled_add(m_evas);
>+        break;
>+    }
>+
>+    if (!m_webView)
>+        return false;
>+
>+    ewk_view_theme_set(m_webView, EFLUnitTests::Config::defaultThemePath);
>+    ewk_view_uri_set(m_webView, m_url);
>+}
>+
>+void EFLTestView::show()
>+{
>+    if (!m_webView)
>+        return;
>+    evas_object_resize(m_webView, m_width, m_height);
>+    evas_object_show(m_webView);
>+    evas_object_focus_set(m_webView, EINA_TRUE);
>+}
>+
>+Evas_Object* EFLTestView::getMainFrame()
>+{
>+    if (m_webView)
>+        return ewk_view_frame_main_get(m_webView);
>+    return 0;
>+}
>+
>+Evas* EFLTestView::getEvas()
>+{
>+    if (m_webView)
>+        return evas_object_evas_get(m_webView);
>+    return 0;
>+}
>+
>+void EFLTestView::bindEvents(void (*callback)(void*, Evas_Object*, void*), const char* eventName, void* ptr)
>+{
>+    if (!m_webView)
>+        return;
>+
>+    evas_object_smart_callback_del(m_webView, eventName, callback);
>+    evas_object_smart_callback_add(m_webView, eventName, callback, ptr);
>+}
>+
>+}
>diff --git a/Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestView.h b/Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestView.h
>new file mode 100644
>index 0000000..9cb2469
>--- /dev/null
>+++ b/Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestView.h
>@@ -0,0 +1,77 @@
>+/*
>+    Copyright (C) 2011 Samsung Electronics
>+
>+    This library is free software; you can redistribute it and/or
>+    modify it under the terms of the GNU Lesser General Public
>+    License as published by the Free Software Foundation; either
>+    version 2.1 of the License, or (at your option) any later version.
>+
>+    This library is distributed in the hope that it will be useful,
>+    but WITHOUT ANY WARRANTY; without even the implied warranty of
>+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>+    Lesser General Public License for more details.
>+
>+    You should have received a copy of the GNU Lesser General Public License
>+    along with this library; if not, write to the Free Software Foundation,
>+    Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>+*/
>+
>+#ifndef EFLTestView_h
>+#define EFLTestView_h
>+
>+#include <Ecore_Evas.h>
>+#include <Evas.h>
>+
>+namespace EFLUnitTests {
>+
>+class EFLTestEcoreEvas {
>+public:
>+    EFLTestEcoreEvas();
>+    EFLTestEcoreEvas(const char* engine_name, int viewport_x, int viewport_y, int viewport_w, int viewport_h, const char* extra_options);
>+    ~EFLTestEcoreEvas();
>+
>+    Evas* getEvas();
>+    void show();
>+
>+private:
>+    Ecore_Evas* m_ecoreEvas;
>+};
>+
>+class EFLTestView {
>+public:
>+    enum EwkViewType {
>+        SingleView = 0,
>+        TiledView,
>+    };
>+
>+    explicit EFLTestView(Evas*);
>+    EFLTestView(Evas*, const char* url);
>+    EFLTestView(Evas*, EwkViewType, const char* url);
>+    EFLTestView(Evas*, EwkViewType, const char* url, int width, int height);
>+
>+    virtual ~EFLTestView();
>+
>+    Evas_Object* getWebView() { return m_webView; }
>+    Evas_Object* getMainFrame();
>+    Evas* getEvas();
>+    void show();
>+
>+    bool init();
>+    void bindEvents(void (*callback)(void*, Evas_Object*, void*), const char* eventName, void* ptr);
>+protected:
>+    char* createTestUrl(const char* url);
>+private:
>+    EFLTestView(const EFLTestView&);
>+    EFLTestView operator=(const EFLTestView&);
>+
>+    Evas* m_evas;
>+    Evas_Object* m_webView;
>+
>+    char* m_url;
>+    int m_width, m_height;
>+    EwkViewType m_defaultViewType;
>+};
>+
>+}
>+
>+#endif
Comment 10 Krzysztof Czech 2011-10-05 07:12:20 PDT
Created attachment 109785 [details]
Patch
Comment 11 WebKit Review Bot 2011-10-19 00:26:03 PDT
Attachment 109785 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/efl/ChangeLog', u'Source/Web..." exit_code: 1

Source/WebKit/efl/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 1 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 12 Krzysztof Czech 2011-10-19 01:23:43 PDT
Comment on attachment 109785 [details]
Patch

>diff --git a/Source/WebKit/efl/ChangeLog b/Source/WebKit/efl/ChangeLog
>index 69c60a2..c6336c7 100755
>--- a/Source/WebKit/efl/ChangeLog
>+++ b/Source/WebKit/efl/ChangeLog
>@@ -1,3 +1,47 @@
>+2011-09-21  Krzysztof Czech  <k.czech@samsung.com>
>+
>+        Implementation of Unit Tests framework for the WebKit-Efl port.
>+        https://bugs.webkit.org/show_bug.cgi?id=68509
>+
>+        Unit Tests for the WebKit-Efl port are based on the gtest library.
>+        There's also been added a simple framework for running tests that require instance of a webview
>+        and example of unit tests.
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * tests/default_test_page.html: Added.
>+        * tests/src/EFLTestsRun.cpp: Added.
>+        (main):
>+        * tests/src/EwkEditableTests.cpp: Added.
>+        (editableSetTestCallback):
>+        (TEST):
>+        * tests/src/EwkUriTests.cpp: Added.
>+        (ewkUriSetTestCallback):
>+        (TEST):
>+        * tests/src/UnitTestLauncher/EFLTestConfig.h: Added.
>+        * tests/src/UnitTestLauncher/EFLTestLauncher.cpp: Added.
>+        (EFLUnitTests::EFLTestLauncher::init):
>+        (EFLUnitTests::EFLTestLauncher::startTest):
>+        (EFLUnitTests::EFLTestLauncher::endTest):
>+        (EFLUnitTests::EFLTestLauncher::createTest):
>+        (EFLUnitTests::EFLTestLauncher::runTest):
>+        * tests/src/UnitTestLauncher/EFLTestLauncher.h: Added.
>+        * tests/src/UnitTestLauncher/EFLTestView.cpp: Added.
>+        (EFLUnitTests::EFLTestEcoreEvas::EFLTestEcoreEvas):
>+        (EFLUnitTests::EFLTestEcoreEvas::~EFLTestEcoreEvas):
>+        (EFLUnitTests::EFLTestEcoreEvas::getEvas):
>+        (EFLUnitTests::EFLTestEcoreEvas::show):
>+        (EFLUnitTests::EFLTestView::EFLTestView):
>+        (EFLUnitTests::EFLTestView::~EFLTestView):
>+        (EFLUnitTests::EFLTestView::createTestUrl):
>+        (EFLUnitTests::EFLTestView::init):
>+        (EFLUnitTests::EFLTestView::show):
>+        (EFLUnitTests::EFLTestView::getMainFrame):
>+        (EFLUnitTests::EFLTestView::getEvas):
>+        (EFLUnitTests::EFLTestView::bindEvents):
>+        * tests/src/UnitTestLauncher/EFLTestView.h: Added.
>+        (EFLUnitTests::EFLTestView::getWebView):
>+
> 2011-09-17  Mihai Parparita  <mihaip@chromium.org>
> 
>         FrameLoaderClient BackForwardList-related methods are unsued
>diff --git a/Source/WebKit/efl/tests/default_test_page.html b/Source/WebKit/efl/tests/default_test_page.html
>new file mode 100644
>index 0000000..edd81e7
>--- /dev/null
>+++ b/Source/WebKit/efl/tests/default_test_page.html
>@@ -0,0 +1,6 @@
>+<HTML>
>+<BODY>
>+<H2 align="center">EFL Unit Tests</H2>
>+<H2 align="center">Default Testing Web Page</H2>
>+</BODY>
>+</HTML>
>diff --git a/Source/WebKit/efl/tests/src/EFLTestsRun.cpp b/Source/WebKit/efl/tests/src/EFLTestsRun.cpp
>new file mode 100644
>index 0000000..a22a61c
>--- /dev/null
>+++ b/Source/WebKit/efl/tests/src/EFLTestsRun.cpp
>@@ -0,0 +1,30 @@
>+/*
>+    Copyright (C) 2011 Samsung Electronics
>+
>+    This library is free software; you can redistribute it and/or
>+    modify it under the terms of the GNU Lesser General Public
>+    License as published by the Free Software Foundation; either
>+    version 2.1 of the License, or (at your option) any later version.
>+
>+    This library is distributed in the hope that it will be useful,
>+    but WITHOUT ANY WARRANTY; without even the implied warranty of
>+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>+    Lesser General Public License for more details.
>+
>+    You should have received a copy of the GNU Lesser General Public License
>+    along with this library; if not, write to the Free Software Foundation,
>+    Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>+*/
>+
>+#include <EFLTestLauncher.h>
>+#include <gtest/gtest.h>
>+#include <stdio.h>
>+
>+using namespace EFLUnitTests;
>+
>+int main(int argc, char** argv)
>+{
>+    atexit(EFLTestLauncher::shutdownAll);
>+    ::testing::InitGoogleTest(&argc, argv);
>+    return RUN_ALL_TESTS();
>+}
>diff --git a/Source/WebKit/efl/tests/src/EwkEditableTests.cpp b/Source/WebKit/efl/tests/src/EwkEditableTests.cpp
>new file mode 100644
>index 0000000..e866026
>--- /dev/null
>+++ b/Source/WebKit/efl/tests/src/EwkEditableTests.cpp
>@@ -0,0 +1,35 @@
>+/*
>+    Copyright (C) 2011 Samsung Electronics
>+
>+    This library is free software; you can redistribute it and/or
>+    modify it under the terms of the GNU Lesser General Public
>+    License as published by the Free Software Foundation; either
>+    version 2.1 of the License, or (at your option) any later version.
>+
>+    This library is distributed in the hope that it will be useful,
>+    but WITHOUT ANY WARRANTY; without even the implied warranty of
>+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>+    Lesser General Public License for more details.
>+
>+    You should have received a copy of the GNU Lesser General Public License
>+    along with this library; if not, write to the Free Software Foundation,
>+    Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>+*/
>+
>+#include "EFLTestLauncher.h"
>+#include "EWebKit.h"
>+#include <gtest/gtest.h>
>+
>+using namespace EFLUnitTests;
>+
>+void editableSetTestCallback(void* eventInfo, Evas_Object* o, void* data)
>+{
>+    EXPECT_EQ(EINA_TRUE, ewk_view_editable_set(o, EINA_FALSE));
>+    EXPECT_EQ(EINA_FALSE, ewk_view_editable_get(o));
>+    END_TEST();
>+}
>+
>+TEST(EwkEditableTests, EditableSetTest)
>+{
>+    RUN_TEST(editableSetTestCallback, "load,finished", 0);
>+}
>diff --git a/Source/WebKit/efl/tests/src/EwkUriTests.cpp b/Source/WebKit/efl/tests/src/EwkUriTests.cpp
>new file mode 100644
>index 0000000..eccd060
>--- /dev/null
>+++ b/Source/WebKit/efl/tests/src/EwkUriTests.cpp
>@@ -0,0 +1,34 @@
>+/*
>+    Copyright (C) 2011 Samsung Electronics
>+
>+    This library is free software; you can redistribute it and/or
>+    modify it under the terms of the GNU Lesser General Public
>+    License as published by the Free Software Foundation; either
>+    version 2.1 of the License, or (at your option) any later version.
>+
>+    This library is distributed in the hope that it will be useful,
>+    but WITHOUT ANY WARRANTY; without even the implied warranty of
>+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>+    Lesser General Public License for more details.
>+
>+    You should have received a copy of the GNU Lesser General Public License
>+    along with this library; if not, write to the Free Software Foundation,
>+    Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>+*/
>+
>+#include "EFLTestLauncher.h"
>+#include "EWebKit.h"
>+#include <gtest/gtest.h>
>+
>+using namespace EFLUnitTests;
>+
>+void ewkUriSetTestCallback(void* eventInfo, Evas_Object* o, void* data)
>+{
>+    EXPECT_STREQ("http://www.webkit.org/", ewk_view_uri_get(o));
>+    END_TEST();
>+}
>+
>+TEST(EwkUriTests, EwkUriSetTest)
>+{
>+    RUN_TEST("http://www.webkit.org", ewkUriSetTestCallback, "load,finished", 0);
>+}
>diff --git a/Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestConfig.h b/Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestConfig.h
>new file mode 100644
>index 0000000..ddcfcb6
>--- /dev/null
>+++ b/Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestConfig.h
>@@ -0,0 +1,31 @@
>+/*
>+    Copyright (C) 2011 Samsung Electronics
>+
>+    This library is free software; you can redistribute it and/or
>+    modify it under the terms of the GNU Lesser General Public
>+    License as published by the Free Software Foundation; either
>+    version 2.1 of the License, or (at your option) any later version.
>+
>+    This library is distributed in the hope that it will be useful,
>+    but WITHOUT ANY WARRANTY; without even the implied warranty of
>+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>+    Lesser General Public License for more details.
>+
>+    You should have received a copy of the GNU Lesser General Public License
>+    along with this library; if not, write to the Free Software Foundation,
>+    Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>+*/
>+
>+#ifndef EFLTestConfig_h
>+#define EFLTestConfig_h
>+
>+namespace EFLUnitTests {
>+namespace Config {
>+static const int defaultViewWidth = 600;
>+static const int defaultViewHeight = 800;
>+static const char* defaultThemePath = DEFAULT_THEME_PATH"/default.edj";
>+static const char* defaultTestPage = "file://"DEFAULT_TEST_PAGE_DIR"/default_test_page.html";
>+}
>+}
>+
>+#endif
>diff --git a/Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestLauncher.cpp b/Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestLauncher.cpp
>new file mode 100644
>index 0000000..0b46e18
>--- /dev/null
>+++ b/Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestLauncher.cpp
>@@ -0,0 +1,102 @@
>+/*
>+    Copyright (C) 2011 Samsung Electronics
>+
>+    This library is free software; you can redistribute it and/or
>+    modify it under the terms of the GNU Lesser General Public
>+    License as published by the Free Software Foundation; either
>+    version 2.1 of the License, or (at your option) any later version.
>+
>+    This library is distributed in the hope that it will be useful,
>+    but WITHOUT ANY WARRANTY; without even the implied warranty of
>+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>+    Lesser General Public License for more details.
>+
>+    You should have received a copy of the GNU Lesser General Public License
>+    along with this library; if not, write to the Free Software Foundation,
>+    Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>+*/
>+
>+#include "config.h"
>+#include "EFLTestLauncher.h"
>+
>+#include "EFLTestConfig.h"
>+#include "EFLTestView.h"
>+#include "EWebKit.h"
>+
>+#include <Ecore.h>
>+#include <Edje.h>
>+#include <Eina.h>
>+
>+namespace EFLUnitTests {
>+
>+bool EFLTestLauncher::init()
>+{
>+    if (!ecore_evas_init())
>+        return false;
>+
>+    if (!edje_init()) {
>+        ecore_evas_shutdown();
>+        return false;
>+    }
>+    ewk_init();
>+    return true;
>+}
>+
>+void EFLTestLauncher::shutdown()
>+{
>+    ecore_evas_shutdown();
>+    edje_shutdown();
>+    ewk_shutdown();
>+}
>+
>+void EFLTestLauncher::shutdownAll()
>+{
>+    int count = 0;
>+
>+    while ((count = ecore_evas_shutdown()) > 0) { }
>+    while ((count = edje_shutdown()) > 0) { }
>+    while ((count = ewk_shutdown()) > 0) { }
>+}
>+
>+void EFLTestLauncher::startTest()
>+{
>+    ecore_main_loop_begin();
>+}
>+
>+void EFLTestLauncher::endTest()
>+{
>+    ecore_main_loop_quit();
>+}
>+
>+bool EFLTestLauncher::createTest(const char* url, void (*event_callback)(void*, Evas_Object*, void*), const char* event_name, void* event_data)
>+{
>+    EFL_INIT_RET();
>+
>+    EFLTestEcoreEvas evas;
>+    if (!evas.getEvas())
>+        return false;
>+    evas.show();
>+
>+    EFLTestView view(evas.getEvas(), url);
>+    if (!view.init())
>+        return false;
>+
>+    view.bindEvents(event_callback, event_name, event_data);
>+    view.show();
>+
>+    START_TEST();
>+
>+    return true;
>+}
>+
>+bool EFLTestLauncher::runTest(void (*event_callback)(void*, Evas_Object*, void*), const char* event_name, void* event_data)
>+{
>+    return createTest(EFLUnitTests::Config::defaultTestPage, event_callback, event_name, event_data);
>+}
>+
>+bool EFLTestLauncher::runTest(const char* url, void (*event_callback)(void*, Evas_Object*, void*), const char* event_name, void* event_data)
>+{
>+    return createTest(url, event_callback, event_name, event_data);
>+}
>+
>+}
>diff --git a/Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestLauncher.h b/Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestLauncher.h
>new file mode 100644
>index 0000000..746f95d
>--- /dev/null
>+++ b/Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestLauncher.h
>@@ -0,0 +1,88 @@
>+/*
>+    Copyright (C) 2011 Samsung Electronics
>+
>+    This library is free software; you can redistribute it and/or
>+    modify it under the terms of the GNU Lesser General Public
>+    License as published by the Free Software Foundation; either
>+    version 2.1 of the License, or (at your option) any later version.
>+
>+    This library is distributed in the hope that it will be useful,
>+    but WITHOUT ANY WARRANTY; without even the implied warranty of
>+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>+    Lesser General Public License for more details.
>+
>+    You should have received a copy of the GNU Lesser General Public License
>+    along with this library; if not, write to the Free Software Foundation,
>+    Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>+*/
>+
>+#ifndef EFLTestLauncher_h
>+#define EFLTestLauncher_h
>+
>+#include <Evas.h>
>+
>+#ifdef GTEST_TEST_FRAMEWORK
>+#include <gtest/gtest.h>
>+#endif
>+
>+#ifdef GTEST_TEST_FRAMEWORK
>+    #define RUN_TEST(args...)   \
>+        do {    \
>+            ASSERT_EQ(true, EFLTestLauncher::runTest(args));    \
>+        } while (0)
>+#else
>+    #define RUN_TEST(args...)   \
>+    do {    \
>+        EFLTestLauncher::runTest(args); \
>+    } while (0)
>+#endif
>+
>+#define START_TEST()    \
>+    do {    \
>+        EFLTestLauncher::startTest();   \
>+    } while (0)
>+
>+#define END_TEST()    \
>+    do {    \
>+        EFLTestLauncher::endTest(); \
>+    } while (0)
>+
>+#define EFL_INIT_RET()  \
>+    do {    \
>+        if (!EFLTestLauncher::init())   \
>+            return false;    \
>+    } while (0)
>+
>+#define EFL_INIT()  \
>+    do {    \
>+        EFLTestLauncher::init();    \
>+    } while (0)
>+
>+#define EFL_SHUTDOWN()  \
>+    do {    \
>+        EFLTestLauncher::shutdown();    \
>+    } while (0)
>+
>+#define EFL_SHUTDOWN_ALL()  \
>+    do {    \
>+        EFLTestLauncher::shutdownAll(); \
>+    } while (0)
>+
>+namespace EFLUnitTests {
>+
>+class EFLTestLauncher {
>+    static bool createTest(const char* url, void (*event_callback)(void*, Evas_Object*, void*), const char* event_name, void* event_data);
>+public:
>+    static bool init();
>+    static void shutdown();
>+    static void shutdownAll();
>+    static void startTest();
>+    static void endTest();
>+
>+    static bool runTest(const char* url, void (*event_callback)(void*, Evas_Object*, void*), const char* event_name, void* event_data);
>+    static bool runTest(void (*event_callback)(void*, Evas_Object*, void*), const char* event_name, void* event_data);
>+};
>+
>+}
>+
>+#endif
>diff --git a/Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestView.cpp b/Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestView.cpp
>new file mode 100644
>index 0000000..8053b3a
>--- /dev/null
>+++ b/Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestView.cpp
>@@ -0,0 +1,177 @@
>+/*
>+    Copyright (C) 2011 Samsung Electronics
>+
>+    This library is free software; you can redistribute it and/or
>+    modify it under the terms of the GNU Lesser General Public
>+    License as published by the Free Software Foundation; either
>+    version 2.1 of the License, or (at your option) any later version.
>+
>+    This library is distributed in the hope that it will be useful,
>+    but WITHOUT ANY WARRANTY; without even the implied warranty of
>+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>+    Lesser General Public License for more details.
>+
>+    You should have received a copy of the GNU Lesser General Public License
>+    along with this library; if not, write to the Free Software Foundation,
>+    Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>+*/
>+
>+#include "config.h"
>+#include "EFLTestView.h"
>+
>+#include "EFLTestConfig.h"
>+#include "EWebKit.h"
>+
>+#include <Ecore.h>
>+#include <Ecore_File.h>
>+#include <Edje.h>
>+#include <Eina.h>
>+
>+#include <string.h>
>+
>+namespace EFLUnitTests {
>+
>+EFLTestEcoreEvas::EFLTestEcoreEvas()
>+    : m_ecoreEvas(0)
>+{
>+    m_ecoreEvas = ecore_evas_new(0, 0, 0, EFLUnitTests::Config::defaultViewWidth, EFLUnitTests::Config::defaultViewHeight, 0);
>+}
>+
>+EFLTestEcoreEvas::EFLTestEcoreEvas(const char* engine_name, int viewport_x, int viewport_y, int viewport_w, int viewport_h, const char* extra_options)
>+    : m_ecoreEvas(0)
>+{
>+    m_ecoreEvas = ecore_evas_new(engine_name, viewport_x, viewport_y, viewport_w, viewport_h, extra_options);
>+}
>+
>+EFLTestEcoreEvas::~EFLTestEcoreEvas()
>+{
>+    if (m_ecoreEvas)
>+        ecore_evas_free(m_ecoreEvas);
>+}
>+
>+Evas* EFLTestEcoreEvas::getEvas()
>+{
>+    if (m_ecoreEvas)
>+        return ecore_evas_get(m_ecoreEvas);
>+    return 0;
>+}
>+
>+void EFLTestEcoreEvas::show()
>+{
>+    if (m_ecoreEvas)
>+        ecore_evas_show(m_ecoreEvas);
>+}
>+
>+EFLTestView::EFLTestView(Evas* evas)
>+    : m_webView(0)
>+    , m_evas(evas)
>+    , m_url(0)
>+    , m_defaultViewType(TiledView)
>+    , m_width(EFLUnitTests::Config::defaultViewWidth)
>+    , m_height(EFLUnitTests::Config::defaultViewHeight)
>+{
>+    m_url = createTestUrl(EFLUnitTests::Config::defaultTestPage);
>+}
>+
>+EFLTestView::EFLTestView(Evas* evas, const char* url)
>+    : m_webView(0)
>+    , m_evas(evas)
>+    , m_url(0)
>+    , m_defaultViewType(TiledView)
>+    , m_width(EFLUnitTests::Config::defaultViewWidth)
>+    , m_height(EFLUnitTests::Config::defaultViewHeight)
>+{
>+    m_url = createTestUrl(url);
>+}
>+
>+EFLTestView::EFLTestView(Evas* evas, EwkViewType type, const char* url)
>+    : m_webView(0)
>+    , m_evas(evas)
>+    , m_url(0)
>+    , m_defaultViewType(type)
>+    , m_width(EFLUnitTests::Config::defaultViewWidth)
>+    , m_height(EFLUnitTests::Config::defaultViewHeight)
>+{
>+    m_url = createTestUrl(url);
>+}
>+
>+EFLTestView::EFLTestView(Evas* evas, EwkViewType type, const char* url, int width, int height)
>+    : m_webView(0)
>+    , m_evas(evas)
>+    , m_url(0)
>+    , m_defaultViewType(type)
>+    , m_width(width)
>+    , m_height(height)
>+{
>+    m_url = createTestUrl(url);
>+}
>+
>+EFLTestView::~EFLTestView()
>+{
>+    if (m_webView)
>+        evas_object_del(m_webView);
>+    free(m_url);
>+}
>+
>+char* EFLTestView::createTestUrl(const char* url)
>+{
>+    if (url)
>+        return strdup(url);
>+    return 0;
>+}
>+
>+bool EFLTestView::init()
>+{
>+    if (!m_evas || !m_url)
>+        return false;
>+
>+    switch (m_defaultViewType) {
>+    case SingleView:
>+        m_webView = ewk_view_single_add(m_evas);
>+        break;
>+
>+    case TiledView:
>+        m_webView = ewk_view_tiled_add(m_evas);
>+        break;
>+    }
>+
>+    if (!m_webView)
>+        return false;
>+
>+    ewk_view_theme_set(m_webView, EFLUnitTests::Config::defaultThemePath);
>+    ewk_view_uri_set(m_webView, m_url);
>+}
>+
>+void EFLTestView::show()
>+{
>+    if (!m_webView)
>+        return;
>+    evas_object_resize(m_webView, m_width, m_height);
>+    evas_object_show(m_webView);
>+    evas_object_focus_set(m_webView, EINA_TRUE);
>+}
>+
>+Evas_Object* EFLTestView::getMainFrame()
>+{
>+    if (m_webView)
>+        return ewk_view_frame_main_get(m_webView);
>+    return 0;
>+}
>+
>+Evas* EFLTestView::getEvas()
>+{
>+    if (m_webView)
>+        return evas_object_evas_get(m_webView);
>+    return 0;
>+}
>+
>+void EFLTestView::bindEvents(void (*callback)(void*, Evas_Object*, void*), const char* eventName, void* ptr)
>+{
>+    if (!m_webView)
>+        return;
>+
>+    evas_object_smart_callback_del(m_webView, eventName, callback);
>+    evas_object_smart_callback_add(m_webView, eventName, callback, ptr);
>+}
>+
>+}
>diff --git a/Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestView.h b/Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestView.h
>new file mode 100644
>index 0000000..9cb2469
>--- /dev/null
>+++ b/Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestView.h
>@@ -0,0 +1,77 @@
>+/*
>+    Copyright (C) 2011 Samsung Electronics
>+
>+    This library is free software; you can redistribute it and/or
>+    modify it under the terms of the GNU Lesser General Public
>+    License as published by the Free Software Foundation; either
>+    version 2.1 of the License, or (at your option) any later version.
>+
>+    This library is distributed in the hope that it will be useful,
>+    but WITHOUT ANY WARRANTY; without even the implied warranty of
>+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>+    Lesser General Public License for more details.
>+
>+    You should have received a copy of the GNU Lesser General Public License
>+    along with this library; if not, write to the Free Software Foundation,
>+    Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>+*/
>+
>+#ifndef EFLTestView_h
>+#define EFLTestView_h
>+
>+#include <Ecore_Evas.h>
>+#include <Evas.h>
>+
>+namespace EFLUnitTests {
>+
>+class EFLTestEcoreEvas {
>+public:
>+    EFLTestEcoreEvas();
>+    EFLTestEcoreEvas(const char* engine_name, int viewport_x, int viewport_y, int viewport_w, int viewport_h, const char* extra_options);
>+    ~EFLTestEcoreEvas();
>+
>+    Evas* getEvas();
>+    void show();
>+
>+private:
>+    Ecore_Evas* m_ecoreEvas;
>+};
>+
>+class EFLTestView {
>+public:
>+    enum EwkViewType {
>+        SingleView = 0,
>+        TiledView,
>+    };
>+
>+    explicit EFLTestView(Evas*);
>+    EFLTestView(Evas*, const char* url);
>+    EFLTestView(Evas*, EwkViewType, const char* url);
>+    EFLTestView(Evas*, EwkViewType, const char* url, int width, int height);
>+
>+    virtual ~EFLTestView();
>+
>+    Evas_Object* getWebView() { return m_webView; }
>+    Evas_Object* getMainFrame();
>+    Evas* getEvas();
>+    void show();
>+
>+    bool init();
>+    void bindEvents(void (*callback)(void*, Evas_Object*, void*), const char* eventName, void* ptr);
>+protected:
>+    char* createTestUrl(const char* url);
>+private:
>+    EFLTestView(const EFLTestView&);
>+    EFLTestView operator=(const EFLTestView&);
>+
>+    Evas* m_evas;
>+    Evas_Object* m_webView;
>+
>+    char* m_url;
>+    int m_width, m_height;
>+    EwkViewType m_defaultViewType;
>+};
>+
>+}
>+
>+#endif
Comment 13 Krzysztof Czech 2011-10-19 01:28:16 PDT
Created attachment 111574 [details]
Patch
Comment 14 Raphael Kubo da Costa (:rakuco) 2012-01-01 16:25:06 PST
(In reply to comment #6)
> (In reply to comment #5)
> > I would also prefer to use as many "native" C++ and WebKit types as possible, so you have bool instead of Eina_Bool where possible, and you use Strings instead of char*'s whose memory you need to manage manually. Speaking of memory management, you could also use OwnPtrs for the Evas_Objects and other pointers so you also don't need to manage their memory by hand.
> 
> I think it might be a problem using WebKit's OwnPtrs in unit tests context.
> Tests are linked against WebKit library. Those internals like OwnPtr are not publicly exported.

From the latest patch in bug 68510, the tests actually link against WebCore, JSC and many other libraries, so there should be no problem with using OwnPtr's.

> Other way I am not sure if is a good idea to mix Evas_Object's with smart pointers. How do you think ?

What problems do you see? It is already done in other places in ewk and EFL's DumpRenderTree.
Comment 15 Krzysztof Czech 2012-01-02 05:50:12 PST
I see potential problems with mixing c++ and c routines regarding to memory management. I don't mind using OwnPtr to Evas_Object and other structures created by the "new" statement. I rather think about the situation when OwnPtr adopts a pointer crated by the malloc and then it calls a destructor which cleans the pointer by using "delete".
Comment 16 Krzysztof Czech 2012-01-02 06:18:52 PST
Regarding to my last post and potential problems, I meant about the situation like as follows:
OwnPtr<Ecore_Evas> ee = adoptPtr(ecore_evas_new(...));
Will OwnPtr call ecore_evas_free, I think it might be a problem.
What do you think about this?.
Comment 17 Raphael Kubo da Costa (:rakuco) 2012-01-02 09:30:34 PST
(In reply to comment #16)
> Regarding to my last post and potential problems, I meant about the situation like as follows:
> OwnPtr<Ecore_Evas> ee = adoptPtr(ecore_evas_new(...));
> Will OwnPtr call ecore_evas_free, I think it might be a problem.

Was your last sentence actually a question? If so, then yes, OwnPtr will destroy the object with ecore_evas_free (see Sources/JavaScriptCore/wtf/efl/OwnPtrEfl.cpp). Why is that a problem?
Comment 18 Krzysztof Czech 2012-01-13 08:28:51 PST
Created attachment 122431 [details]
Corrected patch with build scripts and implementation
Comment 19 Krzysztof Czech 2012-01-13 08:31:39 PST
I added new patch. It consists of build scripts and implementation
Comment 20 Gyuyoung Kim 2012-01-13 08:59:58 PST
Comment on attachment 122431 [details]
Corrected patch with build scripts and implementation

Attachment 122431 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/11236047
Comment 21 Krzysztof Czech 2012-01-16 06:05:16 PST
Created attachment 122623 [details]
Corrected patch with build scripts and implementation
Comment 22 Gyuyoung Kim 2012-01-16 06:17:55 PST
Comment on attachment 122623 [details]
Corrected patch with build scripts and implementation

Attachment 122623 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/11255162
Comment 23 Krzysztof Czech 2012-01-16 07:03:17 PST
Created attachment 122630 [details]
Corrected patch with build scripts and implementation
Comment 24 Krzysztof Czech 2012-01-17 00:25:48 PST
Created attachment 122725 [details]
Corrected patch with build scripts and implementation
Comment 25 Krzysztof Czech 2012-01-17 09:13:39 PST
Hello Raphael would you make a review of this patch
Comment 26 Krzysztof Czech 2012-05-14 06:41:55 PDT
Created attachment 141718 [details]
Corrected patch with build scripts and implementation
Comment 27 Krzysztof Czech 2012-05-18 00:49:56 PDT
Created attachment 142656 [details]
Corrected patch with build scripts and implementation
Comment 28 Thiago Marcos P. Santos 2012-05-24 07:52:18 PDT
Comment on attachment 142656 [details]
Corrected patch with build scripts and implementation

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

One general problem that I see here is that our tests should be written in C, not C++, since the main customer of ours API are C programs. Not sure if this is really a big deal or not, but writing tests in C makes us consumers of our own API and might influence some design decisions.

> Source/WebKit/PlatformEfl.cmake:297
> +    ${EFLDEPS_LIBRARIES}

I don't think we need to link with all these libraries directly since we are not using it.

> Source/WebKit/PlatformEfl.cmake:305
> +    "${JAVASCRIPTCORE_DIR}/wtf"

Should use "${WTF_DIR}" here instead.

> Source/WebKit/PlatformEfl.cmake:306
> +    "${JAVASCRIPTCORE_DIR}/ForwardingHeaders"

Ditto.

> Source/WebKit/PlatformEfl.cmake:315
> +    ${EFLDEPS_LDFLAGS}

IMO not all are needed here.

> Source/WebKit/PlatformEfl.cmake:329
> +SET(DEFAULT_TEST_PAGE_DIR ${CMAKE_SOURCE_DIR}/Source/WebKit/efl/tests)

I would put it on resources like GTK.

> Source/WebKit/PlatformEfl.cmake:333
> +

I don't know if efl_test_launcher is a good name for this. If I understood correctly, this is a convenience around gtest, that loads a page for you. Maybe the class should be named EFLTestBase (or EWK?) instead of EFLTestLauncher and the library something like efl_test_utils.

> Source/WebKit/PlatformEfl.cmake:336
> +    ${WEBKIT_DIR}/efl/tests/src/UnitTestLauncher/EFLTestView.cpp

I don't see a need for this ./src/ folder here.

> Source/WebKit/PlatformEfl.cmake:339
> +SET(EUnitTests_SOURCES

This is a test example right?

I like the approach of having a file named after the subject of the test. The idea here is to test ewk_*.h right? And also would be nice to have one executable per tested API. It will make it easier to debug and find memory leaks. CMake + CTest can take care of creating a global test runner.

What about test_ewk_settings.cpp, test_ewk_view.cpp, etc.?

> Source/WebKit/efl/tests/src/EFLTestsRun.cpp:17
> +*/

Nit, needs a blank line here.

> Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestLauncher.cpp:42
> +    return true;

What if ewk_init() fails?

return ewk_init(); maybe?

> Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestView.cpp:24
> +

Nit. I think these headers should be packed and alpha sorted.

> Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestView.cpp:66
> +{ }

{
}

> Source/WebKit/efl/tests/src/UnitTestLauncher/EFLTestView.cpp:74
> +{ }

Ditto, same for the others bellow.
Comment 29 Krzysztof Czech 2012-05-25 02:08:49 PDT
> One general problem that I see here is that our tests should be written in C, 
> not C++, since the main customer of ours API are C programs. Not sure if this
> is really a big deal or not, but writing tests in C makes us consumers of our
> own API and might influence some design decisions.

What do you mean that tests should be written in C?.
Generally if you look at the example tests sources (EwkUriTests.cpp), there are no any specific C++ features.
I guess, you probably thought about EFLTestView.h and "explicit" keyword.
I agree with you that our main targets are C programs, so in this case mixing C++ may not be a good idea.
It will not be a big effort to tailor it for our needs.
Comment 30 Krzysztof Czech 2012-06-06 06:15:57 PDT
>> Source/WebKit/PlatformEfl.cmake:329
>> +SET(DEFAULT_TEST_PAGE_DIR ${CMAKE_SOURCE_DIR}/Source/WebKit/efl/tests)

>I would put it on resources like GTK.

Do you mean resources as a separate folder under "tests" directory and change the path to DEFAULT_TEST_PAGE_DIR ?. What I see, GTK does it similar, but resources are under ./WebKit/gtk/ and access to this path is via some WebCore's API.

GTK's solution could be done as a next step, since it requires some discussion. I think having DEFAULT_TEST_PAGE_DIR under ./efl/tests directory looks good, since it's internal part of unit tests.

> What about test_ewk_settings.cpp, test_ewk_view.cpp, etc.?

I also thought about this, but on the other way, it might cause some mess. It wouldn't be readable. What do you think ?. One function can have couple of test cases and it's good to have them in one file for further debugging and understanding.
What do you think, if we have separate folders for each test's category under WebKit/efl/tests/. Example ./WebKit/efl/tests/ewk_view. All the test related to ewk_view API would be placed there.
Comment 31 Thiago Marcos P. Santos 2012-06-06 06:42:16 PDT
> > What about test_ewk_settings.cpp, test_ewk_view.cpp, etc.?
> 
> I also thought about this, but on the other way, it might cause some mess. It wouldn't be readable. What do you think ?. One function can have couple of test cases and it's good to have them in one file for further debugging and understanding.
> What do you think, if we have separate folders for each test's category under WebKit/efl/tests/. Example ./WebKit/efl/tests/ewk_view. All the test related to ewk_view API would be placed there.

I like the idea of using a prefix because it is easier for the test runner to list what has to be executed.

I'm fine on using: prefix_realm.cpp (test_settings.cpp, test_localization.cpp, test_webview.cpp, test_view_single.cpp, test_view_tiled.cpp, etc). But I don't think that a separated folder is necessary since it is likely that most of the folders will have just one file.
Comment 32 Krzysztof Czech 2012-06-06 06:59:48 PDT
Created attachment 146014 [details]
Corrected patch with build scripts and implementation
Comment 33 Krzysztof Czech 2012-06-06 07:53:21 PDT
> I like the idea of using a prefix because it is easier for the test runner to list what has to be executed.

> I'm fine on using: prefix_realm.cpp (test_settings.cpp, test_localization.cpp, test_webview.cpp, test_view_single.cpp, test_view_tiled.cpp, etc). 
> But I don't think that a separated folder is necessary since it is likely that most of the folders will have just one file.


Folders would have test cases. In this solution one test file would test one API's function. One test file might have several test cases.
Generally I also agree with having test_webview.cpp, etc. My only concerns was readability and tests's management.
Comment 34 Thiago Marcos P. Santos 2012-06-06 08:13:39 PDT
Comment on attachment 146014 [details]
Corrected patch with build scripts and implementation

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

> Source/WebKit/PlatformEfl.cmake:318
> +    "${WEBKIT_DIR}/efl/tests/src/UnitTestLauncher"

UnitTestLauncher does not exists

> Source/WebKit/PlatformEfl.cmake:320
> +    "${WTF_DIR}/ForwardingHeaders"

${WTF_DIR}/ForwardingHeaders does not exists

> Source/WebKit/PlatformEfl.cmake:370
> +    ADD_EXECUTABLE(bin/${testName} ${WEBKIT_EFL_TEST_DIR}/${testName}.cpp ${WEBKIT_EFL_TEST_DIR}/EwkTestsMain.cpp)

IMO is important to prefix the tests with something. We will get here:

$ ls WebKitBuild/Debug/bin/
DumpRenderTree
EWebLauncher
EwkViewUriGetTest
EwkViewEditableGetTest
ImageDiff
jsc


I personally would like to see something more intuitive like:

$ ls WebKitBuild/Debug/bin/
DumpRenderTree
EWebLauncher
ImageDiff
jsc
test_EwkViewUriGetTest
test_EwkViewEditableGetTest

> Source/WebKit/efl/tests/UnitTestUtils/EWKTestBase.h:51
> +#define EFL_SHUTDOWN()  \

EFL_SHUTDOWN* macros are not in use.

> Source/WebKit/efl/tests/default_test_page.html:1
> +<HTML>

I still don't like the idea of mixing test "resources" with source code. IMO it should be placed in:
Source/WebKit/efl/tests/resources/
Comment 35 Gyuyoung Kim 2012-06-06 08:52:11 PDT
Comment on attachment 146014 [details]
Corrected patch with build scripts and implementation

Attachment 146014 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/12906634
Comment 36 Krzysztof Czech 2012-06-13 03:29:38 PDT
Created attachment 147276 [details]
Corrected patch with build scripts and implementation
Comment 37 Gyuyoung Kim 2012-06-13 10:06:58 PDT
Comment on attachment 147276 [details]
Corrected patch with build scripts and implementation

Attachment 147276 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/12953234
Comment 38 Krzysztof Czech 2012-06-14 03:51:02 PDT
Created attachment 147550 [details]
Corrected patch with build scripts and implementation - correcting build problems
Comment 39 Krzysztof Czech 2012-06-19 08:24:30 PDT
Created attachment 148338 [details]
Corrected patch with build scripts and implementation - updating
Comment 40 Thiago Marcos P. Santos 2012-06-20 03:28:13 PDT
Patch looks great to me. Just tried the last version on my machine. Thank you for this important QA tool that will help a lot to stabilize EFL public API's.
Comment 41 Grzegorz Czajkowski 2012-06-20 05:28:09 PDT
Comment on attachment 148338 [details]
Corrected patch with build scripts and implementation - updating

This patch is important for WebKit-EFL but before landing it requires some changes especially in styles. Please use CamelCase coding styles for variables.

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

> Source/WebKit/efl/ChangeLog:6
> +        Patch by Krzysztof Czech <k.czech@samsung.com>, Tomasz Morawski <t.morawski@samsung.com> on 2012-01-16

If those changes are made by two developers please mention about it in first line of ChangeLog.

> Source/WebKit/efl/tests/UnitTestUtils/EWKTestConfig.h:26
> +static const char* defaultThemePath = DEFAULT_THEME_PATH"/default.edj";

For initialization like this it's recommend to use tables instead of pointers.
static const char* defaultThemePath = .... -> static const char defaultThemePath[] = ...

> Source/WebKit/efl/tests/UnitTestUtils/EWKTestConfig.h:27
> +static const char* defaultTestPage = "file://"DEFAULT_TEST_PAGE_DIR"/default_test_page.html";

Ditto.

> Source/WebKit/efl/tests/UnitTestUtils/EWKTestView.cpp:29
> +EWKTestEcoreEvas::EWKTestEcoreEvas()

I think that this constructor isn't needed if we can call EWKTestEcoreEvas::EWKTestEcoreEvas(1) with the same result. It's better to add a default value of constructor's parameter.

> Source/WebKit/efl/tests/UnitTestUtils/EWKTestView.cpp:103
> +EWKTestView::~EWKTestView()

Can be removed.

> Source/WebKit/efl/tests/UnitTestUtils/EWKTestView.h:55
> +    virtual ~EWKTestView();

virtual is not needed in this case.

> Source/WebKit/efl/tests/UnitTestUtils/EWKTestView.h:57
> +    Evas_Object* getWebView() { return m_webView.get(); }

WebKit doesn't use get prefixes (only for setter set prefix is added).
Comment 42 Thiago Marcos P. Santos 2012-06-20 06:02:22 PDT
> > Source/WebKit/efl/tests/UnitTestUtils/EWKTestConfig.h:26
> > +static const char* defaultThemePath = DEFAULT_THEME_PATH"/default.edj";
> 
> For initialization like this it's recommend to use tables instead of pointers.
> static const char* defaultThemePath = .... -> static const char defaultThemePath[] = ...

Why?
Comment 43 Krzysztof Czech 2012-06-20 09:42:33 PDT
(In reply to comment #42)
> > > Source/WebKit/efl/tests/UnitTestUtils/EWKTestConfig.h:26
> > > +static const char* defaultThemePath = DEFAULT_THEME_PATH"/default.edj";
> > 
> > For initialization like this it's recommend to use tables instead of pointers.
> > static const char* defaultThemePath = .... -> static const char defaultThemePath[] = ...
> 
> Why?

I'm also wondering why it is recommended of using char arrays to char pointers ?.
Comment 44 Grzegorz Czajkowski 2012-06-21 00:52:44 PDT
(In reply to comment #43)
> (In reply to comment #42)
> > > > Source/WebKit/efl/tests/UnitTestUtils/EWKTestConfig.h:26
> > > > +static const char* defaultThemePath = DEFAULT_THEME_PATH"/default.edj";
> > > 
> > > For initialization like this it's recommend to use tables instead of pointers.
> > > static const char* defaultThemePath = .... -> static const char defaultThemePath[] = ...
> > 
> > Why?
> 
> I'm also wondering why it is recommended of using char arrays to char pointers ?.

I am not C pedantic but this issue has been discussed a lot. You're declaring the pointer and it's initialized to point to a string constant but the pointer may subsequently be modified to point elsewhere. It can't be done if you define the string as an array.
Comment 45 Krzysztof Czech 2012-06-21 02:23:07 PDT
Created attachment 148752 [details]
Corrected patch with build scripts and implementation
Comment 46 Krzysztof Czech 2012-06-21 02:50:44 PDT
Created attachment 148755 [details]
Corrected patch with build scripts and implementation
Comment 47 Thiago Marcos P. Santos 2012-06-21 04:27:34 PDT
(In reply to comment #46)
> Created an attachment (id=148755) [details]
> Corrected patch with build scripts and implementation

LGTM! Thanks.
Comment 48 Chang Shu 2012-06-26 11:10:57 PDT
Comment on attachment 148755 [details]
Corrected patch with build scripts and implementation

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

Very nice! I am looking forward to seeing the unit tests up and running.

> Source/WebKit/efl/ChangeLog:10
> +        of each test.

Could you please fix the line wrapping above?
Comment 49 Krzysztof Czech 2012-06-27 07:48:32 PDT
Created attachment 149747 [details]
Corrected patch with build scripts and implementation - updating
Comment 50 Gyuyoung Kim 2012-06-27 18:23:55 PDT
(In reply to comment #49)
> Created an attachment (id=149747) [details]
> Corrected patch with build scripts and implementation - updating

Chang Shu already set r+. So, you don't need to request review again. Please request cq? again after adding reviewer name to ChangeLog.
Comment 51 Chang Shu 2012-06-27 19:00:59 PDT
Comment on attachment 149747 [details]
Corrected patch with build scripts and implementation - updating

For your convenience. :)
Comment 52 WebKit Review Bot 2012-06-27 19:17:55 PDT
Comment on attachment 149747 [details]
Corrected patch with build scripts and implementation - updating

Clearing flags on attachment: 149747

Committed r121398: <http://trac.webkit.org/changeset/121398>
Comment 53 WebKit Review Bot 2012-06-27 19:18:04 PDT
All reviewed patches have been landed.  Closing bug.
Comment 54 WebKit Review Bot 2012-06-27 22:30:57 PDT
Re-opened since this is blocked by 90136
Comment 55 Ryuan Choi 2012-06-27 22:39:17 PDT
I am not sure why this got the green bot.

I got below error message.

/usr/bin/ld: ../../lib/libewkTestUtils.a(EWKTestBase.cpp.o): undefined reference to symbol 'ecore_main_loop_quit'
/usr/bin/ld: note: 'ecore_main_loop_quit' is defined in DSO /home/ryuan/workspace/webkit/efl-webkit/WebKitBuild/Dependencies/Root/lib/libecore.so so try adding it to the linker command line
/home/ryuan/workspace/webkit/efl-webkit/WebKitBuild/Dependencies/Root/lib/libecore.so: could not read symbols: Invalid operation
collect2: ld returned 1 exit status
make[2]: *** [bin/test_ewk_view] Error 1
make[1]: *** [Source/WebKit/CMakeFiles/bin/test_ewk_view.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....
[100%] Built target ewebkit2
make: *** [all] Error 2
Comment 56 Ryuan Choi 2012-06-27 22:56:53 PDT
Comment on attachment 149747 [details]
Corrected patch with build scripts and implementation - updating

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

Additionally, I found some nit to cleanup.

> Source/WebKit/PlatformEfl.cmake:340
> +        ${Gdk_LIBRARIES}

We don't use Gdk. so it should be removed.

> Source/WebKit/PlatformEfl.cmake:367
> +    ADD_EXECUTABLE(bin/${testName} ${WEBKIT_EFL_TEST_DIR}/${testName}.cpp ${WEBKIT_EFL_TEST_DIR}/test_runner.cpp)
> +    TARGET_LINK_LIBRARIES(bin/${testName} ${EWKUnitTests_LIBRARIES} ewkTestUtils gtest pthread)
> +    ADD_TARGET_PROPERTIES(bin/${testName} LINK_FLAGS "${EWKUnitTests_LINK_FLAGS}")
> +    SET_TARGET_PROPERTIES(bin/${testName} PROPERTIES FOLDER "WebKit")
> +    SET_TARGET_PROPERTIES(bin/${testName} PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}")

We removed 'bin/' because EXEC_INSTALL_DIR was given in OptionsCommon.cmake.
And please remove RUNTIME_OUTPUT_DIRECTORY also.

Please refer to http://trac.webkit.org/changeset/120024 .

> +    TARGET_LINK_LIBRARIES(bin/${testName} ${EWKUnitTests_LIBRARIES} ewkTestUtils gtest pthread)
Can you test whether pthread is really required ?
Comment 57 Krzysztof Czech 2012-06-28 08:22:58 PDT
(In reply to comment #56)
> (From update of attachment 149747 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=149747&action=review
> 
> Additionally, I found some nit to cleanup.
> 
> > Source/WebKit/PlatformEfl.cmake:340
> > +        ${Gdk_LIBRARIES}
> 
> We don't use Gdk. so it should be removed.
> 
> > Source/WebKit/PlatformEfl.cmake:367
> > +    ADD_EXECUTABLE(bin/${testName} ${WEBKIT_EFL_TEST_DIR}/${testName}.cpp ${WEBKIT_EFL_TEST_DIR}/test_runner.cpp)
> > +    TARGET_LINK_LIBRARIES(bin/${testName} ${EWKUnitTests_LIBRARIES} ewkTestUtils gtest pthread)
> > +    ADD_TARGET_PROPERTIES(bin/${testName} LINK_FLAGS "${EWKUnitTests_LINK_FLAGS}")
> > +    SET_TARGET_PROPERTIES(bin/${testName} PROPERTIES FOLDER "WebKit")
> > +    SET_TARGET_PROPERTIES(bin/${testName} PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}")
> 
> We removed 'bin/' because EXEC_INSTALL_DIR was given in OptionsCommon.cmake.
> And please remove RUNTIME_OUTPUT_DIRECTORY also.
> 
> Please refer to http://trac.webkit.org/changeset/120024 .
> 
> > +    TARGET_LINK_LIBRARIES(bin/${testName} ${EWKUnitTests_LIBRARIES} ewkTestUtils gtest pthread)
> Can you test whether pthread is really required ?
Yes pthread is required, gtest library depends on it.
Comment 58 Krzysztof Czech 2012-06-29 08:12:13 PDT
Created attachment 150180 [details]
Corrected patch with build scripts and implementation - updating
Comment 59 Ryuan Choi 2012-06-29 17:27:13 PDT
Comment on attachment 150180 [details]
Corrected patch with build scripts and implementation - updating

Thank you.
I double checked in my local.
Comment 60 WebKit Review Bot 2012-06-29 18:20:35 PDT
Comment on attachment 150180 [details]
Corrected patch with build scripts and implementation - updating

Clearing flags on attachment: 150180

Committed r121608: <http://trac.webkit.org/changeset/121608>
Comment 61 WebKit Review Bot 2012-06-29 18:20:45 PDT
All reviewed patches have been landed.  Closing bug.