Simple improvements to RecentTabHelperTest.

Simple improvements to RecentTabHelperTest.

Makes simple improvements to RecentTabHelperTest:
- Use ASSERT calls in some cases where test code would crash later if
  the expectation was not met.
- Added missing check of model().is_loaded().
- Update and reenable the DownloadRequest test.

BUG=674184
Committed: https://crrev.com/b98b091139bd28c644ab975ef39d1bc059bd60ce
Cr-Commit-Position: refs/heads/master@{#438540}

[carlosk, 2016-12-13 22:07:42]

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

[commit-bot: I haz the power, 2016-12-13 22:08:25]

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, 2016-12-13 22:49:21]

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

[commit-bot: I haz the power, 2016-12-13 22:49:22]

Dry run: This issue passed the CQ dry run.

[carlosk, 2016-12-13 23:48:08]

carlosk@chromium.org changed reviewers:
+ fgorski@chromium.org

[carlosk, 2016-12-13 23:48:09]

fgorski@: PTAL.

[fgorski, 2016-12-14 16:05:29]

lgtm

comment about the missing bug: I think you could open one that would cover all
of our code. There is a high chance that we check collection sizes in tests
using expects vs. asserts in more places than one. I prefer asserts.

https://codereview.chromium.org/2571633003/diff/1/chrome/browser/android/offl...
File chrome/browser/android/offline_pages/recent_tab_helper_unittest.cc (right):

https://codereview.chromium.org/2571633003/diff/1/chrome/browser/android/offl...
chrome/browser/android/offline_pages/recent_tab_helper_unittest.cc:463:
EXPECT_TRUE(model()->is_loaded());
this is probably not needed, and Justin is working on removing it.

[carlosk, 2016-12-14 17:49:35]

Description was changed from

==========
Simple improvements to RecentTabHelperTest.

Makes simple improvements to RecentTabHelperTest:
- Use ASSERT calls in some cases where test code would crash later if
  the expectation was not met.
- Added missing check of model().is_loaded().
- Update and reenable the DownloadRequest test.

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

to

==========
Simple improvements to RecentTabHelperTest.

Makes simple improvements to RecentTabHelperTest:
- Use ASSERT calls in some cases where test code would crash later if
  the expectation was not met.
- Added missing check of model().is_loaded().
- Update and reenable the DownloadRequest test.

BUG=674184
==========

[carlosk, 2016-12-14 17:52:06]

The CQ bit was checked by carlosk@chromium.org

[carlosk, 2016-12-14 17:52:06]

The patchset sent to the CQ was uploaded after l-g-t-m from fgorski@chromium.org
Link to the patchset: https://codereview.chromium.org/2571633003/#ps20001
(title: "Addressed reviewer comment.")

[carlosk, 2016-12-14 17:52:22]

Thanks

On 2016/12/14 16:05:29, fgorski wrote:
> lgtm
> 
> comment about the missing bug: I think you could open one that would cover all
> of our code. There is a high chance that we check collection sizes in tests
> using expects vs. asserts in more places than one. I prefer asserts.

Created: https://bugs.chromium.org/p/chromium/issues/detail?id=674184

https://codereview.chromium.org/2571633003/diff/1/chrome/browser/android/offl...
File chrome/browser/android/offline_pages/recent_tab_helper_unittest.cc (right):

https://codereview.chromium.org/2571633003/diff/1/chrome/browser/android/offl...
chrome/browser/android/offline_pages/recent_tab_helper_unittest.cc:463:
EXPECT_TRUE(model()->is_loaded());
On 2016/12/14 16:05:29, fgorski wrote:
> this is probably not needed, and Justin is working on removing it.

Done.

[commit-bot: I haz the power, 2016-12-14 17:52:54]

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, 2016-12-14 18:30:20]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1481737926483830,
"parent_rev": "71a86660ce7bd654dfe8b1758cd7703b713c5a8f", "commit_rev":
"4d2a2bfaa473dee86302edb6f73b06687fd56012"}

[commit-bot: I haz the power, 2016-12-14 18:30:50]

Description was changed from

==========
Simple improvements to RecentTabHelperTest.

Makes simple improvements to RecentTabHelperTest:
- Use ASSERT calls in some cases where test code would crash later if
  the expectation was not met.
- Added missing check of model().is_loaded().
- Update and reenable the DownloadRequest test.

BUG=674184
==========

to

==========
Simple improvements to RecentTabHelperTest.

Makes simple improvements to RecentTabHelperTest:
- Use ASSERT calls in some cases where test code would crash later if
  the expectation was not met.
- Added missing check of model().is_loaded().
- Update and reenable the DownloadRequest test.

BUG=674184

Review-Url: https://codereview.chromium.org/2571633003
==========

[commit-bot: I haz the power, 2016-12-14 18:30:51]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-12-14 18:34:52]

Description was changed from

==========
Simple improvements to RecentTabHelperTest.

Makes simple improvements to RecentTabHelperTest:
- Use ASSERT calls in some cases where test code would crash later if
  the expectation was not met.
- Added missing check of model().is_loaded().
- Update and reenable the DownloadRequest test.

BUG=674184

Review-Url: https://codereview.chromium.org/2571633003
==========

to

==========
Simple improvements to RecentTabHelperTest.

Makes simple improvements to RecentTabHelperTest:
- Use ASSERT calls in some cases where test code would crash later if
  the expectation was not met.
- Added missing check of model().is_loaded().
- Update and reenable the DownloadRequest test.

BUG=674184

Committed: https://crrev.com/b98b091139bd28c644ab975ef39d1bc059bd60ce
Cr-Commit-Position: refs/heads/master@{#438540}
==========

[commit-bot: I haz the power, 2016-12-14 18:34:53]

Patchset 2 (id:??) landed as
https://crrev.com/b98b091139bd28c644ab975ef39d1bc059bd60ce
Cr-Commit-Position: refs/heads/master@{#438540}