We need unit tests for Web intent registration. This would test the "intent,service,register" signalling on the Ewk_View as well as Ewk_Intent_Service.
Created attachment 158609 [details] Patch
Comment on attachment 158609 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=158609&action=review > Source/WebKit2/UIProcess/API/efl/tests/resources/intent-service.html:5 > + <body onload="loaded()"> where is loaded()?
Created attachment 158612 [details] Patch Remove "onload", Thanks Thiago.
Comment on attachment 158612 [details] Patch LGTM. Thanks!
Comment on attachment 158612 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=158612&action=review > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_intents.cpp:45 > + ASSERT_TRUE(service); > + ASSERT_STREQ(ewk_intent_service_action_get(service), "action"); Where do I find information about these macros an this test system?
(In reply to comment #5) > (From update of attachment 158612 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=158612&action=review > > > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_intents.cpp:45 > > + ASSERT_TRUE(service); > > + ASSERT_STREQ(ewk_intent_service_action_get(service), "action"); > > Where do I find information about these macros an this test system? http://trac.webkit.org/wiki/EFLWebKitTests http://code.google.com/p/googletest/wiki/Primer
Comment on attachment 158612 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=158612&action=review >> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_intents.cpp:45 >> + ASSERT_STREQ(ewk_intent_service_action_get(service), "action"); > > Where do I find information about these macros an this test system? These are the docs we have: http://trac.webkit.org/wiki/EFLWebKitTests On the wiki you will find links to gtest docs. gtest is the foundation of our utest framework and where these macros are coming from.
Comment on attachment 158612 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=158612&action=review > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_intents.cpp:40 > + ASSERT_FALSE(*intentRegistered); So we are supposed to ASSERT and use EXPECTS_ macros when it is actually possible to continue? > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_intents.cpp:47 > + ASSERT_STREQ(ewk_intent_service_type_get(service), "type"); > + ASSERT_STREQ(ewk_intent_service_title_get(service), "Title"); Wouldn't EXPECT_STREQ be better here? to continue testing even if that one fails?
Comment on attachment 158612 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=158612&action=review >> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_intents.cpp:47 >> + ASSERT_STREQ(ewk_intent_service_title_get(service), "Title"); > > Wouldn't EXPECT_STREQ be better here? to continue testing even if that one fails? This is not the first time this topic is brought up. I voted for using assertions only if a failure after that point breaks the test beyond repair. A counter argument was that too many failures messages can be misleading in case of a regression.
(In reply to comment #9) > (From update of attachment 158612 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=158612&action=review > > >> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_intents.cpp:47 > >> + ASSERT_STREQ(ewk_intent_service_title_get(service), "Title"); > > > > Wouldn't EXPECT_STREQ be better here? to continue testing even if that one fails? > > This is not the first time this topic is brought up. I voted for using assertions only if a failure after that point breaks the test beyond repair. A counter argument was that too many failures messages can be misleading in case of a regression. Sure, they might be, but they might also not be. At least I think it is good to know if it is the only thing failing or everything else also becomes unreliable. EXPECT_* exists for a purpose.
(In reply to comment #10) > (In reply to comment #9) > > (From update of attachment 158612 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=158612&action=review > > > > >> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_intents.cpp:47 > > >> + ASSERT_STREQ(ewk_intent_service_title_get(service), "Title"); > > > > > > Wouldn't EXPECT_STREQ be better here? to continue testing even if that one fails? > > > > This is not the first time this topic is brought up. I voted for using assertions only if a failure after that point breaks the test beyond repair. A counter argument was that too many failures messages can be misleading in case of a regression. > > Sure, they might be, but they might also not be. At least I think it is good to know if it is the only thing failing or everything else also becomes unreliable. EXPECT_* exists for a purpose. Agreed, I'll update the patch. Thanks.
Created attachment 158707 [details] Patch
Comment on attachment 158707 [details] Patch Clearing flags on attachment: 158707 Committed r125762: <http://trac.webkit.org/changeset/125762>
All reviewed patches have been landed. Closing bug.