diff options
7 files changed, 81 insertions, 39 deletions
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index 3413733..d9aa24e 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -190,6 +190,8 @@ 'renderer/mock_printer.h', 'renderer/safe_browsing/mock_feature_extractor_clock.cc', 'renderer/safe_browsing/mock_feature_extractor_clock.h', + 'renderer/safe_browsing/test_utils.cc', + 'renderer/safe_browsing/test_utils.h', 'test/automation/automation_handle_tracker.cc', 'test/automation/automation_handle_tracker.h', 'test/automation/automation_json_requests.cc', diff --git a/chrome/renderer/safe_browsing/features_unittest.cc b/chrome/renderer/safe_browsing/features_unittest.cc index e08de78..4a144a8 100644 --- a/chrome/renderer/safe_browsing/features_unittest.cc +++ b/chrome/renderer/safe_browsing/features_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -6,7 +6,7 @@ #include "base/format_macros.h" #include "base/stringprintf.h" -#include "testing/gmock/include/gmock/gmock.h" +#include "chrome/renderer/safe_browsing/test_utils.h" #include "testing/gtest/include/gtest/gtest.h" namespace safe_browsing { @@ -39,8 +39,7 @@ TEST(PhishingFeaturesTest, IllegalFeatureValue) { expected_features.AddRealFeature("zero", 0.0); expected_features.AddRealFeature("pointfive", 0.5); expected_features.AddRealFeature("one", 1.0); - EXPECT_THAT(features.features(), - ::testing::ContainerEq(expected_features.features())); + ExpectFeatureMapsAreEqual(features, expected_features); } } // namespace safe_browsing diff --git a/chrome/renderer/safe_browsing/phishing_dom_feature_extractor_browsertest.cc b/chrome/renderer/safe_browsing/phishing_dom_feature_extractor_browsertest.cc index 37f76eb..939f6ec 100644 --- a/chrome/renderer/safe_browsing/phishing_dom_feature_extractor_browsertest.cc +++ b/chrome/renderer/safe_browsing/phishing_dom_feature_extractor_browsertest.cc @@ -17,13 +17,13 @@ #include "base/time.h" #include "chrome/renderer/safe_browsing/features.h" #include "chrome/renderer/safe_browsing/mock_feature_extractor_clock.h" +#include "chrome/renderer/safe_browsing/test_utils.h" #include "content/public/test/render_view_fake_resources_test.h" #include "testing/gmock/include/gmock/gmock.h" #include "third_party/WebKit/Source/WebKit/chromium/public/WebFrame.h" #include "third_party/WebKit/Source/WebKit/chromium/public/WebScriptSource.h" #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebString.h" -using ::testing::ContainerEq; using ::testing::DoAll; using ::testing::Invoke; using ::testing::Return; @@ -92,8 +92,7 @@ class PhishingDOMFeatureExtractorTest base::WeakPtrFactory<PhishingDOMFeatureExtractorTest> weak_factory_; }; -// This test is disabled. See bug 131999 -TEST_F(PhishingDOMFeatureExtractorTest, DISABLED_FormFeatures) { +TEST_F(PhishingDOMFeatureExtractorTest, FormFeatures) { // This test doesn't exercise the extraction timing. EXPECT_CALL(clock_, Now()).WillRepeatedly(Return(base::TimeTicks::Now())); responses_["http://host.com/"] = @@ -113,7 +112,7 @@ TEST_F(PhishingDOMFeatureExtractorTest, DISABLED_FormFeatures) { FeatureMap features; LoadURL("http://host.com/"); ASSERT_TRUE(ExtractFeatures(&features)); - EXPECT_THAT(features.features(), ContainerEq(expected_features.features())); + ExpectFeatureMapsAreEqual(features, expected_features); responses_["http://host.com/"] = "<html><head><body>" @@ -126,7 +125,7 @@ TEST_F(PhishingDOMFeatureExtractorTest, DISABLED_FormFeatures) { features.Clear(); LoadURL("http://host.com/"); ASSERT_TRUE(ExtractFeatures(&features)); - EXPECT_THAT(features.features(), ContainerEq(expected_features.features())); + ExpectFeatureMapsAreEqual(features, expected_features); responses_["http://host.com/"] = "<html><head><body><input></body></html>"; @@ -137,7 +136,7 @@ TEST_F(PhishingDOMFeatureExtractorTest, DISABLED_FormFeatures) { features.Clear(); LoadURL("http://host.com/"); ASSERT_TRUE(ExtractFeatures(&features)); - EXPECT_THAT(features.features(), ContainerEq(expected_features.features())); + ExpectFeatureMapsAreEqual(features, expected_features); responses_["http://host.com/"] = "<html><head><body><input type=\"invalid\"></body></html>"; @@ -148,7 +147,7 @@ TEST_F(PhishingDOMFeatureExtractorTest, DISABLED_FormFeatures) { features.Clear(); LoadURL("http://host.com/"); ASSERT_TRUE(ExtractFeatures(&features)); - EXPECT_THAT(features.features(), ContainerEq(expected_features.features())); + ExpectFeatureMapsAreEqual(features, expected_features); } TEST_F(PhishingDOMFeatureExtractorTest, LinkFeatures) { @@ -170,7 +169,7 @@ TEST_F(PhishingDOMFeatureExtractorTest, LinkFeatures) { FeatureMap features; LoadURL("http://www.host.com/"); ASSERT_TRUE(ExtractFeatures(&features)); - EXPECT_THAT(features.features(), ContainerEq(expected_features.features())); + ExpectFeatureMapsAreEqual(features, expected_features); responses_.clear(); responses_["https://www.host.com/"] = @@ -190,11 +189,10 @@ TEST_F(PhishingDOMFeatureExtractorTest, LinkFeatures) { features.Clear(); LoadURL("https://www.host.com/"); ASSERT_TRUE(ExtractFeatures(&features)); - EXPECT_THAT(features.features(), ContainerEq(expected_features.features())); + ExpectFeatureMapsAreEqual(features, expected_features); } -// This test is disabled. See bug 131999 -TEST_F(PhishingDOMFeatureExtractorTest, DISABLED_ScriptAndImageFeatures) { +TEST_F(PhishingDOMFeatureExtractorTest, ScriptAndImageFeatures) { // This test doesn't exercise the extraction timing. EXPECT_CALL(clock_, Now()).WillRepeatedly(Return(base::TimeTicks::Now())); responses_["http://host.com/"] = @@ -206,7 +204,7 @@ TEST_F(PhishingDOMFeatureExtractorTest, DISABLED_ScriptAndImageFeatures) { FeatureMap features; LoadURL("http://host.com/"); ASSERT_TRUE(ExtractFeatures(&features)); - EXPECT_THAT(features.features(), ContainerEq(expected_features.features())); + ExpectFeatureMapsAreEqual(features, expected_features); responses_["http://host.com/"] = "<html><head><script></script><script></script><script></script>" @@ -222,11 +220,10 @@ TEST_F(PhishingDOMFeatureExtractorTest, DISABLED_ScriptAndImageFeatures) { features.Clear(); LoadURL("http://host.com/"); ASSERT_TRUE(ExtractFeatures(&features)); - EXPECT_THAT(features.features(), ContainerEq(expected_features.features())); + ExpectFeatureMapsAreEqual(features, expected_features); } -// This test is disabled. See bug 131999 -TEST_F(PhishingDOMFeatureExtractorTest, DISABLED_SubFrames) { +TEST_F(PhishingDOMFeatureExtractorTest, SubFrames) { // This test doesn't exercise the extraction timing. EXPECT_CALL(clock_, Now()).WillRepeatedly(Return(base::TimeTicks::Now())); @@ -275,7 +272,7 @@ TEST_F(PhishingDOMFeatureExtractorTest, DISABLED_SubFrames) { FeatureMap features; LoadURL("http://host.com/"); ASSERT_TRUE(ExtractFeatures(&features)); - EXPECT_THAT(features.features(), ContainerEq(expected_features.features())); + ExpectFeatureMapsAreEqual(features, expected_features); } TEST_F(PhishingDOMFeatureExtractorTest, Continuation) { @@ -333,7 +330,7 @@ TEST_F(PhishingDOMFeatureExtractorTest, Continuation) { FeatureMap features; LoadURL("http://host.com/"); ASSERT_TRUE(ExtractFeatures(&features)); - EXPECT_THAT(features.features(), ContainerEq(expected_features.features())); + ExpectFeatureMapsAreEqual(features, expected_features); // Make sure none of the mock expectations carry over to the next test. ::testing::Mock::VerifyAndClearExpectations(&clock_); @@ -400,7 +397,7 @@ TEST_F(PhishingDOMFeatureExtractorTest, SubframeRemoval) { FeatureMap features; LoadURL("http://host.com/"); ASSERT_TRUE(ExtractFeatures(&features)); - EXPECT_THAT(features.features(), ContainerEq(expected_features.features())); + ExpectFeatureMapsAreEqual(features, expected_features); } } // namespace safe_browsing diff --git a/chrome/renderer/safe_browsing/phishing_term_feature_extractor_unittest.cc b/chrome/renderer/safe_browsing/phishing_term_feature_extractor_unittest.cc index 61262a65..3e12035 100644 --- a/chrome/renderer/safe_browsing/phishing_term_feature_extractor_unittest.cc +++ b/chrome/renderer/safe_browsing/phishing_term_feature_extractor_unittest.cc @@ -19,10 +19,10 @@ #include "chrome/renderer/safe_browsing/features.h" #include "chrome/renderer/safe_browsing/mock_feature_extractor_clock.h" #include "chrome/renderer/safe_browsing/murmurhash3_util.h" +#include "chrome/renderer/safe_browsing/test_utils.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" -using ::testing::ContainerEq; using ::testing::Return; namespace safe_browsing { @@ -130,7 +130,7 @@ TEST_F(PhishingTermFeatureExtractorTest, ExtractFeatures) { FeatureMap features; ASSERT_TRUE(ExtractFeatures(&page_text, &features)); - EXPECT_THAT(features.features(), ContainerEq(expected_features.features())); + ExpectFeatureMapsAreEqual(features, expected_features); page_text = ASCIIToUTF16("one one"); expected_features.Clear(); @@ -141,7 +141,7 @@ TEST_F(PhishingTermFeatureExtractorTest, ExtractFeatures) { features.Clear(); ASSERT_TRUE(ExtractFeatures(&page_text, &features)); - EXPECT_THAT(features.features(), ContainerEq(expected_features.features())); + ExpectFeatureMapsAreEqual(features, expected_features); page_text = ASCIIToUTF16("bla bla multi word test bla"); expected_features.Clear(); @@ -150,7 +150,7 @@ TEST_F(PhishingTermFeatureExtractorTest, ExtractFeatures) { features.Clear(); ASSERT_TRUE(ExtractFeatures(&page_text, &features)); - EXPECT_THAT(features.features(), ContainerEq(expected_features.features())); + ExpectFeatureMapsAreEqual(features, expected_features); // This text has all of the words for one of the terms, but they are // not in the correct order. @@ -159,7 +159,7 @@ TEST_F(PhishingTermFeatureExtractorTest, ExtractFeatures) { features.Clear(); ASSERT_TRUE(ExtractFeatures(&page_text, &features)); - EXPECT_THAT(features.features(), ContainerEq(expected_features.features())); + ExpectFeatureMapsAreEqual(features, expected_features); page_text = ASCIIToUTF16("Capitalization plus non-space\n" "separator... punctuation!"); @@ -175,14 +175,14 @@ TEST_F(PhishingTermFeatureExtractorTest, ExtractFeatures) { features.Clear(); ASSERT_TRUE(ExtractFeatures(&page_text, &features)); - EXPECT_THAT(features.features(), ContainerEq(expected_features.features())); + ExpectFeatureMapsAreEqual(features, expected_features); // Test with empty page text. page_text = string16(); expected_features.Clear(); features.Clear(); ASSERT_TRUE(ExtractFeatures(&page_text, &features)); - EXPECT_THAT(features.features(), ContainerEq(expected_features.features())); + ExpectFeatureMapsAreEqual(features, expected_features); // Chinese translation of the phrase "hello goodbye". This tests that // we can correctly separate terms in languages that don't use spaces. @@ -195,7 +195,7 @@ TEST_F(PhishingTermFeatureExtractorTest, ExtractFeatures) { features.Clear(); ASSERT_TRUE(ExtractFeatures(&page_text, &features)); - EXPECT_THAT(features.features(), ContainerEq(expected_features.features())); + ExpectFeatureMapsAreEqual(features, expected_features); } TEST_F(PhishingTermFeatureExtractorTest, Continuation) { @@ -245,7 +245,7 @@ TEST_F(PhishingTermFeatureExtractorTest, Continuation) { FeatureMap features; ASSERT_TRUE(ExtractFeatures(&page_text, &features)); - EXPECT_THAT(features.features(), ContainerEq(expected_features.features())); + ExpectFeatureMapsAreEqual(features, expected_features); // Make sure none of the mock expectations carry over to the next test. ::testing::Mock::VerifyAndClearExpectations(&clock_); @@ -309,7 +309,7 @@ TEST_F(PhishingTermFeatureExtractorTest, PartialExtractionTest) { FeatureMap expected_features; expected_features.AddBooleanFeature(features::kPageTerm + std::string("multi word test")); - EXPECT_THAT(features.features(), ContainerEq(expected_features.features())); + ExpectFeatureMapsAreEqual(features, expected_features); } } // namespace safe_browsing diff --git a/chrome/renderer/safe_browsing/phishing_url_feature_extractor_unittest.cc b/chrome/renderer/safe_browsing/phishing_url_feature_extractor_unittest.cc index a07f3ca..e371a47 100644 --- a/chrome/renderer/safe_browsing/phishing_url_feature_extractor_unittest.cc +++ b/chrome/renderer/safe_browsing/phishing_url_feature_extractor_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -7,11 +7,11 @@ #include <string> #include <vector> #include "chrome/renderer/safe_browsing/features.h" +#include "chrome/renderer/safe_browsing/test_utils.h" #include "googleurl/src/gurl.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" -using ::testing::ContainerEq; using ::testing::ElementsAre; namespace safe_browsing { @@ -40,7 +40,7 @@ TEST_F(PhishingUrlFeatureExtractorTest, ExtractFeatures) { FeatureMap features; ASSERT_TRUE(extractor_.ExtractFeatures(GURL(url), &features)); - EXPECT_THAT(features.features(), ContainerEq(expected_features.features())); + ExpectFeatureMapsAreEqual(features, expected_features); url = "http://www.www.cnn.co.uk/sports/sports/index.html?shouldnotappear"; expected_features.Clear(); @@ -60,7 +60,7 @@ TEST_F(PhishingUrlFeatureExtractorTest, ExtractFeatures) { features.Clear(); ASSERT_TRUE(extractor_.ExtractFeatures(GURL(url), &features)); - EXPECT_THAT(features.features(), ContainerEq(expected_features.features())); + ExpectFeatureMapsAreEqual(features, expected_features); url = "http://justadomain.com/"; expected_features.Clear(); @@ -71,7 +71,7 @@ TEST_F(PhishingUrlFeatureExtractorTest, ExtractFeatures) { features.Clear(); ASSERT_TRUE(extractor_.ExtractFeatures(GURL(url), &features)); - EXPECT_THAT(features.features(), ContainerEq(expected_features.features())); + ExpectFeatureMapsAreEqual(features, expected_features); url = "http://witharef.com/#abc"; expected_features.Clear(); @@ -82,7 +82,7 @@ TEST_F(PhishingUrlFeatureExtractorTest, ExtractFeatures) { features.Clear(); ASSERT_TRUE(extractor_.ExtractFeatures(GURL(url), &features)); - EXPECT_THAT(features.features(), ContainerEq(expected_features.features())); + ExpectFeatureMapsAreEqual(features, expected_features); url = "http://...www..lotsodots....com./"; expected_features.Clear(); @@ -95,7 +95,7 @@ TEST_F(PhishingUrlFeatureExtractorTest, ExtractFeatures) { features.Clear(); ASSERT_TRUE(extractor_.ExtractFeatures(GURL(url), &features)); - EXPECT_THAT(features.features(), ContainerEq(expected_features.features())); + ExpectFeatureMapsAreEqual(features, expected_features); url = "http://unrecognized.tld/"; EXPECT_FALSE(extractor_.ExtractFeatures(GURL(url), &features)); diff --git a/chrome/renderer/safe_browsing/test_utils.cc b/chrome/renderer/safe_browsing/test_utils.cc new file mode 100644 index 0000000..b3d8f9b --- /dev/null +++ b/chrome/renderer/safe_browsing/test_utils.cc @@ -0,0 +1,24 @@ +// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/renderer/safe_browsing/test_utils.h" + +#include <map> +#include <string> + +#include "chrome/renderer/safe_browsing/features.h" +#include "testing/gmock/include/gmock/gmock.h" + +namespace safe_browsing { + +void ExpectFeatureMapsAreEqual(const FeatureMap& first, + const FeatureMap& second) { + std::map<std::string, double> sorted_first(first.features().begin(), + first.features().end()); + std::map<std::string, double> sorted_second(second.features().begin(), + second.features().end()); + EXPECT_THAT(sorted_first, testing::ContainerEq(sorted_second)); +} + +} // namespace safe_browsing diff --git a/chrome/renderer/safe_browsing/test_utils.h b/chrome/renderer/safe_browsing/test_utils.h new file mode 100644 index 0000000..f719ec4 --- /dev/null +++ b/chrome/renderer/safe_browsing/test_utils.h @@ -0,0 +1,20 @@ +// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_RENDERER_SAFE_BROWSING_TEST_UTILS_H_ +#define CHROME_RENDERER_SAFE_BROWSING_TEST_UTILS_H_ +#pragma once + +namespace safe_browsing { +class FeatureMap; + +// Compares two FeatureMap objects using gMock. Always use this instead of +// operator== or ContainerEq, since hash_map's equality operator may return +// false if the elements were inserted in different orders. +void ExpectFeatureMapsAreEqual(const FeatureMap& first, + const FeatureMap& second); + +} // namespace safe_browsing + +#endif // CHROME_RENDERER_SAFE_BROWSING_TEST_UTILS_H_ |