Add a unit test for the BookmarkSuggestionsProvider.

Add a unit test for the BookmarkSuggestionsProvider.

This is primarily to verify behavior fixed in https://codereview.chromium.org/2616633002/ but also adds tests for more fundamental behavior of the BookmarkSuggestionsProvider.

This CL also moves ContentSuggestionsProvider test doubles out of specific unit tests and into separate targets so that we can reuse them.

BUG=674178
Review-Url: https://codereview.chromium.org/2618243004
Cr-Commit-Position: refs/heads/master@{#443260}
Committed: https://chromium.googlesource.com/chromium/src/+/25e7324ca818226f5f68aca6eae4348ae739b247

[tschumann, 2017-01-10 16:20:40]

Description was changed from

==========
Add a unit test for the BookmarkSuggestionsProvider.

This CL also moves ContentSuggestionsProvider test doubles out of specific unit
tests and into separate targets so that we can reuse them.

BUG=
==========

to

==========
Add a unit test for the BookmarkSuggestionsProvider.

This is primarily to verify behavior fixed in
https://codereview.chromium.org/2616633002/ but also adds tests for more
fundamental behavior of the BookmarkSuggestionsProvider.

This CL also moves ContentSuggestionsProvider test doubles out of specific unit
tests and into separate targets so that we can reuse them.

BUG=674178
==========

[tschumann, 2017-01-10 16:20:40]

tschumann@chromium.org changed reviewers:
+ jkrcal@chromium.org

[tschumann, 2017-01-10 16:20:55]

The CQ bit was checked by tschumann@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-10 16:21:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-10 16:25:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-10 16:25:05]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[tschumann, 2017-01-10 16:39:20]

The CQ bit was checked by tschumann@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-10 16:39:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-10 17:47:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-10 17:47:37]

Dry run: This issue passed the CQ dry run.

[jkrcal, 2017-01-12 14:50:54]

lgtm, thanks!

https://codereview.chromium.org/2618243004/diff/40001/components/ntp_snippets...
File components/ntp_snippets/bookmarks/bookmark_suggestions_provider_unittest.cc
(right):

https://codereview.chromium.org/2618243004/diff/40001/components/ntp_snippets...
components/ntp_snippets/bookmarks/bookmark_suggestions_provider_unittest.cc:120:
// TODO(tschumann): This seems lik a bug. When marking a bookmark as
This WAI. Suggestions should not be updated after dismissing a bookmark.

See the comment in
https://cs.chromium.org/chromium/src/components/ntp_snippets/content_suggesti...

If you still think this is a bug (because we should update other open NTPs), put
a TODO at content_suggestions_provider.h:83

https://codereview.chromium.org/2618243004/diff/40001/components/ntp_snippets...
File components/ntp_snippets/mock_content_suggestions_provider.h (right):

https://codereview.chromium.org/2618243004/diff/40001/components/ntp_snippets...
components/ntp_snippets/mock_content_suggestions_provider.h:20: class
MockProvider : public ContentSuggestionsProvider {
nit: MockContentSuggestionsProvider to keep naming consistent with the file
name?

[tschumann, 2017-01-12 15:06:23]

https://codereview.chromium.org/2618243004/diff/40001/components/ntp_snippets...
File components/ntp_snippets/bookmarks/bookmark_suggestions_provider_unittest.cc
(right):

https://codereview.chromium.org/2618243004/diff/40001/components/ntp_snippets...
components/ntp_snippets/bookmarks/bookmark_suggestions_provider_unittest.cc:120:
// TODO(tschumann): This seems lik a bug. When marking a bookmark as
On 2017/01/12 14:50:53, jkrcal wrote:
> This WAI. Suggestions should not be updated after dismissing a bookmark.
> 
> See the comment in
>
https://cs.chromium.org/chromium/src/components/ntp_snippets/content_suggesti...
> 
> If you still think this is a bug (because we should update other open NTPs),
put
> a TODO at content_suggestions_provider.h:83

good to know :-) I'll re-phrase this comment then accordingly.

https://codereview.chromium.org/2618243004/diff/40001/components/ntp_snippets...
File components/ntp_snippets/mock_content_suggestions_provider.h (right):

https://codereview.chromium.org/2618243004/diff/40001/components/ntp_snippets...
components/ntp_snippets/mock_content_suggestions_provider.h:20: class
MockProvider : public ContentSuggestionsProvider {
On 2017/01/12 14:50:53, jkrcal wrote:
> nit: MockContentSuggestionsProvider to keep naming consistent with the file
> name?

it's used rarely enough -- fixed.

[tschumann, 2017-01-12 15:06:32]

The CQ bit was checked by tschumann@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-12 15:07:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-12 16:04:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-12 16:04:20]

Dry run: This issue passed the CQ dry run.

[tschumann, 2017-01-12 16:41:50]

The CQ bit was checked by tschumann@chromium.org

[tschumann, 2017-01-12 16:41:51]

The patchset sent to the CQ was uploaded after l-g-t-m from jkrcal@chromium.org
Link to the patchset: https://codereview.chromium.org/2618243004/#ps60001
(title: "renamed MockProvider and updated tests comments about expected
behavior")

[commit-bot: I haz the power, 2017-01-12 16:42:16]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-12 16:46:46]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1484239310972810,
"parent_rev": "9f158899c5bec9e547cbcc49aff6f087620f6129", "commit_rev":
"25e7324ca818226f5f68aca6eae4348ae739b247"}

[commit-bot: I haz the power, 2017-01-12 16:47:17]

Description was changed from

==========
Add a unit test for the BookmarkSuggestionsProvider.

This is primarily to verify behavior fixed in
https://codereview.chromium.org/2616633002/ but also adds tests for more
fundamental behavior of the BookmarkSuggestionsProvider.

This CL also moves ContentSuggestionsProvider test doubles out of specific unit
tests and into separate targets so that we can reuse them.

BUG=674178
==========

to

==========
Add a unit test for the BookmarkSuggestionsProvider.

This is primarily to verify behavior fixed in
https://codereview.chromium.org/2616633002/ but also adds tests for more
fundamental behavior of the BookmarkSuggestionsProvider.

This CL also moves ContentSuggestionsProvider test doubles out of specific unit
tests and into separate targets so that we can reuse them.

BUG=674178

Review-Url: https://codereview.chromium.org/2618243004
Cr-Commit-Position: refs/heads/master@{#443260}
Committed:
https://chromium.googlesource.com/chromium/src/+/25e7324ca818226f5f68aca6eae4...
==========

[commit-bot: I haz the power, 2017-01-12 16:47:22]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/25e7324ca818226f5f68aca6eae4...