Commit 18374b34 authored by Dave Schuyler's avatar Dave Schuyler Committed by Commit Bot

[search_provider] clean up unit test.

Cleanup: there is no logic changes in this CL.

I tried making an edit in this file and found there were many small
style issues*. These changes, done separately, can keep my other CL
cleaner:
- fix use of NULL instead of nullptr
- fix lines over 80 chars long
- fix 3 space indenting
- fix 1 space indent on eol comments
- fix use of assert true for comparison
- fix suggetion to suggestion

*Which create a big list of warnings, making it hard to see if I've
introduced new issues.

TBR=mpearson@chromium.org

Bug: None
Change-Id: I4252ea4c62f33ff201ee80179150c7f10b13084f
Reviewed-on: https://chromium-review.googlesource.com/996752
Commit-Queue: Dave Schuyler <dschuyler@chromium.org>
Reviewed-by: default avatarDave Schuyler <dschuyler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548275}
parent 6bdfed75
...@@ -105,7 +105,7 @@ void SearchProviderForTest::RecordDeletionResult(bool success) { ...@@ -105,7 +105,7 @@ void SearchProviderForTest::RecordDeletionResult(bool success) {
is_success_ = success; is_success_ = success;
} }
} // namespace } // namespace
// SearchProviderTest --------------------------------------------------------- // SearchProviderTest ---------------------------------------------------------
...@@ -153,11 +153,11 @@ class SearchProviderTest : public testing::Test, ...@@ -153,11 +153,11 @@ class SearchProviderTest : public testing::Test,
}; };
SearchProviderTest() SearchProviderTest()
: default_t_url_(NULL), : default_t_url_(nullptr),
term1_(ASCIIToUTF16("term1")), term1_(ASCIIToUTF16("term1")),
keyword_t_url_(NULL), keyword_t_url_(nullptr),
keyword_term_(ASCIIToUTF16("keyword")), keyword_term_(ASCIIToUTF16("keyword")),
run_loop_(NULL) { run_loop_(nullptr) {
ResetFieldTrialList(); ResetFieldTrialList();
} }
...@@ -177,7 +177,9 @@ class SearchProviderTest : public testing::Test, ...@@ -177,7 +177,9 @@ class SearchProviderTest : public testing::Test,
// Adds a search for |term|, using the engine |t_url| to the history, and // Adds a search for |term|, using the engine |t_url| to the history, and
// returns the URL for that search. // returns the URL for that search.
GURL AddSearchToHistory(TemplateURL* t_url, base::string16 term, int visit_count); GURL AddSearchToHistory(TemplateURL* t_url,
base::string16 term,
int visit_count);
// Looks for a match in |provider_| with |contents| equal to |contents|. // Looks for a match in |provider_| with |contents| equal to |contents|.
// Sets |match| to it if found. Returns whether |match| was set. // Sets |match| to it if found. Returns whether |match| was set.
...@@ -202,7 +204,7 @@ class SearchProviderTest : public testing::Test, ...@@ -202,7 +204,7 @@ class SearchProviderTest : public testing::Test,
bool prefer_keyword); bool prefer_keyword);
// Calls QueryForInput(), finishes any suggest query, then if |wyt_match| is // Calls QueryForInput(), finishes any suggest query, then if |wyt_match| is
// non-NULL, sets it to the "what you typed" entry for |text|. // not nullptr, sets it to the "what you typed" entry for |text|.
void QueryForInputAndSetWYTMatch(const base::string16& text, void QueryForInputAndSetWYTMatch(const base::string16& text,
AutocompleteMatch* wyt_match); AutocompleteMatch* wyt_match);
...@@ -252,7 +254,7 @@ class SearchProviderTest : public testing::Test, ...@@ -252,7 +254,7 @@ class SearchProviderTest : public testing::Test,
std::unique_ptr<ChromeAutocompleteProviderClient> client_; std::unique_ptr<ChromeAutocompleteProviderClient> client_;
scoped_refptr<SearchProviderForTest> provider_; scoped_refptr<SearchProviderForTest> provider_;
// If non-NULL, OnProviderUpdate quits the current |run_loop_|. // If not nullptr, OnProviderUpdate quits the current |run_loop_|.
base::RunLoop* run_loop_; base::RunLoop* run_loop_;
DISALLOW_COPY_AND_ASSIGN(SearchProviderTest); DISALLOW_COPY_AND_ASSIGN(SearchProviderTest);
...@@ -319,7 +321,7 @@ void SearchProviderTest::TearDown() { ...@@ -319,7 +321,7 @@ void SearchProviderTest::TearDown() {
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
// Shutdown the provider before the profile. // Shutdown the provider before the profile.
provider_ = NULL; provider_ = nullptr;
} }
void SearchProviderTest::RunTest(TestData* cases, void SearchProviderTest::RunTest(TestData* cases,
...@@ -354,7 +356,7 @@ void SearchProviderTest::RunTest(TestData* cases, ...@@ -354,7 +356,7 @@ void SearchProviderTest::RunTest(TestData* cases,
void SearchProviderTest::OnProviderUpdate(bool updated_matches) { void SearchProviderTest::OnProviderUpdate(bool updated_matches) {
if (run_loop_ && provider_->done()) { if (run_loop_ && provider_->done()) {
run_loop_->Quit(); run_loop_->Quit();
run_loop_ = NULL; run_loop_ = nullptr;
} }
} }
...@@ -437,8 +439,9 @@ GURL SearchProviderTest::AddSearchToHistory(TemplateURL* t_url, ...@@ -437,8 +439,9 @@ GURL SearchProviderTest::AddSearchToHistory(TemplateURL* t_url,
static base::Time last_added_time; static base::Time last_added_time;
last_added_time = std::max(base::Time::Now(), last_added_time = std::max(base::Time::Now(),
last_added_time + base::TimeDelta::FromMicroseconds(1)); last_added_time + base::TimeDelta::FromMicroseconds(1));
history->AddPageWithDetails(search, base::string16(), visit_count, visit_count, history->AddPageWithDetails(search, base::string16(), visit_count,
last_added_time, false, history::SOURCE_BROWSED); visit_count, last_added_time, false,
history::SOURCE_BROWSED);
history->SetKeywordSearchTermsForURL(search, t_url->id(), term); history->SetKeywordSearchTermsForURL(search, t_url->id(), term);
return search; return search;
} }
...@@ -548,7 +551,7 @@ TEST_F(SearchProviderTest, QueryDefaultProvider) { ...@@ -548,7 +551,7 @@ TEST_F(SearchProviderTest, QueryDefaultProvider) {
// Tell the SearchProvider the suggest query is done. // Tell the SearchProvider the suggest query is done.
fetcher->set_response_code(200); fetcher->set_response_code(200);
fetcher->delegate()->OnURLFetchComplete(fetcher); fetcher->delegate()->OnURLFetchComplete(fetcher);
fetcher = NULL; fetcher = nullptr;
// Run till the history results complete. // Run till the history results complete.
RunTillProviderDone(); RunTillProviderDone();
...@@ -643,7 +646,7 @@ TEST_F(SearchProviderTest, QueryKeywordProvider) { ...@@ -643,7 +646,7 @@ TEST_F(SearchProviderTest, QueryKeywordProvider) {
// Tell the SearchProvider the default suggest query is done. // Tell the SearchProvider the default suggest query is done.
default_fetcher->set_response_code(200); default_fetcher->set_response_code(200);
default_fetcher->delegate()->OnURLFetchComplete(default_fetcher); default_fetcher->delegate()->OnURLFetchComplete(default_fetcher);
default_fetcher = NULL; default_fetcher = nullptr;
// Make sure the keyword providers suggest service was queried. // Make sure the keyword providers suggest service was queried.
net::TestURLFetcher* keyword_fetcher = test_factory_.GetFetcherByID( net::TestURLFetcher* keyword_fetcher = test_factory_.GetFetcherByID(
...@@ -660,7 +663,7 @@ TEST_F(SearchProviderTest, QueryKeywordProvider) { ...@@ -660,7 +663,7 @@ TEST_F(SearchProviderTest, QueryKeywordProvider) {
// Tell the SearchProvider the keyword suggest query is done. // Tell the SearchProvider the keyword suggest query is done.
keyword_fetcher->set_response_code(200); keyword_fetcher->set_response_code(200);
keyword_fetcher->delegate()->OnURLFetchComplete(keyword_fetcher); keyword_fetcher->delegate()->OnURLFetchComplete(keyword_fetcher);
keyword_fetcher = NULL; keyword_fetcher = nullptr;
// Run till the history results complete. // Run till the history results complete.
RunTillProviderDone(); RunTillProviderDone();
...@@ -725,16 +728,15 @@ TEST_F(SearchProviderTest, SendDataToSuggestAtAppropriateTimes) { ...@@ -725,16 +728,15 @@ TEST_F(SearchProviderTest, SendDataToSuggestAtAppropriateTimes) {
QueryForInput(ASCIIToUTF16(cases[i].input), false, false); QueryForInput(ASCIIToUTF16(cases[i].input), false, false);
// Make sure the default provider's suggest service was or was not queried // Make sure the default provider's suggest service was or was not queried
// as appropriate. // as appropriate.
EXPECT_EQ( EXPECT_EQ(cases[i].expect_to_send_to_default_provider,
cases[i].expect_to_send_to_default_provider, test_factory_.GetFetcherByID(
test_factory_.GetFetcherByID( SearchProvider::kDefaultProviderURLFetcherID) != nullptr);
SearchProvider::kDefaultProviderURLFetcherID) != NULL);
// Send the same input with an explicitly invoked keyword. In all cases, // Send the same input with an explicitly invoked keyword. In all cases,
// it's okay to send the request to the keyword suggest server. // it's okay to send the request to the keyword suggest server.
QueryForInput(ASCIIToUTF16("k ") + ASCIIToUTF16(cases[i].input), false, QueryForInput(ASCIIToUTF16("k ") + ASCIIToUTF16(cases[i].input), false,
false); false);
EXPECT_TRUE(test_factory_.GetFetcherByID( EXPECT_TRUE(test_factory_.GetFetcherByID(
SearchProvider::kKeywordProviderURLFetcherID) != NULL); SearchProvider::kKeywordProviderURLFetcherID) != nullptr);
} }
} }
...@@ -1941,7 +1943,7 @@ TEST_F(SearchProviderTest, KeywordFetcherSuggestRelevance) { ...@@ -1941,7 +1943,7 @@ TEST_F(SearchProviderTest, KeywordFetcherSuggestRelevance) {
ASSERT_TRUE(default_fetcher); ASSERT_TRUE(default_fetcher);
default_fetcher->set_response_code(200); default_fetcher->set_response_code(200);
default_fetcher->delegate()->OnURLFetchComplete(default_fetcher); default_fetcher->delegate()->OnURLFetchComplete(default_fetcher);
default_fetcher = NULL; default_fetcher = nullptr;
// Set up a keyword fetcher with provided results. // Set up a keyword fetcher with provided results.
net::TestURLFetcher* keyword_fetcher = net::TestURLFetcher* keyword_fetcher =
...@@ -1951,7 +1953,7 @@ TEST_F(SearchProviderTest, KeywordFetcherSuggestRelevance) { ...@@ -1951,7 +1953,7 @@ TEST_F(SearchProviderTest, KeywordFetcherSuggestRelevance) {
keyword_fetcher->set_response_code(200); keyword_fetcher->set_response_code(200);
keyword_fetcher->SetResponseString(cases[i].json); keyword_fetcher->SetResponseString(cases[i].json);
keyword_fetcher->delegate()->OnURLFetchComplete(keyword_fetcher); keyword_fetcher->delegate()->OnURLFetchComplete(keyword_fetcher);
keyword_fetcher = NULL; keyword_fetcher = nullptr;
RunTillProviderDone(); RunTillProviderDone();
} }
...@@ -2551,7 +2553,7 @@ TEST_F(SearchProviderTest, SpecificTypeIdentifierParsing) { ...@@ -2551,7 +2553,7 @@ TEST_F(SearchProviderTest, SpecificTypeIdentifierParsing) {
{{"bar foo", 0}, {"bar foz", 0}}}, {{"bar foo", 0}, {"bar foz", 0}}},
// Check that ids stick to their suggestions when these are reordered // Check that ids stick to their suggestions when these are reordered
// based on suggetion relevance values. // based on suggestion relevance values.
{"e", {"e",
"[\"e\",[\"ef\",\"http://e.com\"],[],[], " "[\"e\",[\"ef\",\"http://e.com\"],[],[], "
"{\"google:suggesttype\":[\"QUERY\", \"NAVIGATION\"]," "{\"google:suggesttype\":[\"QUERY\", \"NAVIGATION\"],"
...@@ -2730,7 +2732,6 @@ TEST_F(SearchProviderTest, NavigationInline) { ...@@ -2730,7 +2732,6 @@ TEST_F(SearchProviderTest, NavigationInline) {
{ "ab", "https://abc.com/path/file.htm?q=x#foo", { "ab", "https://abc.com/path/file.htm?q=x#foo",
"https://abc.com/path/file.htm?q=x#foo", "https://abc.com/path/file.htm?q=x#foo",
"c.com/path/file.htm?q=x#foo", true, false }, "c.com/path/file.htm?q=x#foo", true, false },
}; };
for (size_t i = 0; i < arraysize(cases); ++i) { for (size_t i = 0; i < arraysize(cases); ++i) {
...@@ -3089,7 +3090,7 @@ TEST_F(SearchProviderTest, XSSIGuardedJSONParsing_InvalidResponse) { ...@@ -3089,7 +3090,7 @@ TEST_F(SearchProviderTest, XSSIGuardedJSONParsing_InvalidResponse) {
const ACMatches& matches = provider_->matches(); const ACMatches& matches = provider_->matches();
// Should have exactly one "search what you typed" match // Should have exactly one "search what you typed" match
ASSERT_TRUE(matches.size() == 1); ASSERT_EQ(1U, matches.size());
EXPECT_EQ(input_str, base::UTF16ToUTF8(matches[0].contents)); EXPECT_EQ(input_str, base::UTF16ToUTF8(matches[0].contents));
EXPECT_EQ(AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, EXPECT_EQ(AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED,
matches[0].type); matches[0].type);
...@@ -3178,101 +3179,100 @@ TEST_F(SearchProviderTest, XSSIGuardedJSONParsing_ValidResponses) { ...@@ -3178,101 +3179,100 @@ TEST_F(SearchProviderTest, XSSIGuardedJSONParsing_ValidResponses) {
// Test that deletion url gets set on an AutocompleteMatch when available for a // Test that deletion url gets set on an AutocompleteMatch when available for a
// personalized query or a personalized URL. // personalized query or a personalized URL.
TEST_F(SearchProviderTest, ParseDeletionUrl) { TEST_F(SearchProviderTest, ParseDeletionUrl) {
struct Match { struct Match {
std::string contents; std::string contents;
std::string deletion_url; std::string deletion_url;
AutocompleteMatchType::Type type; AutocompleteMatchType::Type type;
}; };
const Match kEmptyMatch = { const Match kEmptyMatch = {kNotApplicable, std::string(),
kNotApplicable, std::string(), AutocompleteMatchType::NUM_TYPES AutocompleteMatchType::NUM_TYPES};
};
const char* url[] = {
const char* url[] = { "http://defaultturl/complete/deleteitems"
"http://defaultturl/complete/deleteitems" "?delq=ab&client=chrome&deltok=xsrf124",
"?delq=ab&client=chrome&deltok=xsrf124", "http://defaultturl/complete/deleteitems"
"http://defaultturl/complete/deleteitems" "?delq=www.amazon.com&client=chrome&deltok=xsrf123",
"?delq=www.amazon.com&client=chrome&deltok=xsrf123", };
};
struct {
struct { const std::string input_text;
const std::string input_text; const std::string response_json;
const std::string response_json; const Match matches[5];
const Match matches[5]; } cases[] = {
} cases[] = { // clang-format off
// clang-format off // A deletion URL on a personalized query should be reflected in the
// A deletion URL on a personalized query should be reflected in the // resulting AutocompleteMatch.
// resulting AutocompleteMatch. { "a",
{ "a", "[\"a\",[\"ab\", \"ac\",\"www.amazon.com\"],[],[],"
"[\"a\",[\"ab\", \"ac\",\"www.amazon.com\"],[],[]," "{\"google:suggesttype\":[\"PERSONALIZED_QUERY\",\"QUERY\","
"{\"google:suggesttype\":[\"PERSONALIZED_QUERY\",\"QUERY\","
"\"PERSONALIZED_NAVIGATION\"],"
"\"google:suggestrelevance\":[3, 2, 1],"
"\"google:suggestdetail\":[{\"du\":"
"\"/complete/deleteitems?delq=ab&client=chrome"
"&deltok=xsrf124\"}, {}, {\"du\":"
"\"/complete/deleteitems?delq=www.amazon.com&"
"client=chrome&deltok=xsrf123\"}]}]",
{ { "a", "", AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED },
{ "ab", url[0], AutocompleteMatchType::SEARCH_SUGGEST },
{ "ac", "", AutocompleteMatchType::SEARCH_SUGGEST },
{ "amazon.com", url[1],
AutocompleteMatchType::NAVSUGGEST_PERSONALIZED },
kEmptyMatch,
},
},
// Personalized queries or a personalized URL without deletion URLs
// shouldn't cause errors.
{ "a",
"[\"a\",[\"ab\", \"ac\"],[],[],"
"{\"google:suggesttype\":[\"PERSONALIZED_QUERY\",\"QUERY\","
"\"PERSONALIZED_NAVIGATION\"],"
"\"google:suggestrelevance\":[1, 2],"
"\"google:suggestdetail\":[{}, {}]}]",
{ { "a", "", AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED },
{ "ac", "", AutocompleteMatchType::SEARCH_SUGGEST },
{ "ab", "", AutocompleteMatchType::SEARCH_SUGGEST },
{ "amazon.com", "",
AutocompleteMatchType::NAVSUGGEST_PERSONALIZED },
kEmptyMatch,
},
},
// Personalized queries or a personalized URL without
// google:suggestdetail shouldn't cause errors.
{ "a",
"[\"a\",[\"ab\", \"ac\"],[],[],"
"{\"google:suggesttype\":[\"PERSONALIZED_QUERY\",\"QUERY\","
"\"PERSONALIZED_NAVIGATION\"]," "\"PERSONALIZED_NAVIGATION\"],"
"\"google:suggestrelevance\":[1, 2]}]", "\"google:suggestrelevance\":[3, 2, 1],"
{ { "a", "", AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED }, "\"google:suggestdetail\":[{\"du\":"
{ "ac", "", AutocompleteMatchType::SEARCH_SUGGEST }, "\"/complete/deleteitems?delq=ab&client=chrome"
{ "ab", "", AutocompleteMatchType::SEARCH_SUGGEST }, "&deltok=xsrf124\"}, {}, {\"du\":"
{ "amazon.com", "", "\"/complete/deleteitems?delq=www.amazon.com&"
AutocompleteMatchType::NAVSUGGEST_PERSONALIZED }, "client=chrome&deltok=xsrf123\"}]}]",
kEmptyMatch, { { "a", "", AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED },
}, { "ab", url[0], AutocompleteMatchType::SEARCH_SUGGEST },
}, { "ac", "", AutocompleteMatchType::SEARCH_SUGGEST },
// clang-format on { "amazon.com", url[1],
}; AutocompleteMatchType::NAVSUGGEST_PERSONALIZED },
kEmptyMatch,
for (size_t i = 0; i < arraysize(cases); ++i) { },
QueryForInputAndWaitForFetcherResponses( },
ASCIIToUTF16(cases[i].input_text), false, cases[i].response_json, // Personalized queries or a personalized URL without deletion URLs
std::string()); // shouldn't cause errors.
{ "a",
const ACMatches& matches = provider_->matches(); "[\"a\",[\"ab\", \"ac\"],[],[],"
ASSERT_FALSE(matches.empty()); "{\"google:suggesttype\":[\"PERSONALIZED_QUERY\",\"QUERY\","
"\"PERSONALIZED_NAVIGATION\"],"
SCOPED_TRACE("for input with json = " + cases[i].response_json); "\"google:suggestrelevance\":[1, 2],"
"\"google:suggestdetail\":[{}, {}]}]",
for (size_t j = 0; j < matches.size(); ++j) { { { "a", "", AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED },
const Match& match = cases[i].matches[j]; { "ac", "", AutocompleteMatchType::SEARCH_SUGGEST },
SCOPED_TRACE(" and match index: " + base::NumberToString(j)); { "ab", "", AutocompleteMatchType::SEARCH_SUGGEST },
EXPECT_EQ(match.contents, base::UTF16ToUTF8(matches[j].contents)); { "amazon.com", "",
EXPECT_EQ(match.deletion_url, matches[j].GetAdditionalInfo( AutocompleteMatchType::NAVSUGGEST_PERSONALIZED },
"deletion_url")); kEmptyMatch,
} },
} },
// Personalized queries or a personalized URL without
// google:suggestdetail shouldn't cause errors.
{ "a",
"[\"a\",[\"ab\", \"ac\"],[],[],"
"{\"google:suggesttype\":[\"PERSONALIZED_QUERY\",\"QUERY\","
"\"PERSONALIZED_NAVIGATION\"],"
"\"google:suggestrelevance\":[1, 2]}]",
{ { "a", "", AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED },
{ "ac", "", AutocompleteMatchType::SEARCH_SUGGEST },
{ "ab", "", AutocompleteMatchType::SEARCH_SUGGEST },
{ "amazon.com", "",
AutocompleteMatchType::NAVSUGGEST_PERSONALIZED },
kEmptyMatch,
},
},
// clang-format on
};
for (size_t i = 0; i < arraysize(cases); ++i) {
QueryForInputAndWaitForFetcherResponses(ASCIIToUTF16(cases[i].input_text),
false, cases[i].response_json,
std::string());
const ACMatches& matches = provider_->matches();
ASSERT_FALSE(matches.empty());
SCOPED_TRACE("for input with json = " + cases[i].response_json);
for (size_t j = 0; j < matches.size(); ++j) {
const Match& match = cases[i].matches[j];
SCOPED_TRACE(" and match index: " + base::NumberToString(j));
EXPECT_EQ(match.contents, base::UTF16ToUTF8(matches[j].contents));
EXPECT_EQ(match.deletion_url,
matches[j].GetAdditionalInfo("deletion_url"));
}
}
} }
TEST_F(SearchProviderTest, CanSendURL) { TEST_F(SearchProviderTest, CanSendURL) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment