Bug 59188 - Reftests can not work on Mac build of WebKit.
Summary: Reftests can not work on Mac build of WebKit.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Hayato Ito
URL:
Keywords:
Depends on:
Blocks: 36065 58858
  Show dependency treegraph
 
Reported: 2011-04-22 04:33 PDT by Hayato Ito
Modified: 2011-06-13 23:31 PDT (History)
4 users (show)

See Also:


Attachments
Patch (1.68 KB, patch)
2011-06-09 03:31 PDT, Hayato Ito
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.