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

Issue 659631 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug

Blocked on:
issue 666696



Sign in to add a comment

Improve test coverage of ntp

Project Member Reported by mvanouwe...@chromium.org, Oct 26 2016

Issue description

Our current test coverage of Java code across junit and instrumentation tests is 51% but we can do better than that.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 26 2016

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

commit 2f9c3eb5b0abb57af96273082500434ba9f23626
Author: mvanouwerkerk <mvanouwerkerk@chromium.org>
Date: Wed Oct 26 13:54:46 2016

Ntp: add test for setSearchProviderHasLogo using old ui.

* Add testSetSearchProviderHasLogo, this is our only test for the old (non-snippets) ui
* Move private helpers to bottom of file
* Move setUp, tearDown, and startMainActivity to top of file

BUG= 659631 

Review-Url: https://codereview.chromium.org/2456503002
Cr-Commit-Position: refs/heads/master@{#427679}

[modify] https://crrev.com/2f9c3eb5b0abb57af96273082500434ba9f23626/chrome/android/javatests/src/org/chromium/chrome/browser/ntp/NewTabPageTest.java

Comment 2 by fi...@chromium.org, Oct 27 2016

Labels: M-57 zine-client-ux-v1 zine-triaged
Labels: zine-16-11-07
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 16 2016

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

commit 59a04c33c644823c1ded38f1b9f274c6d969db85
Author: mvanouwerkerk <mvanouwerkerk@chromium.org>
Date: Wed Nov 16 16:57:32 2016

Add NewTabPageRecyclerViewTest.

* Adds test for clicking a suggestion, dismissing all, and dismissing with context menu.
* Bumps coverage of org.chromium.chrome.browser.ntp* from 53% to 57%.

BUG= 659631 

Review-Url: https://codereview.chromium.org/2504113003
Cr-Commit-Position: refs/heads/master@{#432523}

[modify] https://crrev.com/59a04c33c644823c1ded38f1b9f274c6d969db85/chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuHandler.java
[modify] https://crrev.com/59a04c33c644823c1ded38f1b9f274c6d969db85/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java
[modify] https://crrev.com/59a04c33c644823c1ded38f1b9f274c6d969db85/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java
[modify] https://crrev.com/59a04c33c644823c1ded38f1b9f274c6d969db85/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java
[modify] https://crrev.com/59a04c33c644823c1ded38f1b9f274c6d969db85/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/FakeSuggestionsSource.java
[modify] https://crrev.com/59a04c33c644823c1ded38f1b9f274c6d969db85/chrome/android/java_sources.gni
[modify] https://crrev.com/59a04c33c644823c1ded38f1b9f274c6d969db85/chrome/android/javatests/src/org/chromium/chrome/browser/ntp/NewTabPageTest.java
[add] https://crrev.com/59a04c33c644823c1ded38f1b9f274c6d969db85/chrome/android/javatests/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerViewTest.java

Blockedon: 666696
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 4 2017

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

commit 04b297ee5b0a8061c42fd23ffe04148b3d27ce76
Author: mvanouwerkerk <mvanouwerkerk@chromium.org>
Date: Wed Jan 04 16:26:45 2017

Delete dead code for CurrentlyOpenTabs

RecentTabsManager#getCurrentlyOpenTabs() always returns null,
and from that point on all this other code seems to be unused.

BUG= 659631 

Review-Url: https://codereview.chromium.org/2604453004
Cr-Commit-Position: refs/heads/master@{#441387}

[delete] https://crrev.com/e5e86244987fa95cb6784aeb06d2004bb44e5f3f/chrome/android/java/src/org/chromium/chrome/browser/ntp/CurrentlyOpenTab.java
[modify] https://crrev.com/04b297ee5b0a8061c42fd23ffe04148b3d27ce76/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPagePrefs.java
[modify] https://crrev.com/04b297ee5b0a8061c42fd23ffe04148b3d27ce76/chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentTabsGroupView.java
[modify] https://crrev.com/04b297ee5b0a8061c42fd23ffe04148b3d27ce76/chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentTabsManager.java
[modify] https://crrev.com/04b297ee5b0a8061c42fd23ffe04148b3d27ce76/chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentTabsRowAdapter.java
[modify] https://crrev.com/04b297ee5b0a8061c42fd23ffe04148b3d27ce76/chrome/android/java/strings/android_chrome_strings.grd
[modify] https://crrev.com/04b297ee5b0a8061c42fd23ffe04148b3d27ce76/chrome/android/java_sources.gni
[modify] https://crrev.com/04b297ee5b0a8061c42fd23ffe04148b3d27ce76/chrome/browser/android/ntp/new_tab_page_prefs.cc
[modify] https://crrev.com/04b297ee5b0a8061c42fd23ffe04148b3d27ce76/chrome/browser/android/ntp/new_tab_page_prefs.h
[modify] https://crrev.com/04b297ee5b0a8061c42fd23ffe04148b3d27ce76/chrome/common/pref_names.cc
[modify] https://crrev.com/04b297ee5b0a8061c42fd23ffe04148b3d27ce76/chrome/common/pref_names.h

Project Member

Comment 7 by bugdroid1@chromium.org, Jan 6 2017

Project Member

Comment 8 by bugdroid1@chromium.org, Jan 6 2017

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

commit 7e0ae49ce66d97e8fb353d55563b0ed040e49416
Author: mvanouwerkerk <mvanouwerkerk@chromium.org>
Date: Fri Jan 06 15:03:27 2017

Add RecentTabsPageTest.

* Adds the first test for RecentTabsPage.
* Extracts RecentlyClosedTabManager interface from RecentlyClosedBridge to enable injection of a fake in tests.
* Deletes a bunch of one-line helper functions.
* Minor cleanups like using diamond operators and restricting visibility.

BUG= 659631 

Review-Url: https://codereview.chromium.org/2610143002
Cr-Commit-Position: refs/heads/master@{#441935}

[add] https://crrev.com/7e0ae49ce66d97e8fb353d55563b0ed040e49416/chrome/android/java/src/org/chromium/chrome/browser/ntp/FakeRecentlyClosedTabManager.java
[modify] https://crrev.com/7e0ae49ce66d97e8fb353d55563b0ed040e49416/chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentTabsManager.java
[modify] https://crrev.com/7e0ae49ce66d97e8fb353d55563b0ed040e49416/chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentTabsRowAdapter.java
[modify] https://crrev.com/7e0ae49ce66d97e8fb353d55563b0ed040e49416/chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentlyClosedBridge.java
[add] https://crrev.com/7e0ae49ce66d97e8fb353d55563b0ed040e49416/chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentlyClosedTab.java
[add] https://crrev.com/7e0ae49ce66d97e8fb353d55563b0ed040e49416/chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentlyClosedTabManager.java
[modify] https://crrev.com/7e0ae49ce66d97e8fb353d55563b0ed040e49416/chrome/android/java_sources.gni
[add] https://crrev.com/7e0ae49ce66d97e8fb353d55563b0ed040e49416/chrome/android/javatests/src/org/chromium/chrome/browser/ntp/RecentTabsPageTest.java
[modify] https://crrev.com/7e0ae49ce66d97e8fb353d55563b0ed040e49416/chrome/browser/android/recently_closed_tabs_bridge.cc
[modify] https://crrev.com/7e0ae49ce66d97e8fb353d55563b0ed040e49416/chrome/browser/android/recently_closed_tabs_bridge.h

Project Member

Comment 9 by bugdroid1@chromium.org, Jan 6 2017

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

commit 8739cd56aa328f7972c60cba5f0d3348a108c273
Author: mvanouwerkerk <mvanouwerkerk@chromium.org>
Date: Fri Jan 06 16:16:33 2017

RecentTabsPage code cleanup.

* Delete some one-line helper functions.
* Provide some more default method implementations in class Group.

BUG= 659631 

Review-Url: https://codereview.chromium.org/2616953002
Cr-Commit-Position: refs/heads/master@{#441946}

[modify] https://crrev.com/8739cd56aa328f7972c60cba5f0d3348a108c273/chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentTabsPage.java
[modify] https://crrev.com/8739cd56aa328f7972c60cba5f0d3348a108c273/chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentTabsRowAdapter.java

Status: Fixed (was: Started)
The overall by-line coverage is now 68%.

By package:
ntp 62%
ntp.snippets 73%
ntp.cards 79%

While we could go higher, we'll hit diminishing returns, so I'm calling this done for now.

Sign in to add a comment