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+

Kinuko Yasuda
Reported 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
Attachments
Patch (1.49 KB, patch)
2013-03-04 03:52 PST, Kinuko Yasuda 		no flags
DetailsFormatted DiffDiff
Patch (2.06 KB, patch)
2013-03-04 04:36 PST, Kinuko Yasuda 		jochen: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Kinuko Yasuda
Comment 1 2013-03-04 03:52:21 PST
Created attachment 191197 [details] Patch
jochen
Comment 2 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
Kinuko Yasuda
Comment 3 2013-03-04 04:24:35 PST
Oh we can use the familiar DISABLED_ prefix here... Sounds much better, I'll update the patch.
Kinuko Yasuda
Comment 4 2013-03-04 04:36:39 PST
Created attachment 191204 [details] Patch
Kinuko Yasuda
Comment 5 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)
Kinuko Yasuda
Comment 6 2013-03-04 04:43:39 PST
CC-ing the original patch author too
jochen
Comment 7 2013-03-04 04:46:47 PST
Comment on attachment 191204 [details] Patch ok
Kinuko Yasuda
Comment 8 2013-03-04 05:02:53 PST
Committed r144619: <http://trac.webkit.org/changeset/144619>
Darin Adler
Comment 9 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!
Darin Adler
Comment 10 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.
Geoffrey Garen
Comment 11 2013-03-11 15:17:06 PDT
Committed r145412: <http://trac.webkit.org/changeset/145412>
Note You need to log in before you can comment on or make changes to this bug.