Bug 111284

Summary: Some StringHasher tests are broken because of missing null termination
Product: WebKit Reporter: Kinuko Yasuda <kinuko>
Component: Tools / TestsAssignee: Kinuko Yasuda <kinuko>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, ggaren, jochen
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Android   
OS: Android   
Attachments:
Description Flags
Patch
none
Patch jochen: review+

Description Kinuko Yasuda 2013-03-03 22:19:58 PST 
[chromium] TestWebKitAPI WTF.StringHasher_addCharacters is broken on Chromium Android.

[ RUN      ] WTF.StringHasher_addCharacters
../../third_party/WebKit/Tools/TestWebKitAPI/Tests/WTF/StringHasher.cpp:173: Failure
Value of: hasher.hash()
  Actual: 452317957
Expected: testBHash5
Which is: 2032145993

[ RUN      ] WTF.StringHasher_addCharactersAssumingAligned
../../third_party/WebKit/Tools/TestWebKitAPI/Tests/WTF/StringHasher.cpp:328: Failure
Value of: hasher.hash()
  Actual: 452317957
Expected: testBHash5
Which is: 2032145993
[  FAILED  ] WTF.StringHasher_addCharactersAssumingAligned
Comment 1 Kinuko Yasuda 2013-03-04 03:52:21 PST 
Created attachment 191197 [details]
Patch
Comment 2 jochen 2013-03-04 04:22:31 PST 
Are just the two tests failing?

I'd just rename the tests to MAYBE_xxx and use an ifdef to rename MAYBE_xxx to either xxx or DISABLED_xxx
Comment 3 Kinuko Yasuda 2013-03-04 04:24:35 PST 
Oh we can use the familiar DISABLED_ prefix here...  Sounds much better, I'll update the patch.
Comment 4 Kinuko Yasuda 2013-03-04 04:36:39 PST 
Created attachment 191204 [details]
Patch
Comment 5 Kinuko Yasuda 2013-03-04 04:40:42 PST 
Updated the patch. (I guess this one can probably go without review-- let me just make sure it doesn't break)
Comment 6 Kinuko Yasuda 2013-03-04 04:43:39 PST 
CC-ing the original patch author too
Comment 7 jochen 2013-03-04 04:46:47 PST 
Comment on attachment 191204 [details]
Patch

ok
Comment 8 Kinuko Yasuda 2013-03-04 05:02:53 PST 
Committed r144619: <http://trac.webkit.org/changeset/144619>
Comment 9 Darin Adler 2013-03-04 14:59:45 PST 
Very interesting. The fact that you get the wrong hash here on Android probably indicates a real problem. Someone should investigate what’s going on!
Comment 10 Darin Adler 2013-03-04 15:00:40 PST 
It’s bizarre that the code now references this bug. Instead, someone should file a bug about the hash values being incorrect and investigate why. And that’s the bug that the disabled test should point to.
Comment 11 Geoffrey Garen 2013-03-11 15:17:06 PDT 
Committed r145412: <http://trac.webkit.org/changeset/145412>