Commit 321b701a authored by Wei-Yin Chen (陳威尹)'s avatar Wei-Yin Chen (陳威尹) Committed by Commit Bot

Fix image data matching in DistillerTest.DistillMultiplePages

In DistillerTest.DistillMultiplePages, the images were added in an
incorrect way, so that there were actually no images in the distilled
results. Therefore, the image data matching in function
VerifyArticleProtoMatchesMultipageData() were never executed.

This was discovered when working on https://crbug.com/864015, and the
lack of coverage was identified here:
https://chromium-coverage.appspot.com/reports/590937/linux/chromium/src/components/dom_distiller/core/distiller_unittest.cc.html

This CL fixes this by constructing the expected distillation
correctly in CreateMultipageDistillerDataWithImages().

Bug: None
Change-Id: Iaf6a1fc81ac956f362ee584d691b066dcde2dab7
Reviewed-on: https://chromium-review.googlesource.com/1226517Reviewed-by: default avatarMatthew Jones <mdjones@chromium.org>
Commit-Queue: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591426}
parent 9625f0fb
......@@ -51,7 +51,9 @@ namespace {
const char kTitle[] = "Title";
const char kContent[] = "Content";
const char kURL[] = "http://a.com/";
const size_t kTotalGoodImages = 2;
const size_t kTotalImages = 3;
// Good images need to be in the front.
const char* kImageURLs[kTotalImages] = {"http://a.com/img1.jpg",
"http://a.com/img2.jpg",
"./bad_url_should_fail"};
......@@ -162,28 +164,35 @@ string GeneratePrevPageUrl(const std::string& url_prefix, size_t page_num) {
return page_num > 0 ? url_prefix + base::NumberToString(page_num - 1) : "";
}
std::unique_ptr<MultipageDistillerData>
CreateMultipageDistillerDataWithoutImages(size_t pages_size) {
std::unique_ptr<MultipageDistillerData> CreateMultipageDistillerDataWithImages(
const vector<vector<int>>& image_ids) {
size_t pages_size = image_ids.size();
std::unique_ptr<MultipageDistillerData> result(new MultipageDistillerData());
string url_prefix = kURL;
result->image_ids = image_ids;
for (size_t page_num = 0; page_num < pages_size; ++page_num) {
result->page_urls.push_back(url_prefix + base::NumberToString(page_num));
result->content.push_back("Content for page:" +
base::NumberToString(page_num));
result->image_ids.push_back(vector<int>());
string next_page_url =
GenerateNextPageUrl(url_prefix, page_num, pages_size);
string prev_page_url =
GeneratePrevPageUrl(url_prefix, page_num);
std::unique_ptr<base::Value> distilled_value =
CreateDistilledValueReturnedFromJS(kTitle, result->content[page_num],
result->image_ids[page_num],
next_page_url, prev_page_url);
image_ids[page_num], next_page_url,
prev_page_url);
result->distilled_values.push_back(std::move(distilled_value));
}
return result;
}
std::unique_ptr<MultipageDistillerData>
CreateMultipageDistillerDataWithoutImages(size_t pages_size) {
return CreateMultipageDistillerDataWithImages(
vector<vector<int>>(pages_size));
}
void VerifyArticleProtoMatchesMultipageData(
const dom_distiller::DistilledArticleProto* article_proto,
const MultipageDistillerData* distiller_data,
......@@ -442,10 +451,9 @@ TEST_F(DistillerTest, DistillPageWithImages) {
TEST_F(DistillerTest, DistillMultiplePages) {
base::MessageLoopForUI loop;
const size_t kNumPages = 8;
std::unique_ptr<MultipageDistillerData> distiller_data =
CreateMultipageDistillerDataWithoutImages(kNumPages);
// Add images.
vector<vector<int>> image_ids;
int next_image_number = 0;
for (size_t page_num = 0; page_num < kNumPages; ++page_num) {
// Each page has different number of images.
......@@ -453,11 +461,14 @@ TEST_F(DistillerTest, DistillMultiplePages) {
vector<int> image_indices;
for (size_t img_num = 0; img_num < tot_images; img_num++) {
image_indices.push_back(next_image_number);
next_image_number = (next_image_number + 1) % kTotalImages;
next_image_number = (next_image_number + 1) % kTotalGoodImages;
}
distiller_data->image_ids.push_back(image_indices);
image_ids.push_back(image_indices);
}
std::unique_ptr<MultipageDistillerData> distiller_data =
CreateMultipageDistillerDataWithImages(image_ids);
distiller_.reset(
new DistillerImpl(url_fetcher_factory_, DomDistillerOptions()));
DistillPage(distiller_data->page_urls[0],
......
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