[Webkit-unassigned] [Bug 59369] Convert LayoutTests/fast/events/mouseout-dead-node.html from pixel test to text-based test

Tue Apr 26 09:30:53 PDT 2011

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

--- Comment #6 from Naoki Takano <takano.naoki at gmail.com>  2011-04-26 09:30:53 PST ---
Thank you for your review, Daniel.

I'll do that.

(In reply to comment #5)
> (From update of attachment 91005 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=91005&action=review
> 
> We can further improve this patch.
> 
> I suggest we use the LayoutTests/fast/js/resources/js-test-pre.js and its testPassed(), testFailed() functions instead of out(). See <http://trac.webkit.org/browser/trunk/LayoutTests/fast/events/drag-and-drop.html> for an example of using js-test-pre.js.
> 
> Also, the paths in the this patch aren't from the the top-level project directory. It looks like the patch was created from within the LayoutTests directory. As you can extrapolate from the EWS bot output (e.g. <https://webkit-commit-queue.appspot.com/results/8505750>), the commit queue is not smart enough to apply such a patch. So, it would need to be landed by hand. Instead, I suggest you generate your patch from the top-level WebKit directory.
> 
> > fast/events/mouseout-dead-node-expected.txt:3
> > +you should see PASS below
> > +you should see PASS below
> 
> Can we remove this text when this test is run using Dump Render Tree so as to simplify the result output?
> 
> See <http://trac.webkit.org/browser/trunk/LayoutTests/fast/events/drag-and-drop.html> for an example of this. In particular, see <http://trac.webkit.org/browser/trunk/LayoutTests/fast/events/drag-and-drop.html#L181> where we remove the test container so that we only leave the test description and test results in the output.
> 
> > fast/events/mouseout-dead-node.html:1
> > -<body onload='test()'>
> > +<body onload='autoTest()'>
> 
> No need to rename this. This name of the function is sufficiently descriptive as-is.
> 
> > fast/events/mouseout-dead-node.html:-26
> > -    if (window.eventSender) {
> 
> We shouldn't change this to layoutTestController since the code within this block uses aspects of the EventSender object. Although, in practice the existence of the EventSender object can be seen as equivalent to the presence of the LayoutTestController object ;=> we are running the test under Dump Render Tree, I suggest we instead only use LayoutTestController and EventSender functionality when we know such objects exist, respectively. For example, we should check for the presence of the EventSender object before using eventSender.mouseMoveTo().
> 
> > fast/events/mouseout-dead-node.html:26
> > +    if (window.layoutTestController) {
> > +        layoutTestController.dumpAsText();
> 
> I would hoist this if-statement outside of this function so that it's at the top of the <script> body.

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