Bug 72659

Summary: Pointer Lock: Initial Tests for navigator.webkitPonter
Product: WebKit Reporter: Vincent Scheib <scheib>
Component: New BugsAssignee: Vincent Scheib <scheib>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, jchaffraix, tkent, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
added MouseEvent.webkitMovementX/Y test
none
switched to js-test-pre/post.js
none
rebase to TOT fishd: review+

Description Vincent Scheib 2011-11-17 14:28:32 PST
Pointer Lock: Initial Tests for navigator.webkitPonter
Comment 1 Vincent Scheib 2011-11-17 14:29:37 PST
Created attachment 115683 [details]
Patch
Comment 2 Vincent Scheib 2011-11-17 14:37:36 PST
Created attachment 115685 [details]
Patch
Comment 3 Vincent Scheib 2011-11-18 15:19:51 PST
Created attachment 115888 [details]
added MouseEvent.webkitMovementX/Y test
Comment 4 Julien Chaffraix 2011-11-18 15:36:59 PST
Comment on attachment 115888 [details]
added MouseEvent.webkitMovementX/Y test

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

> Tools/ChangeLog:9
> +        (TestShell::TestShell):

It would be nice to fill in this ChangeLog. Sometimes like:

Enabling PointLock in TestShell

would be enough.

> LayoutTests/pointer-lock/pointer-lock-test.js:10
> +    layoutTestController.dumpAsText(runPixelTests);

Be careful with that. dumpAsText(true) is likely not doing what you think -> http://trac.webkit.org/wiki/Writing%20Layout%20Tests%20for%20DumpRenderTree#Howtodisablerendertreedumpsbutkeepthepixels

> LayoutTests/pointer-lock/pointer-lock-test.js:16
> +    // FIXME: WKTR does not yet support the keyDown() message.  Do a mouseDown here

WKTR?

> LayoutTests/pointer-lock/pointer-lock-test.js:31
> +        console = document.createElement('div');

This is a nice pattern. Most tests would just query an element in the main html page and register themselves on the load event (so that the element has been parsed).

> LayoutTests/pointer-lock/pointer-lock-test.js:127
> +        logResult(eval(testFuncString), "EVENT(" + eventName + ") TEST(" + testFuncString + ")");

Any reasons you don't reuse the testing framework used in the LayoutTests (js-test-pre.js + js-test-post.js). Look at one of the TEMPLATE.html files on how to use it.
Comment 5 Vincent Scheib 2011-11-22 10:54:43 PST
Comment on attachment 115888 [details]
added MouseEvent.webkitMovementX/Y test

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

>> Tools/ChangeLog:9
>> +        (TestShell::TestShell):
> 
> It would be nice to fill in this ChangeLog. Sometimes like:
> 
> Enabling PointLock in TestShell
> 
> would be enough.

Done

>> LayoutTests/pointer-lock/pointer-lock-test.js:127
>> +        logResult(eval(testFuncString), "EVENT(" + eventName + ") TEST(" + testFuncString + ")");
> 
> Any reasons you don't reuse the testing framework used in the LayoutTests (js-test-pre.js + js-test-post.js). Look at one of the TEMPLATE.html files on how to use it.

Good idea. I have switched to using js-test-pre/post.js.
Comment 6 Vincent Scheib 2011-11-22 10:55:36 PST
Created attachment 116248 [details]
switched to js-test-pre/post.js
Comment 7 Vincent Scheib 2011-11-22 11:14:50 PST
Created attachment 116254 [details]
rebase to TOT
Comment 8 Vincent Scheib 2011-11-22 13:41:22 PST
Committed r101025: <http://trac.webkit.org/changeset/101025>