Bug 97624 - [EFL][WK2] Some expected and actual parameters in unit tests are reversed
Summary: [EFL][WK2] Some expected and actual parameters in unit tests are reversed
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Linux
: P2 Normal
Assignee: Jinwoo Song
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-25 19:52 PDT by Jinwoo Song
Modified: 2012-09-28 09:12 PDT (History)
5 users (show)

See Also:


Attachments
Patch (50.46 KB, patch)
2012-09-28 04:01 PDT, Jinwoo Song
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jinwoo Song 2012-09-25 19:52:49 PDT
According to the gtest guide, EXPECT_XXX(expected, actual) is recommended but some unit tests do not follow this.

http://code.google.com/p/googletest/wiki/Primer
"In the event of a failure, Google Test prints both val1 and val2 . In ASSERT_EQ* and EXPECT_EQ* (and all other equality assertions we'll introduce later), you should put the expression you want to test in the position of actual, and put its expected value in expected, as Google Test's failure messages are optimized for this convention."
Comment 1 Jinwoo Song 2012-09-28 04:01:06 PDT
Created attachment 166198 [details]
Patch
Comment 2 Raphael Kubo da Costa (:rakuco) 2012-09-28 05:10:12 PDT
Comment on attachment 166198 [details]
Patch

Looks harmless.
Comment 3 Chris Dumez 2012-09-28 05:21:18 PDT
Comment on attachment 166198 [details]
Patch

Interesting. Looks good, thanks.
Comment 4 Laszlo Gombos 2012-09-28 06:05:54 PDT
Comment on attachment 166198 [details]
Patch

r=me.
Comment 5 WebKit Review Bot 2012-09-28 07:00:19 PDT
Comment on attachment 166198 [details]
Patch

Clearing flags on attachment: 166198

Committed r129892: <http://trac.webkit.org/changeset/129892>
Comment 6 WebKit Review Bot 2012-09-28 07:00:24 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 Sudarsana Nagineni (babu) 2012-09-28 08:28:19 PDT
This is causing a failure in API tests.

http://build.webkit.org/builders/EFL%20Linux%2064-bit%20Debug/builds/6521/steps/API%20tests/logs/stdio

/home/buildslave-1/webkit-buildslave/efl-linux-64-debug/build/Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_cookie_manager.cpp:212: Failure
Value of: countHostnamesWithCookies(cookieManager)
  Actual: 0
Expected: 2
[  FAILED  ] EWK2UnitTestBase.ewk_cookie_manager_changes_watch (136 ms)
Comment 8 Sudarsana Nagineni (babu) 2012-09-28 08:41:03 PDT
Comment on attachment 166198 [details]
Patch

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

> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_context.cpp:171
> +    ASSERT_EQ(ewk_context_ref(context), context);

This change is not needed.

> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_cookie_manager.cpp:212
> +    ASSERT_EQ(2, countHostnamesWithCookies(cookieManager));

Expected value should be 0, not 2.
Comment 9 Sudarsana Nagineni (babu) 2012-09-28 08:45:25 PDT
(In reply to comment #8)
> > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_context.cpp:171
> > +    ASSERT_EQ(ewk_context_ref(context), context);
> 
> This change is not needed.
> 
> > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_cookie_manager.cpp:212
> > +    ASSERT_EQ(2, countHostnamesWithCookies(cookieManager));
> 
> Expected value should be 0, not 2.

These are now fixed in http://trac.webkit.org/changeset/129900
Thanks Raphael.
Comment 10 Jinwoo Song 2012-09-28 09:12:57 PDT
Thanks for fixing, Raphael.