Bug 90454 - [EFL][WK2] Add API unit tests for Web Intents
Summary: [EFL][WK2] Add API unit tests for Web Intents
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords:
Depends on:
Blocks: 90451 91832
  Show dependency treegraph
 
Reported: 2012-07-03 06:07 PDT by Thiago Marcos P. Santos
Modified: 2012-08-19 05:29 PDT (History)
10 users (show)

See Also:


Attachments
Patch (7.89 KB, patch)
2012-08-17 13:18 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (7.89 KB, patch)
2012-08-17 13:21 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (7.87 KB, patch)
2012-08-19 00:26 PDT, Chris Dumez
kenneth: review+
kenneth: commit-queue-
Details | Formatted Diff | Diff
Patch for landing (7.88 KB, patch)
2012-08-19 03:43 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Thiago Marcos P. Santos 2012-07-03 06:07:49 PDT
The API was added to EWK without unit tests.
Comment 1 Chris Dumez 2012-08-17 13:18:48 PDT
Created attachment 159185 [details]
Patch
Comment 2 Chris Dumez 2012-08-17 13:21:32 PDT
Created attachment 159187 [details]
Patch
Comment 3 Thiago Marcos P. Santos 2012-08-17 16:06:46 PDT
Comment on attachment 159187 [details]
Patch

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

> Source/WebKit2/UIProcess/API/efl/tests/UnitTestUtils/EWK2UnitTestBase.cpp:121
> +void EWK2UnitTestBase::simulateClick(int x, int y)
> +{
> +    Evas* evas = evas_object_evas_get(m_webView);
> +    evas_event_feed_mouse_move(evas, x, y, 0, 0);
> +    evas_event_feed_mouse_down(evas, /* Left */ 1, EVAS_BUTTON_NONE, 0, 0);
> +    evas_event_feed_mouse_up(evas, /* Left */ 1, EVAS_BUTTON_NONE, 0, 0);
> +}

It turned out that the idea of injecting the WebKitTestRunner bundle into our API test runner was overkill. IMO this KISS approach is the way to go. The only two things I would like to suggest here is to change rename to simulateLeftClick (like in the Tools/TestWebKitAPI) and use unsigned int at the coordinates.

> Source/WebKit2/UIProcess/API/efl/tests/resources/intent-request.html:1
> +<html>

Nit: s/intent-request.html/intent_request.html
Comment 4 Chris Dumez 2012-08-18 00:13:48 PDT
Comment on attachment 159187 [details]
Patch

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

>> Source/WebKit2/UIProcess/API/efl/tests/UnitTestUtils/EWK2UnitTestBase.cpp:121
>> +}
> 
> It turned out that the idea of injecting the WebKitTestRunner bundle into our API test runner was overkill. IMO this KISS approach is the way to go. The only two things I would like to suggest here is to change rename to simulateLeftClick (like in the Tools/TestWebKitAPI) and use unsigned int at the coordinates.

Why switch to unsigned? See EFL prototype:
evas_event_feed_mouse_move (Evas *e, int x, int y, unsigned int timestamp, const void *data);

Regarding the renaming to simulateLeftClick(). I'm not convinced either. By default, a click is a left click. Also, I don't believe we will ever add a simulateRightClick() function. If we later need right click, we would probably add a "button" argument to simulateClick().

>> Source/WebKit2/UIProcess/API/efl/tests/resources/intent-request.html:1
>> +<html>
> 
> Nit: s/intent-request.html/intent_request.html

I'm already using intent-service.html so it is consistent.
Comment 5 Kenneth Rohde Christiansen 2012-08-18 16:03:36 PDT
Comment on attachment 159187 [details]
Patch

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

> Source/WebKit2/UIProcess/API/efl/tests/UnitTestUtils/EWK2UnitTestBase.cpp:122
> +

I would just call it mouseClick or similar. I think this is what Qt does. Qt additionally has touchTap() etc.
Comment 6 Chris Dumez 2012-08-19 00:26:29 PDT
Created attachment 159283 [details]
Patch

Rename simulateClick() to mouseClick() as suggested by Kenneth.
Comment 7 Thiago Marcos P. Santos 2012-08-19 02:19:39 PDT
Comment on attachment 159283 [details]
Patch

LGTM. Thanks!
Comment 8 Kenneth Rohde Christiansen 2012-08-19 03:35:49 PDT
Comment on attachment 159283 [details]
Patch

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

> Source/WebKit2/ChangeLog:14
> +        (EWK2UnitTest::EWK2UnitTestBase::simulateClick): Add utility method to simulate a click at given

you need to remember to update the changelog, check the webkit wiki, there is a tool you can hook up with git
Comment 9 Chris Dumez 2012-08-19 03:43:03 PDT
Created attachment 159292 [details]
Patch for landing

Could someone please cq+ ?
Comment 10 WebKit Review Bot 2012-08-19 05:29:34 PDT
Comment on attachment 159292 [details]
Patch for landing

Clearing flags on attachment: 159292

Committed r125974: <http://trac.webkit.org/changeset/125974>
Comment 11 WebKit Review Bot 2012-08-19 05:29:40 PDT
All reviewed patches have been landed.  Closing bug.