[webkit-reviews] review denied: [Bug 52002] commit-queue mentions "Text diff mismatch" 4 times instead of once per failure : [Attachment 78143] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jan 6 12:21:30 PST 2011


Mihai Parparita <mihaip at chromium.org> has denied Eric Seidel
<eric at webkit.org>'s request for review:
Bug 52002: commit-queue mentions "Text diff mismatch" 4 times instead of once
per failure
https://bugs.webkit.org/show_bug.cgi?id=52002

Attachment 78143: Patch
https://bugs.webkit.org/attachment.cgi?id=78143&action=review

------- Additional Comments from Mihai Parparita <mihaip at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=78143&action=review

> Tools/Scripts/webkitpy/layout_tests/layout_package/test_failures.py:88
> +    # FIXME: We don't actually use TestFailure objects as instances,

You mean that we don't use TestFailure directly, but instead use subclaseses
like FailureCrash? Perhaps the comment could be more explicit.

> Tools/Scripts/webkitpy/layout_tests/layout_package/test_failures.py:97
> +	   # An attempt to make sure that __class__ and self don't hash to the
same value.

Though it may not matter in practice, it seems like the implementation of this
should just be hash(self.__class__.__name__), since you never compare
self.__class__ directly in __eq__. (roughly, you should only hash things that
you compare in __eq__, otherwise they'll end up in different buckets even
though they would pass the equality test).


More information about the webkit-reviews mailing list