[Webkit-unassigned] [Bug 68509] [EFL] Implementation of Unit Tests framework for the WebKit-Efl port

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=68509


Raphael Kubo da Costa <kubo at profusion.mobi> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #108121|review?, commit-queue?      |review-
               Flag|                            |




--- Comment #5 from Raphael Kubo da Costa <kubo at profusion.mobi>  2011-09-21 07:10:05 PST ---
(From update of attachment 108121)
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.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.