[Webkit-unassigned] [Bug 46953] Add simple API tester for WebKit2

[bugzilla-daemon at webkit.org]

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


Sam Weinig <sam at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED




--- Comment #6 from Sam Weinig <sam at webkit.org>  2010-10-01 12:06:47 PST ---
(In reply to comment #5)
> (From update of attachment 69413 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=69413&action=review
> 
> I didn't look at the Perl very closely.
> 
> When should we integrate this into run-webkit-tests?
> 
> > WebKitTools/ChangeLog:24
> > +        * TestWebKitAPI: Added.
> 
> Maybe TestWebKit2API would be better?

There is not much in here that has to be WebKit2 specific.  I have even provided a test case for WTF/Vector.h.

> 
> > WebKitTools/TestWebKitAPI/PlatformWebView.h:30
> > +#if __APPLE__
> > +#if __OBJC__
> 
> #ifdef is better if you want to turn on the "warn for undefined macros" warning someday.
> 
> > WebKitTools/TestWebKitAPI/PlatformWebView.h:38
> > +typedef WKView* PlatformWKView;
> > +typedef NSWindow* PlatformWindow;
> 
> Misplaced *s.
> 

Done.

> > WebKitTools/TestWebKitAPI/PlatformWebView.h:52
> > +    PlatformWKView platformView() { return m_view; }
> 
> Can be a const member function.

Done.

> 
> > WebKitTools/TestWebKitAPI/ForwardingHeaders/wtf/ASCIICType.h:1
> > +#include <JavaScriptCore/ASCIICType.h>
> 
> Can we just use WebCore's forwarding headers instead? They're copied into WebCore's PrivateHeaders.
> 

I am not sure how to do that. If that works, we should change both WebKitTestRunner and this.


> > WebKitTools/TestWebKitAPI/Tests/WTF/Vector1.cpp:38
> > +TEST(Vector1)
> > +{
> > +    Vector<int> intVector;
> > +    TEST_ASSERT(intVector.isEmpty());
> > +    TEST_ASSERT(intVector.size() == 0);
> > +    TEST_ASSERT(intVector.capacity() == 0);
> > +}
> 
> WTF isn't really part of the WebKit2 API. Why are we testing it here?

See above.

> 
> > WebKitTools/TestWebKitAPI/Tests/WebKit2/BasicTest1.cpp:52
> > +    State* state = ((State*)clientInfo);
> 
> Extra parentheses here. And static_cast/const_cast would maybe be better.

Done.

> > WebKitTools/TestWebKitAPI/Tests/WebKit2/BasicTest1.cpp:57
> > +    TEST_ASSERT(state->didDecidePolicyForNavigationAction);
> > +    TEST_ASSERT(!state->didCommitLoadForFrame);
> > +
> > +    state->didStartProvisionalLoadForFrame = true;
> 
> Should we assert that didStartProvisionalLoadForFrame is false before this?

Done.

> > WebKitTools/TestWebKitAPI/Tests/WebKit2/BasicTest1.cpp:107
> > +    PlatformWebView webView(pageNamespace.get());
> 
> Having PlatformWebView both here and in WebKitTestRunner makes me think that getting the view from the page (rather than vice-versa) might be better overall.

Perhaps.

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