Remove "<HR>" tags to import Firefox bookmarks correctly.

chrome/utility/importer/bookmark_html_reader_unittest.cc

 // Copyright 2013 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/utility/importer/bookmark_html_reader.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/files/file_path.h"
@@ skipping 161 matching lines @@
 }
 
 void ExpectSecondEpiphanyBookmark(const ImportedBookmarkEntry& entry) {
   EXPECT_EQ(ASCIIToUTF16("Google"), entry.title);
   EXPECT_EQ("http://www.google.com/", entry.url.spec());
   EXPECT_EQ(0U, entry.path.size());
 }
 
 }  // namespace
 
 TEST(BookmarkHTMLReaderTest, Firefox2BookmarkFileImport) {

[gab, 2013/08/09 13:27:30]

Make all the BookmarkHTMLReaderTest here and below test fixtures, e.g. something
like:

class BookmarkHTMLReaderTestWithData : public ::testing::Test {
 public:
  virtual void SetUp() OVERRIDE {
    ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &test_data_path_));
    test_data_path_ = test_data_path_.AppendASCII("bookmark_html_reader");
  }

 protected:
  base::FilePath test_data_path_;
};

And then change all tests here and below from 
TEST(BookmarkHTMLReaderTest, ...)
to
TEST_F(BookmarkHTMLReaderTestWithData,  ...)

Instances of |path| should be replaced with |test_data_path_| in every test.

This avoids repeating the initialization code that all of these tests have in
common.

Furthermore, all of the methods in the unnamed namespace above can then move as
protected methods of the test fixture.


(note that BookmarkHTMLReaderTest.ParseTests can remain a TEST with no fixture
since it requires no initialization)

[zhchbin, 2013/08/09 14:20:00]

I will finish it as soon as I can. Thanks for the instruction.

   base::FilePath path;
   ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &path));
   path = path.AppendASCII("bookmark_html_reader");
   path = path.AppendASCII("firefox2.html");
 
   std::vector<ImportedBookmarkEntry> bookmarks;
   ImportBookmarksFile(base::Callback<bool(void)>(),
                       base::Callback<bool(const GURL&)>(),
                       path, &bookmarks, NULL);
 
   ASSERT_EQ(3U, bookmarks.size());
   ExpectFirstFirefox2Bookmark(bookmarks[0]);
   ExpectSecondFirefox2Bookmark(bookmarks[1]);
   ExpectThirdFirefox2Bookmark(bookmarks[2]);
 }
 
+TEST(BookmarkHTMLReaderTest, BookmarkFileWithHrTagImport) {
+  base::FilePath path;
+  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &path));
+  path = path.AppendASCII("bookmark_html_reader");
+  path = path.AppendASCII("firefox3.html");
+
+  std::vector<ImportedBookmarkEntry> bookmarks;
+  ImportBookmarksFile(base::Callback<bool(void)>(),
+                      base::Callback<bool(const GURL&)>(),
+                      path, &bookmarks, NULL);
+
+  ASSERT_EQ(7U, bookmarks.size());

[gab, 2013/08/09 13:27:30]

Can you also verify the bookmarks' contents like the other tests do.

[zhchbin, 2013/08/09 14:20:00]

I mean to keep this test case simple and only testing the size seems already
enough for this case. However, if we need it I will add.

[gab, 2013/08/09 14:29:03]

Well, from knowing the implementation, we know that this only makes sure it
works with the <HR> tags, but this test doesn't need to know that and should
check that the full bookmarks were imported as expected.

Feel free to export a smaller test file than 7 bookmarks to make for the
smallest test possible that has full coverage.

+}
+
 TEST(BookmarkHTMLReaderTest, EpiphanyBookmarkFileImport) {
   base::FilePath path;
   ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &path));
   path = path.AppendASCII("bookmark_html_reader");
   path = path.AppendASCII("epiphany.html");
 
   std::vector<ImportedBookmarkEntry> bookmarks;
   ImportBookmarksFile(base::Callback<bool(void)>(),
                       base::Callback<bool(const GURL&)>(),
                       path, &bookmarks, NULL);
@@ skipping 57 matching lines @@
   ImportBookmarksFile(base::Callback<bool(void)>(),
                       base::Bind(&IsURLValid),
                       path, &bookmarks, NULL);
 
   ASSERT_EQ(2U, bookmarks.size());
   ExpectFirstFirefox2Bookmark(bookmarks[0]);
   ExpectThirdFirefox2Bookmark(bookmarks[1]);
 }
 
 }  // namespace bookmark_html_reader