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

Issue 788166 link

Starred by 0 users

Issue metadata

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



Sign in to add a comment

Split up local_ntp_browsertest.cc

Project Member Reported by treib@chromium.org, Nov 23 2017

Issue description

local_ntp_browsertest.cc is almost 1500 LoC, and it contains a bunch of largely unrelated classes of tests. This makes it all pretty hard to read. We should split it up into multiple smaller files.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 23 2017

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

commit a6751476fd7cb8c3b17927f9066a99c29d268b81
Author: Marc Treib <treib@chromium.org>
Date: Thu Nov 23 16:08:01 2017

Local NTP tests cleanup: Move Doodle tests into separate file

These are independent of the other local NTP tests.
This also creates local_ntp_test_utils.h/cc which for now contains only
a single function, but more will follow.

No actual code changes here, just moving stuff around.

Bug:  788166 
Change-Id: I7336b420f82ddf0e71c40ebe2b5f7cfc4f21141f
Reviewed-on: https://chromium-review.googlesource.com/787890
Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518950}
[modify] https://crrev.com/a6751476fd7cb8c3b17927f9066a99c29d268b81/chrome/browser/ui/search/local_ntp_browsertest.cc
[add] https://crrev.com/a6751476fd7cb8c3b17927f9066a99c29d268b81/chrome/browser/ui/search/local_ntp_doodle_browsertest.cc
[add] https://crrev.com/a6751476fd7cb8c3b17927f9066a99c29d268b81/chrome/browser/ui/search/local_ntp_test_utils.cc
[add] https://crrev.com/a6751476fd7cb8c3b17927f9066a99c29d268b81/chrome/browser/ui/search/local_ntp_test_utils.h
[modify] https://crrev.com/a6751476fd7cb8c3b17927f9066a99c29d268b81/chrome/test/BUILD.gn

Project Member

Comment 2 by bugdroid1@chromium.org, Nov 23 2017

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

commit 5718a54ca055445e157a85e7d6ac8bc864821577
Author: Marc Treib <treib@chromium.org>
Date: Thu Nov 23 16:27:10 2017

Local NTP tests cleanup: Move OneGoogleBar tests into separate file

These are independent of the other local NTP tests.

No actual code changes here, just moving stuff around.

Bug:  788166 
Change-Id: I26e4d89a0f9f3a2521a1835e8142574d9071f3a6
Reviewed-on: https://chromium-review.googlesource.com/787970
Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518956}
[modify] https://crrev.com/5718a54ca055445e157a85e7d6ac8bc864821577/chrome/browser/ui/search/local_ntp_browsertest.cc
[add] https://crrev.com/5718a54ca055445e157a85e7d6ac8bc864821577/chrome/browser/ui/search/local_ntp_one_google_bar_browsertest.cc
[modify] https://crrev.com/5718a54ca055445e157a85e7d6ac8bc864821577/chrome/test/BUILD.gn

Project Member

Comment 3 by bugdroid1@chromium.org, Nov 23 2017

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

commit d57e8f1da782430416e40ecbd22517aea076a94b
Author: Marc Treib <treib@chromium.org>
Date: Thu Nov 23 17:01:51 2017

Local NTP tests cleanup: Move VoiceSearch tests into separate file

These are independent of the other local NTP tests.

No actual code changes here, just moving stuff around.

Bug:  788166 
Change-Id: I40a3f9ad6f2afdceec953684056497a73def166e
Reviewed-on: https://chromium-review.googlesource.com/787974
Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518966}
[modify] https://crrev.com/d57e8f1da782430416e40ecbd22517aea076a94b/chrome/browser/ui/search/local_ntp_browsertest.cc
[add] https://crrev.com/d57e8f1da782430416e40ecbd22517aea076a94b/chrome/browser/ui/search/local_ntp_voice_search_browsertest.cc
[modify] https://crrev.com/d57e8f1da782430416e40ecbd22517aea076a94b/chrome/test/BUILD.gn

Project Member

Comment 4 by bugdroid1@chromium.org, Nov 23 2017

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

commit a4a279ec5dd9d1c5f36a03584b31280389e28ed8
Author: Marc Treib <treib@chromium.org>
Date: Thu Nov 23 17:14:53 2017

Local NTP tests cleanup: Move utility functions to test_utils

This moves SetUserSelectedDefaultSearchProvider and SwitchToFrench
(renamed to SwitchBrowserLanguageToFrench) from local_ntp_browsertest
to local_ntp_test_utils.

Bug:  788166 
Change-Id: Iefcef3e0f56c0fdd670860c1fc4532c84c3edfc2
Reviewed-on: https://chromium-review.googlesource.com/788150
Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518968}
[modify] https://crrev.com/a4a279ec5dd9d1c5f36a03584b31280389e28ed8/chrome/browser/ui/search/local_ntp_browsertest.cc
[modify] https://crrev.com/a4a279ec5dd9d1c5f36a03584b31280389e28ed8/chrome/browser/ui/search/local_ntp_test_utils.cc
[modify] https://crrev.com/a4a279ec5dd9d1c5f36a03584b31280389e28ed8/chrome/browser/ui/search/local_ntp_test_utils.h

Comment 5 by mastiz@chromium.org, Nov 28 2017

Labels: zine-triaged
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 28 2017

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

commit 7949a0e56443813c50d068fe5066d5e55ace51aa
Author: Marc Treib <treib@chromium.org>
Date: Tue Nov 28 17:54:02 2017

Local NTP tests cleanup: Rewrite LoadsIframe to use the real local NTP

Before, it used local_ntp_browsertest.html instead, for no good reason.

Bug:  788166 
Change-Id: Iba4fe5c06eab1bc9fafebe6f528955c208c24359
Reviewed-on: https://chromium-review.googlesource.com/787897
Reviewed-by: Chris Pickel <sfiera@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519737}
[modify] https://crrev.com/7949a0e56443813c50d068fe5066d5e55ace51aa/chrome/browser/ui/search/local_ntp_browsertest.cc
[modify] https://crrev.com/7949a0e56443813c50d068fe5066d5e55ace51aa/chrome/test/data/local_ntp/local_ntp_browsertest.js

Comment 8 by treib@chromium.org, Nov 29 2017

Status: Fixed (was: Started)

Sign in to add a comment