Bug 91572

Summary: Test cases in DecimalTest should use EXPECT_STREQ for ease of debugging test case
Product: WebKit Reporter: yosin
Component: PlatformAssignee: yosin
Status: RESOLVED FIXED    
Severity: Normal CC: tkent
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 91481    
Attachments:
Description Flags
Patch 1
none
Patch 2 none

yosin
Reported 2012-07-17 19:27:05 PDT
EXPECT_EQ(String(...), decimalNumber.toString()) displays object dump. We can't understand what is expected string.
Attachments
Patch 1 (4.30 KB, patch)
2012-07-17 20:30 PDT, yosin 		no flags
DetailsFormatted DiffDiff
Patch 2 (8.56 KB, patch)
2012-07-17 20:55 PDT, yosin 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
yosin
Comment 1 2012-07-17 20:30:10 PDT
Created attachment 152911 [details] Patch 1
yosin
Comment 2 2012-07-17 20:30:57 PDT
Comment on attachment 152911 [details] Patch 1 Could you review this patch? Thanks in advance.
Kent Tamura
Comment 3 2012-07-17 20:36:32 PDT
Comment on attachment 152911 [details] Patch 1 View in context: https://bugs.webkit.org/attachment.cgi?id=152911&action=review > Source/WebKit/chromium/tests/DecimalTest.cpp:1025 > + EXPECT_STREQ("0", Decimal::zero(Positive).toString().ascii().data()); Please add another macro like EXPECT_DECIMALSTRING(expectation, decimal) EXPECT_STREQ(expectation, decimal.toString().ascii().data())
yosin
Comment 4 2012-07-17 20:55:33 PDT
Created attachment 152913 [details] Patch 2
yosin
Comment 5 2012-07-17 20:58:45 PDT
Comment on attachment 152913 [details] Patch 2 Could you review this patch? Thanks in advance. = Changes since the last review = * Change summary to extend scope of using EXPECT_STREQ for all test cases which use EXPECT_EQ with String class. * Introduce EXPECT_DECIMAL_STREQ * Change DecimalTest::stepDown() and stepUp() for adopting EXPECT_DECIMAL_STREQ.
Kent Tamura
Comment 6 2012-07-17 21:06:05 PDT
Comment on attachment 152913 [details] Patch 2 Looks nice.
yosin
Comment 7 2012-07-17 21:09:11 PDT
Comment on attachment 152913 [details] Patch 2 Clearing flags on attachment: 152913 Committed r122916: <http://trac.webkit.org/changeset/122916>
yosin
Comment 8 2012-07-17 21:09:16 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.