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

Issue 803484 link

Starred by 1 user

Issue metadata

Status: Untriaged
Owner: ----
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug


Participants' hotlists:
ErrorProne-Fixes


Sign in to add a comment

Fix unnecessary catch/fail losing exception stack trace in test code

Project Member Reported by wnwen@chromium.org, Jan 18 2018

Issue description

1. Read http://errorprone.info/bugpattern/CatchFail
2. Remove 'CatchFail' suppression for errorprone.
3. Fix or suppress each instance in code.
4. Upgrade 'CatchFail' to errorprone error to prevent future regressions.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 24 2018

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

commit 9330d6131bdd0bf9c48917eb22ef1ae63904f049
Author: Troy Hildebrandt <thildebr@chromium.org>
Date: Wed Jan 24 21:40:16 2018

Fix NewApi Lint errors for browser/locale tests.

NewApi warnings were suppressed accidentally and an AssertionError
constructor that requires API 19 was used. The NewApi warning are back,
so this fix avoids using the AssertionError, and throws the exceptions
as they are.

R=tedchoc@chromium.org

Bug:  805509 ,803484
Change-Id: I35723261c6d72bdac08d0236dbb8e46d752e0bfb
Reviewed-on: https://chromium-review.googlesource.com/883566
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531690}
[modify] https://crrev.com/9330d6131bdd0bf9c48917eb22ef1ae63904f049/chrome/android/javatests/src/org/chromium/chrome/browser/locale/DefaultSearchEnginePromoDialogTest.java
[modify] https://crrev.com/9330d6131bdd0bf9c48917eb22ef1ae63904f049/chrome/android/javatests/src/org/chromium/chrome/browser/locale/LocaleManagerReferralTest.java
[modify] https://crrev.com/9330d6131bdd0bf9c48917eb22ef1ae63904f049/chrome/android/javatests/src/org/chromium/chrome/browser/locale/LocaleManagerTest.java

Project Member

Comment 2 by bugdroid1@chromium.org, Jan 24 2018

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

commit 2b6c4fefb6b58422f4c2b21ab4d9593192a946be
Author: Troy Hildebrandt <thildebr@chromium.org>
Date: Wed Jan 24 22:09:55 2018

Fix NewApi Lint errors for Android chrome/browser tests.

NewApi warnings were suppressed accidentally and an AssertionError
constructor that requires API 19 was used. The NewApi warnings are back,
so this fix avoids using the AssertionError and throws the exceptions
as they are.

Bug:  805509 ,803484
Change-Id: I1f342d06cb7e4e24ea816803d0e97dbeab6f59d9
Reviewed-on: https://chromium-review.googlesource.com/884385
Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531705}
[modify] https://crrev.com/2b6c4fefb6b58422f4c2b21ab4d9593192a946be/chrome/android/javatests/src/org/chromium/chrome/browser/BackgroundSyncLauncherTest.java
[modify] https://crrev.com/2b6c4fefb6b58422f4c2b21ab4d9593192a946be/chrome/android/javatests/src/org/chromium/chrome/browser/ProcessIsolationTest.java
[modify] https://crrev.com/2b6c4fefb6b58422f4c2b21ab4d9593192a946be/chrome/android/javatests/src/org/chromium/chrome/browser/ShareIntentTest.java
[modify] https://crrev.com/2b6c4fefb6b58422f4c2b21ab4d9593192a946be/chrome/android/javatests/src/org/chromium/chrome/browser/TabsTest.java

Project Member

Comment 3 by bugdroid1@chromium.org, Jan 24 2018

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

commit 3027e9c514adf2e3dd0e3a455a2074296cab3cbd
Author: Troy Hildebrandt <thildebr@chromium.org>
Date: Wed Jan 24 22:28:12 2018

Fix NewApi Lint errors for UndoTabModelTest.

NewApi warnings were suppressed accidentally and an AssertionError
constructor that requires API 19 was used. The NewApi warnings are back,
so this fix avoids using the AssertionError, and throws the exceptions
as they are.

