New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 767406 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 3
Type: Task



Sign in to add a comment

Clean chrome/browser/ui/search/ntp_user_data_logger_unittest.cc

Project Member Reported by fhorschig@chromium.org, Sep 21 2017

Issue description

The tests in chrome/browser/ui/search/ntp_user_data_logger_unittest.cc are very cluttered and tests like TestLogMostVisitedImpression cover more than one workflow.
It's error-prone and hard to follow in reviews if incremental changes are made.
 

Comment 1 by treib@chromium.org, Sep 21 2017

Labels: OS-Chrome
Owner: treib@chromium.org
Status: Assigned (was: Available)

Comment 2 by treib@chromium.org, Oct 13 2017

Status: Started (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d1a3364efe3dabfaf2158eb631ae05bbf0982145

commit d1a3364efe3dabfaf2158eb631ae05bbf0982145
Author: Marc Treib <treib@chromium.org>
Date: Fri Oct 13 16:38:58 2017

Cleanup NTPUserDataLoggerTest, part 1

This CL makes the expectations for TestMostVisitedNavigation and
TestLogMostVisitedImpression easier to read, by splitting them into
"sections" with separate HistogramTester instances.

Bug:  767406 
Change-Id: I7776ebe56d5be86ce4a52f630a22a8d5a8ca3585
Reviewed-on: https://chromium-review.googlesource.com/718738
Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508721}
[modify] https://crrev.com/d1a3364efe3dabfaf2158eb631ae05bbf0982145/chrome/browser/ui/search/ntp_user_data_logger_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2a83238d8b2a8ab211c1b837feb2ff12b7096678

commit 2a83238d8b2a8ab211c1b837feb2ff12b7096678
Author: Marc Treib <treib@chromium.org>
Date: Mon Oct 16 14:10:48 2017

Cleanup NTPUserDataLoggerTest, part 2

This CL splits TestLogMostVisitedImpression into smaller tests. It also
renames all tests to follow the ShouldDoTheThing naming convention.

Bug:  767406 
Change-Id: I82dc2726e236c2244f48daebd2aea018956f367a
Reviewed-on: https://chromium-review.googlesource.com/720916
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509033}
[modify] https://crrev.com/2a83238d8b2a8ab211c1b837feb2ff12b7096678/chrome/browser/ui/search/ntp_user_data_logger.h
[modify] https://crrev.com/2a83238d8b2a8ab211c1b837feb2ff12b7096678/chrome/browser/ui/search/ntp_user_data_logger_unittest.cc

Comment 5 by treib@chromium.org, Oct 16 2017

Status: Fixed (was: Started)

Sign in to add a comment