WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
60862
Convert api tester over to using gtest expectations directly
https://bugs.webkit.org/show_bug.cgi?id=60862
Summary
Convert api tester over to using gtest expectations directly
Sam Weinig
Reported
2011-05-15 17:42:58 PDT
Convert api tester over to using gtest expectations directly
Attachments
Patch
(37.09 KB, patch)
2011-05-15 17:45 PDT
,
Sam Weinig
no flags
Details
Formatted Diff
Diff
Patch
(37.13 KB, patch)
2011-05-15 17:49 PDT
,
Sam Weinig
no flags
Details
Formatted Diff
Diff
Patch
(38.20 KB, patch)
2011-05-16 10:26 PDT
,
Sam Weinig
levin
: review+
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Sam Weinig
Comment 1
2011-05-15 17:45:30 PDT
Created
attachment 93595
[details]
Patch
Sam Weinig
Comment 2
2011-05-15 17:46:59 PDT
I am new to this gtest thing, so I am not sure how idiomatic these changes are. I couldn't find a good way to state the expectation that a pointer was not null for instance, so I am just using EXPECT_TRUE(pointer).
WebKit Review Bot
Comment 3
2011-05-15 17:47:23 PDT
Attachment 93595
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/TestWebKitAPI/Ja..." exit_code: 1 Tools/TestWebKitAPI/PlatformUtilities.h:57: The parameter name "string" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 28 files If any of these errors are false positives, please file a bug against check-webkit-style.
Sam Weinig
Comment 4
2011-05-15 17:49:14 PDT
Created
attachment 93597
[details]
Patch
WebKit Review Bot
Comment 5
2011-05-15 17:51:09 PDT
Attachment 93597
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/TestWebKitAPI/Ja..." exit_code: 1 Tools/TestWebKitAPI/PlatformUtilities.h:57: The parameter name "string" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 28 files If any of these errors are false positives, please file a bug against check-webkit-style.
Darin Adler
Comment 6
2011-05-15 20:20:33 PDT
Comment on
attachment 93597
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=93597&action=review
> Tools/TestWebKitAPI/PlatformUtilities.h:58 > std::string toSTD(WKStringRef string); > +std::string toSTD(WKRetainPtr<WKStringRef> string); > +std::string toSTD(const char* string);
That StyleQueue is so smart. We should remove those names!
David Levin
Comment 7
2011-05-15 21:46:53 PDT
Comment on
attachment 93597
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=93597&action=review
> Tools/TestWebKitAPI/PlatformUtilities.h:63 > +static inline ::testing::AssertionResult assertWKStrigEqual(const char* expected_expression, const char* actual_expression, T expected, U actual)
typo: Strig I'm surprised that EXPECT_STREQ just didn't work but this is perfect.
> Tools/TestWebKitAPI/Tests/WebKit2/AboutBlankLoad.cpp:38 > + EXPECT_WK_STREQ(Util::MIMETypeForWKURLResponse(response), "text/html");
This is fine. The one odd thing about this way of doing it is that your expected and actual are reversed, so if there is a mismatch, then it will say EXPECT: "foo" ACTUAL: "text/html" I always find the ordering of this to be odd, but I think it was done to be consistent with what other pre-existing test frameworks were doing like JUnit. (This ordering issue to true throughout these changes.)
David Levin
Comment 8
2011-05-15 21:49:11 PDT
(In reply to
comment #2
)
> I couldn't find a good way to state the expectation that a pointer was not null for instance, so I am just using EXPECT_TRUE(pointer).
That's perfect. I think it was debated at one point about adding something specifically for this but I think they didn't see enough value over just plain EXPECT_TRUE. (Actually, I'm not 100% sure of the reasons, but I could probably find out why there isn't a specific EXPECT for this if you want to know.)
Sam Weinig
Comment 9
2011-05-16 10:26:29 PDT
Created
attachment 93665
[details]
Patch
Sam Weinig
Comment 10
2011-05-16 10:27:46 PDT
This version fixes the expected/actual ordering issue and adds NULL/NON_NULL macros for convenience.
David Levin
Comment 11
2011-05-16 10:37:40 PDT
Comment on
attachment 93665
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=93665&action=review
> Tools/TestWebKitAPI/PlatformUtilities.h:63 > +static inline ::testing::AssertionResult assertWKStrigEqual(const char* expected_expression, const char* actual_expression, T expected, U actual)
typo: strig I'm surprised that EXPECT_STREQ doesn't work for you. (I see you used it with char* inside of TEST(WebKit2, WKString)
Sam Weinig
Comment 12
2011-05-16 11:36:03 PDT
(In reply to
comment #11
)
> (From update of
attachment 93665
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=93665&action=review
> > > Tools/TestWebKitAPI/PlatformUtilities.h:63 > > +static inline ::testing::AssertionResult assertWKStrigEqual(const char* expected_expression, const char* actual_expression, T expected, U actual) > > typo: strig > > I'm surprised that EXPECT_STREQ doesn't work for you. (I see you used it with char* inside of TEST(WebKit2, WKString)
EXPECT_STREQ only works with char*, the idea of this is you can pass a WKStringRef or a WKRetainPtr<WKStringRef> as either parameter.
Sam Weinig
Comment 13
2011-05-16 13:28:25 PDT
Committed
r86602
: <
http://trac.webkit.org/changeset/86602
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug