|
Description
Krzysztof Czech
2011-09-21 01:29:06 PDT
Created attachment 108121 [details]
Patch
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. 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. I meant all patches by 68509, 68510, 68511 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. (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. > 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.
Created attachment 109766 [details]
Patch
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 Created attachment 109785 [details]
Patch
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 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 Created attachment 111574 [details]
Patch
(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. 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". 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?. (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? Created attachment 122431 [details]
Corrected patch with build scripts and implementation
I added new patch. It consists of build scripts and implementation 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 Created attachment 122623 [details]
Corrected patch with build scripts and implementation
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 Created attachment 122630 [details]
Corrected patch with build scripts and implementation
Created attachment 122725 [details]
Corrected patch with build scripts and implementation
Hello Raphael would you make a review of this patch Created attachment 141718 [details]
Corrected patch with build scripts and implementation
Created attachment 142656 [details]
Corrected patch with build scripts and implementation
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. > 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.
>> 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. > > 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.
Created attachment 146014 [details]
Corrected patch with build scripts and implementation
> 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 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 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 Created attachment 147276 [details]
Corrected patch with build scripts and implementation
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 Created attachment 147550 [details]
Corrected patch with build scripts and implementation - correcting build problems
Created attachment 148338 [details]
Corrected patch with build scripts and implementation - updating
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 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). > > 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?
(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 ?. (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. Created attachment 148752 [details]
Corrected patch with build scripts and implementation
Created attachment 148755 [details]
Corrected patch with build scripts and implementation
(In reply to comment #46) > Created an attachment (id=148755) [details] > Corrected patch with build scripts and implementation LGTM! Thanks. 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? Created attachment 149747 [details]
Corrected patch with build scripts and implementation - updating
(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 on attachment 149747 [details]
Corrected patch with build scripts and implementation - updating
For your convenience. :)
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> All reviewed patches have been landed. Closing bug. Re-opened since this is blocked by 90136 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 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 ? (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. Created attachment 150180 [details]
Corrected patch with build scripts and implementation - updating
Comment on attachment 150180 [details]
Corrected patch with build scripts and implementation - updating
Thank you.
I double checked in my local.
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> All reviewed patches have been landed. Closing bug. |