Bug 59188

Summary: Reftests can not work on Mac build of WebKit.
Product: WebKit Reporter: Hayato Ito <hayato>
Component: Tools / TestsAssignee: Hayato Ito <hayato>
Status: RESOLVED FIXED    
Severity: Normal CC: dpranke, hayato, ojan, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 36065, 58858    
Attachments:
Description Flags
Patch none

Description Hayato Ito 2011-04-22 04:33:37 PDT
Please see the following bug for the detail:
https://bugs.webkit.org/show_bug.cgi?id=58858

The issue comes from the different behavior between ChrominDriver and WebKitDriver which new-run-webkit-tests uses.
I'll update this entry with more details later.
Comment 1 Hayato Ito 2011-05-10 20:51:29 PDT
Let me update the status.

It seems that WebKitDriver.run_test() returns 'None' as an image_hash value when an actual image_hash matches the given driver input's image_hash.
Whereas, ChromiumDriver.run_test() returns an actual image_hash value as is when the actual image_hash matches the given driver input's image_hash.

I think that this different behavior doesn't cause any problems to existing pixel tests tests because existing pixel tests skips 'comparing images' if an actual image_hash is 'None'.

But reftests always try to compare image_hash values of both HTML files. That is the root cause.
Comment 2 Hayato Ito 2011-05-10 21:05:09 PDT
I guess there are two ways to fix the issue:

A). Update WebKitDriver, maybe including a DumpRenderTree itself used by WebKitDriver, so that it returns an actual image_hash value even when image_hash_values matches.

B). Update reftests runner so that it can handle 'None' value from WebKitDriver.
Comment 3 Hayato Ito 2011-05-10 21:14:04 PDT
Hi Dirk, Ojan,

(In reply to comment #1)
> Let me update the status.
> 
> It seems that WebKitDriver.run_test() returns 'None' as an image_hash value when an actual image_hash matches the given driver input's image_hash.
> Whereas, ChromiumDriver.run_test() returns an actual image_hash value as is when the actual image_hash matches the given driver input's image_hash.

Could you tell me that this difference between WebKitDriver and ChromiumDriver is an expected one?



(In reply to comment #2)
> I guess there are two ways to fix the issue:
> 
> A). Update WebKitDriver, maybe including a DumpRenderTree itself used by WebKitDriver, so that it returns an actual image_hash value even when image_hash_values matches.
> 
> B). Update reftests runner so that it can handle 'None' value from WebKitDriver.


If this different behavior is unexpected one, I'd like to choose A) to fix the issue rather than B)
Comment 4 Dirk Pranke 2011-05-10 23:32:46 PDT
(In reply to comment #3)
> Hi Dirk, Ojan,
> 
> (In reply to comment #1)
> > Let me update the status.
> > 
> > It seems that WebKitDriver.run_test() returns 'None' as an image_hash value when an actual image_hash matches the given driver input's image_hash.
> > Whereas, ChromiumDriver.run_test() returns an actual image_hash value as is when the actual image_hash matches the given driver input's image_hash.
> 
> Could you tell me that this difference between WebKitDriver and ChromiumDriver is an expected one?
> 

No, it's a bug. The Driver should always return an image hash if it got one from DRT. Feel free to upload a patch to fix this :)

Seems like pixel tests couldn't possibly work on the Mac port if this was true, but maybe there's something else at play as well. Then again, I haven't tried pixel tests on the Mac port in a long time.

- Dirk
Comment 5 Hayato Ito 2011-05-10 23:44:29 PDT
Thank you for the replay, Dirk.

Okay. I'll take a look further. I suspect DumpRenderTree itself doesn't return image_hash on Mac Port.

(In reply to comment #4)
> (In reply to comment #3)
> > Hi Dirk, Ojan,
> > 
> > (In reply to comment #1)
> > > Let me update the status.
> > > 
> > > It seems that WebKitDriver.run_test() returns 'None' as an image_hash value when an actual image_hash matches the given driver input's image_hash.
> > > Whereas, ChromiumDriver.run_test() returns an actual image_hash value as is when the actual image_hash matches the given driver input's image_hash.
> > 
> > Could you tell me that this difference between WebKitDriver and ChromiumDriver is an expected one?
> > 
> 
> No, it's a bug. The Driver should always return an image hash if it got one from DRT. Feel free to upload a patch to fix this :)
> 
> Seems like pixel tests couldn't possibly work on the Mac port if this was true, but maybe there's something else at play as well. Then again, I haven't tried pixel tests on the Mac port in a long time.
> 
> - Dirk
Comment 6 Dirk Pranke 2011-05-11 09:39:16 PDT
(In reply to comment #5)
> Thank you for the replay, Dirk.
> 
> Okay. I'll take a look further. I suspect DumpRenderTree itself doesn't return image_hash on Mac Port.
> 

If it doesn't, you can always return the value that is passed in. Note that you don't (and shouldn't) need to return a value if you are not doing pixel tests (and I don't think one is even passed in in that case).
Comment 7 Hayato Ito 2011-06-09 03:31:22 PDT
Created attachment 96568 [details]
Patch
Comment 8 Ojan Vafai 2011-06-09 14:28:12 PDT
Comment on attachment 96568 [details]
Patch

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

> Tools/ChangeLog:12
> +        No tests since I'll add sample reftests to make sure mismatch reftests work later,
> +        which should be in a separate patch.

I'd actually prefer putting the sample reftests in this patch, but a separate patch is fine too.
Comment 9 WebKit Review Bot 2011-06-13 23:31:31 PDT
Comment on attachment 96568 [details]
Patch

Clearing flags on attachment: 96568

Committed r88780: <http://trac.webkit.org/changeset/88780>
Comment 10 WebKit Review Bot 2011-06-13 23:31:36 PDT
All reviewed patches have been landed.  Closing bug.