Bug 59188 – Reftests can not work on Mac build of WebKit.

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Hayato Ito]

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.

[Hayato Ito]

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.

[Hayato Ito]

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.

[Hayato Ito]

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)

[Dirk Pranke]

(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

[Hayato Ito]

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

[Dirk Pranke]

(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).

[Hayato Ito]

Created attachment 96568 [details]
Patch

[Ojan Vafai]

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.

[WebKit Review Bot]

Comment on attachment 96568 [details]
Patch

Clearing flags on attachment: 96568

Committed r88780: <http://trac.webkit.org/changeset/88780>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.