(Android) Removing deprecated methods in chrome_junit_tests

(Android) Removing deprecated methods in chrome_junit_tests

BUG=683781
Review-Url: https://codereview.chromium.org/2645243002
Cr-Commit-Position: refs/heads/master@{#446863}
Committed: https://chromium.googlesource.com/chromium/src/+/04617ab5401a00491238d22ae25bd5da5ce5f66d

[marcin, 2017-01-23 02:05:18]

Description was changed from

==========
x


x


indent


Deprecated tests 1

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

to

==========
(Android) Removing deprecated methods in chrome_junit_tests

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

[marcin, 2017-01-23 02:09:05]

Patchset #1 (id:1) has been deleted

[marcin, 2017-01-23 02:12:55]

Patchset #1 (id:20001) has been deleted

[marcin, 2017-01-23 02:18:26]

Description was changed from

==========
(Android) Removing deprecated methods in chrome_junit_tests

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

to

==========
(Android) Removing deprecated methods in chrome_junit_tests

BUG=683781
==========

[marcin, 2017-01-23 02:20:48]

marcin@mwiacek.com changed reviewers:
+ avayvod@chromium.org, dfalcantara@chromium.org, peconn@chromium.org,
petewil@chromium.org

[marcin, 2017-01-23 02:20:49]

All,

Please review files, which are owned by you. Thank you in advance.

[PEConn, 2017-01-23 09:57:21]

On 2017/01/23 02:20:49, marcin wrote:
> All,
> 
> Please review files, which are owned by you. Thank you in advance.

**/cards/* LGTM

[Pete Williamson, 2017-01-23 17:30:45]

OfflinePageBridgeUnitTest changes lgtm

[marcin, 2017-01-23 18:46:35]

marcin@mwiacek.com changed reviewers:
+ mlamouri@chromium.org

[marcin, 2017-01-23 18:48:48]

[...]

Thank you for reviews,

mlamouri@, could you review MediaImageManagerTest.java  please ?

and 

dfalcantara@, could you review WebappDataStorageTest.java  please ?

[gone, 2017-01-23 19:05:06]

WebappDataStorageTest.java lgtm

[dgn, 2017-01-24 11:21:47]

dgn@chromium.org changed reviewers:
+ dgn@chromium.org

[dgn, 2017-01-24 11:21:48]

Thanks for the patch! Your changes for SuggestionsSectionTest don't make the
code more wrong, but if we are fixing inconsistencies, we might as well do it
properly. I added some suggestions in comments. 

Alternatively, you can exclude this file from the patch, I'll do the changes
separately and add some asserts in the prod code to enforce consistency between
the section category ID and the one of the suggestions it contains.

https://codereview.chromium.org/2645243002/diff/40001/chrome/android/junit/sr...
File
chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSectionTest.java
(right):

https://codereview.chromium.org/2645243002/diff/40001/chrome/android/junit/sr...
chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSectionTest.java:81:
List<SnippetArticle> snippets = createDummySuggestions(3,
KnownCategories.BOOKMARKS);
The method was deprecated because it can cause tests to fail because the
category of the created suggestions does not match the one expected in the
tests. Could you please also provide the right one where it makes sense?

Here the category id used (through #createSectionWithReloadAction) is 42

I think the better way to fix this would be to define in this class a
DEFAULT_CATEGORY_ID constant and use it both in createSectionWithReloadAction
and as argument for the related createDummySuggestions calls.

https://codereview.chromium.org/2645243002/diff/40001/chrome/android/junit/sr...
chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSectionTest.java:520:
List<SnippetArticle> snippets = createDummySuggestions(3,
KnownCategories.ARTICLES, "old");
this should also use 42 or the default id, as the section is created below with
that ID.

https://codereview.chromium.org/2645243002/diff/40001/chrome/android/junit/sr...
chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSectionTest.java:549:
List<SnippetArticle> snippets = createDummySuggestions(4,
KnownCategories.ARTICLES, "old");
same here

[whywhat, 2017-01-24 18:05:49]

avayvod@chromium.org changed reviewers:
- mlamouri@chromium.org

[whywhat, 2017-01-24 18:05:49]

MediaImageManagerTest lgtm

[marcin, 2017-01-24 19:02:26]

The CQ bit was checked by marcin@mwiacek.com

[commit-bot: I haz the power, 2017-01-24 19:03:23]

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

[marcin, 2017-01-24 19:04:57]

On 2017/01/24 11:21:48, dgn wrote:
> Thanks for the patch! Your changes for SuggestionsSectionTest don't make the
> code more wrong, but if we are fixing inconsistencies, we might as well do it
> properly. I added some suggestions in comments. 

> Alternatively, you can exclude this file from the patch, I'll do the changes
> separately and add some asserts in the prod code to enforce consistency
between
> the section category ID and the one of the suggestions it contains.

I see  "Your changes for SuggestionsSectionTest don't make the code more wrong"
and I have lgtm already. I will prepare separate patch and submit to you.
I hope/I think, that it's the best/most effective way

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

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

[commit-bot: I haz the power, 2017-01-24 19:16:47]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[marcin, 2017-01-24 19:18:45]

The CQ bit was checked by marcin@mwiacek.com

[commit-bot: I haz the power, 2017-01-24 19:19:18]

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-24 19:29:31]

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

[commit-bot: I haz the power, 2017-01-24 19:29:33]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[marcin, 2017-01-24 23:16:01]

The CQ bit was checked by marcin@mwiacek.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-24 23:17:04]

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-24 23:29:00]

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

[commit-bot: I haz the power, 2017-01-24 23:29:02]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[marcin, 2017-01-25 00:02:55]

The CQ bit was checked by marcin@mwiacek.com to run a CQ dry run

[marcin, 2017-01-25 00:03:02]

Patchset #2 (id:60001) has been deleted

[commit-bot: I haz the power, 2017-01-25 00:04:18]

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-25 00:18:42]

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

[commit-bot: I haz the power, 2017-01-25 00:18:43]

Dry run: This issue passed the CQ dry run.

[marcin, 2017-01-25 00:29:56]

Hi,

I had to make patch in different way:

avayvod@, please review MediaImageManagerTest.java again

Pete Williamson, please review OfflinePageBridgeUnitTest.java again

dgn, I haven't merged your proposal and I will want to make separate patch about
it, please tell me if this is not good for you

[Pete Williamson, 2017-01-25 00:48:45]

OfflinePageBridgeUnitTest still lgtm

[marcin, 2017-01-25 00:55:49]

The CQ bit was checked by marcin@mwiacek.com to run a CQ dry run

[marcin, 2017-01-25 00:56:09]

Patchset #2 (id:80001) has been deleted

[commit-bot: I haz the power, 2017-01-25 00:56:46]

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

[marcin, 2017-01-25 01:01:50]

On 2017/01/25 00:48:45, Pete Williamson wrote:
> OfflinePageBridgeUnitTest still lgtm

Hi,

Thx, I have uploaded 3rd version, it’s quite similar to 2nd (which I deleted).

If there is something wrong, pls let me know.

…and I’m really sorry for too fast sending request for 2nd review

Best Regards,
Marcin

[commit-bot: I haz the power, 2017-01-25 01:08:52]

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

[commit-bot: I haz the power, 2017-01-25 01:08:53]

Dry run: This issue passed the CQ dry run.

[Pete Williamson, 2017-01-25 01:19:09]

OfflinePageBridgeUnitTest still still lgtm

[marcin, 2017-01-27 22:38:53]

The CQ bit was checked by marcin@mwiacek.com

[marcin, 2017-01-27 22:38:53]

The patchset sent to the CQ was uploaded after l-g-t-m from
avayvod@chromium.org, peconn@chromium.org, dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/2645243002/#ps100001
(title: "Next patch version")

[commit-bot: I haz the power, 2017-01-27 22:39:59]

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-27 22:43:29]

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

[commit-bot: I haz the power, 2017-01-27 22:43:31]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[marcin, 2017-01-28 00:28:26]

Patchset #3 (id:120001) has been deleted

[marcin, 2017-01-28 00:34:47]

The CQ bit was checked by marcin@mwiacek.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-28 00:35:18]

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-28 00:48:26]

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

[commit-bot: I haz the power, 2017-01-28 00:48:27]

Dry run: This issue passed the CQ dry run.

[marcin, 2017-01-28 00:54:17]

The CQ bit was checked by marcin@mwiacek.com

[marcin, 2017-01-28 00:54:17]

The patchset sent to the CQ was uploaded after l-g-t-m from
avayvod@chromium.org, petewil@chromium.org, peconn@chromium.org,
dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/2645243002/#ps140001
(title: "Patch version after last code changes from other members")

[commit-bot: I haz the power, 2017-01-28 00:54:53]

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-28 00:59:36]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1485564857252390,
"parent_rev": "dc1ee54c6abd46f02adb02fcb119a553ee661a84", "commit_rev":
"04617ab5401a00491238d22ae25bd5da5ce5f66d"}

[commit-bot: I haz the power, 2017-01-28 01:00:38]

Description was changed from

==========
(Android) Removing deprecated methods in chrome_junit_tests

BUG=683781
==========

to

==========
(Android) Removing deprecated methods in chrome_junit_tests

BUG=683781

Review-Url: https://codereview.chromium.org/2645243002
Cr-Commit-Position: refs/heads/master@{#446863}
Committed:
https://chromium.googlesource.com/chromium/src/+/04617ab5401a00491238d22ae25b...
==========

[commit-bot: I haz the power, 2017-01-28 01:00:39]

Committed patchset #3 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/04617ab5401a00491238d22ae25b...