Fix erros in below API tests, ewk_context_additional_plugin_path_set(), ewk_view_contents_size_get(), ewk_context_menu_item_select(), ewk_view_script_execute(). [ RUN ] EWK2ContextTest.ewk_context_additional_plugin_path_set ERR<9873>: /home/shiva.jm/webkit-git/svngitmay14/WebKit/Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:490 ewk_context_additional_plugin_path_set() safety check failed: path == NULL [ OK ] EWK2ContextTest.ewk_context_additional_plugin_path_set (131 ms) [----------] 12 tests from EWK2ContextTest (11581 ms total) [ RUN ] EWK2ViewTest.ewk_view_contents_size_get ERR<11319>: /home/shiva.jm/webkit-git/svngitmay14/WebKit/Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:54 toEwkViewChecked() safety check failed: evasObject == NULL CRI<11319>: /home/shiva.jm/webkit-git/svngitmay14/WebKit/Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:763 ewk_view_contents_size_get() no private data for object (nil) [ OK ] EWK2ViewTest.ewk_view_contents_size_get (790 ms) [----------] 36 tests from EWK2ViewTest (29847 ms total) [----------] 1 test from EWK2UnitTestBase [ RUN ] EWK2UnitTestBase.ewk_view_script_execute ERR<11319>: /home/shiva.jm/webkit-git/svngitmay14/WebKit/Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:692 ewk_view_script_execute() safety check failed: script == NULL [ OK ] EWK2UnitTestBase.ewk_view_script_execute (675 ms) Expected: true ERR<11033>: /home/shiva.jm/webkit-git/svngitmay14/WebKit/Source/WebKit2/UIProcess/API/efl/ewk_context_menu.cpp:158 ewk_context_menu_item_select() safety check failed: item == NULL /home/shiva.jm/webkit-git/svngitmay14/WebKit/Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:167: Failure Value of: text Actual: "aa"
Created attachment 231498 [details] Patch ewk_view_contents_size_get unit test has tested unsupported evas object type. But ewk APIs gives critical error, when given evas object is not ewk type. So, we can't test wrong evas object type by using current test. so Removing the test line.
Comment on attachment 231498 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=231498&action=review > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp:-1146 > - ewk_view_contents_size_get(0, &contentsWidth, &contentsHeight); I think this is to check if width and height can be initialized with 0 when Evas_Object is null. To be honest, I don't know why we have to initialize width/height when evas object is null. Eina_Bool ewk_view_contents_size_get(const Evas_Object* ewkView, Evas_Coord* width, Evas_Coord* height) { EwkView* impl = toEwkViewChecked(ewkView); if (EINA_UNLIKELY(!impl)) { EINA_LOG_CRIT("no private data for object %p", ewkView); if (width) *width = 0; if (height) *height = 0; return false; } If we remove this test logic, I think we have to remove the initialization code together.
Comment on attachment 231498 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=231498&action=review > Source/WebKit2/ChangeLog:9 > + ewk_view_contents_size_get unit test has tested unsupported evas object type. But ewk APIs gives critical error, when given evas object > + is not ewk type. So, we can't test wrong evas object type by using current test. so Removing the test line. I am not sure what is the problem. It is intentionally tested. >> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp:-1146 >> - ewk_view_contents_size_get(0, &contentsWidth, &contentsHeight); > > I think this is to check if width and height can be initialized with 0 when Evas_Object is null. To be honest, I don't know why we have to initialize width/height when evas object is null. > > Eina_Bool ewk_view_contents_size_get(const Evas_Object* ewkView, Evas_Coord* width, Evas_Coord* height) > { > EwkView* impl = toEwkViewChecked(ewkView); > if (EINA_UNLIKELY(!impl)) { > EINA_LOG_CRIT("no private data for object %p", ewkView); > if (width) > *width = 0; > if (height) > *height = 0; > > return false; > } > > If we remove this test logic, I think we have to remove the initialization code together. Maybe, I don't have objection about removing the logic that clears the value. But I still want test case although we remove this logic. This test case explains the behavior of ewebkit well when wrong ewkView is given.
If you want clear log, it looks better to turn off the EINA_LOG for our test cases. Test cases should cover not only positive cases but also negative cases.
(In reply to comment #4) > If you want clear log, it looks better to turn off the EINA_LOG for our test cases. > > Test cases should cover not only positive cases but also negative cases. How about turning on EINA_LOG only when debug mode ? Personally I hope to see the message on debug mode. ;)
I would like to postpone to fix this problem later, since to remove error logs for negative test, we may need to seperate negative tests or enable error logs in debug mode only