Bug:  805509 ,803484
Change-Id: Ibf17400b5703a92f98bf647ef01cc7e819d6e323
Reviewed-on: https://chromium-review.googlesource.com/884364
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531718}
[modify] https://crrev.com/3027e9c514adf2e3dd0e3a455a2074296cab3cbd/chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/UndoTabModelTest.java

Project Member

Comment 4 by bugdroid1@chromium.org, Jan 25 2018

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

commit d949ea71bea77aeb5dde464028a513eb529eabb5
Author: Troy Hildebrandt <thildebr@chromium.org>
Date: Thu Jan 25 18:10:06 2018

Fix NewApi Lint errors for browser/customtabs tests.

NewApi warnings were suppressed accidentally and an AssertionError
constructor that requires API 19 was used. The NewApi warnings are back,
so this fix avoids using the AssertionError, and throws the exceptions
as they are.

R=yusufo@chromium.org

Bug:  805509 ,803484
Change-Id: I488c5f8de4351acac14874c7743a52ee0612c37c
Reviewed-on: https://chromium-review.googlesource.com/884206
Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531942}
[modify] https://crrev.com/d949ea71bea77aeb5dde464028a513eb529eabb5/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabActivityTest.java
[modify] https://crrev.com/d949ea71bea77aeb5dde464028a513eb529eabb5/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabsConnectionTest.java
[modify] https://crrev.com/d949ea71bea77aeb5dde464028a513eb529eabb5/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabsTestUtils.java

Project Member

Comment 5 by bugdroid1@chromium.org, Jan 25 2018

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

commit e37c39aa8e1fce4d4b61571116870d1b4fe4ce8a
Author: Troy Hildebrandt <thildebr@chromium.org>
Date: Thu Jan 25 18:16:55 2018

Fix NewApi Lint errors for SearchActivityTest.

NewApi warnings were suppressed accidentally and an AssertionError
constructor that requires API 19 was used. The NewApi warnings are back,
so this fix avoids using the AssertionError, and throws the exceptions
as they are.

Since a couple calls that were being caught in a try/catch block before
need to throw exceptions but can't from a Runnable, a waitForActivity
method in ActivityUtils was added that takes a Callable<Void> was
added. The original that takes a Runnable is just a wrapper around this
new method so other tests are not affected.

Bug:  805509 ,803484
Change-Id: I7960b2725b4fad1496fbfde7aafa8e2cf52ec686
Reviewed-on: https://chromium-review.googlesource.com/884362
Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531951}
[modify] https://crrev.com/e37c39aa8e1fce4d4b61571116870d1b4fe4ce8a/chrome/android/javatests/src/org/chromium/chrome/browser/searchwidget/SearchActivityTest.java
[modify] https://crrev.com/e37c39aa8e1fce4d4b61571116870d1b4fe4ce8a/chrome/test/android/javatests/src/org/chromium/chrome/test/util/ActivityUtils.java

Project Member

Comment 6 by bugdroid1@chromium.org, Jan 29 2018

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

commit 4c915dfebfa4c3f0d5d11fe5731fe2c4be6c17bd
Author: Troy Hildebrandt <thildebr@chromium.org>
Date: Mon Jan 29 19:25:44 2018

Fix NewApi Lint errors for TabsTest.

NewApi warnings were suppressed accidentally and an AssertionError
constructor that requires API 19 was used. The NewApi warnings are back,
so this fix avoids using the AssertionError, and throws the exceptions
as they are.

This is an accidental leftover from the previous fixes, and is blocking
the CL that removes the NewApi error suppression.

Bug:  805509 ,803484
Change-Id: Ib2cc11c3ffdcfc50a4618d98d016ab52e0f1f624
Reviewed-on: https://chromium-review.googlesource.com/891578
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532538}
[modify] https://crrev.com/4c915dfebfa4c3f0d5d11fe5731fe2c4be6c17bd/chrome/android/javatests/src/org/chromium/chrome/browser/TabsTest.java

Status: Untriaged (was: Assigned)
Assigned, but no owner or component? Please find a component and/or owner
Labels: DevX

Sign in to add a comment