| Summary: | commit-queue mentions "Text diff mismatch" 4 times instead of once per failure | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Eric Seidel (no email) <eric> | ||||||
| Component: | New Bugs | Assignee: | Eric Seidel (no email) <eric> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | abarth, commit-queue, mihaip, ojan | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Other | ||||||||
| OS: | OS X 10.5 | ||||||||
| Attachments: |
|
||||||||
|
Description
Eric Seidel (no email)
2011-01-06 12:04:46 PST
Created attachment 78143 [details]
Patch
Comment on attachment 78143 [details] Patch 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). Comment on attachment 78143 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=78143&action=review >> Tools/Scripts/webkitpy/layout_tests/layout_package/test_failures.py:88 > > You mean that we don't use TestFailure directly, but instead use subclaseses like FailureCrash? Perhaps the comment could be more explicit. We basically always pass around FooFailure() instead of FooFailure (the class) directly, yet we *never* actually store any state on the instances. I want to re-write this module so that it works more like tool/steps does. Where you use it by importing the module, and then reference these as test_failures.Crash and don't ever bother instantiting one. Basically like a big enum. >> Tools/Scripts/webkitpy/layout_tests/layout_package/test_failures.py:97 > > 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). I was trying to avoid having hash(FooFailure) == hash("FooFailure") isn't __class__ as singleton? So it should always have the same hash? I could alternatively just do hash(sef.__class__.__name__) + 5, to avoid it being the same hash as "FooFailure" Created attachment 78152 [details]
Patch
Comment on attachment 78152 [details] Patch Clearing flags on attachment: 78152 Committed r75191: <http://trac.webkit.org/changeset/75191> All reviewed patches have been landed. Closing bug. |