Fix AssertionError lint errors |
|||
Issue description
Here is one example, there are lots of these in the codebase:
../SRC_ROOT1/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabsTestUtils.java:74 Call requires API level 19 (current min is 16): new java.lang.Assert
ionError: NewApi [warning] throw new AssertionError("Unexpected exception.", e);
Are these allowed for API level 16? Please give rationale and remove the suppression (that I will add to turn these checks back on) and fix these warnings.
Thanks!
,
Jan 24 2018
I think you don't have to do a clean revert, since you had some other cleanups in your CLs too. After my CL goes in (or you can apply it locally), you can just remove the suppression line and recompile all and it'll guide you to remove each one with failures: https://crrev.com/c/884004
,
Jan 24 2018
Either way it's up to you, thanks for doing cleanups though! Sorry that the warning wasn't working when you submitted those CLs.
,
Jan 24 2018
That's okay, I'll take care of this today, thanks for the quick heads up!
,
Jan 24 2018
,
Jan 24 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3955da70d67e9e6018208b9f7f89f92d803cfe40 commit 3955da70d67e9e6018208b9f7f89f92d803cfe40 Author: Peter Wen <wnwen@chromium.org> Date: Wed Jan 24 18:42:31 2018 Android: Update suppressions and fix NewApi Now that separate bugs have been filed to fix lint suppressions the code should be updated accordingly. Unforunately the NewApi suppression was wrongly added and allowed a whole bunch of failures into the codebase in a matter of days. Fixing that in this CL. BUG= 799070 , 804449 , 804438 , 805509 TBR=bauerb@chromium.org,michaelbai@chromium.org Change-Id: I22222837a70da3680a5968e7e002862ecdf12d04 Reviewed-on: https://chromium-review.googlesource.com/884004 Reviewed-by: agrieve <agrieve@chromium.org> Reviewed-by: Peter Beverloo <peter@chromium.org> Commit-Queue: Peter Wen <wnwen@chromium.org> Cr-Commit-Position: refs/heads/master@{#531622} [modify] https://crrev.com/3955da70d67e9e6018208b9f7f89f92d803cfe40/build/android/lint/suppressions.xml [modify] https://crrev.com/3955da70d67e9e6018208b9f7f89f92d803cfe40/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridge.java [modify] https://crrev.com/3955da70d67e9e6018208b9f7f89f92d803cfe40/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferences.java [modify] https://crrev.com/3955da70d67e9e6018208b9f7f89f92d803cfe40/tools/android/memconsumer/java/AndroidManifest.xml
,
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
,
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
,
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
,
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
,
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
,
Jan 25 2018
Closing this out, all the offending tests have been changed to avoid using the API 19 AssertionError constructor.
,
Jan 25 2018
As a last request, would you mind removing the suppression line from "build/android/lint/suppressions.xml"? This line and the corresponding comment: <ignore regexp="Call requires API level 19.*new java.lang.AssertionError"/> That will ensure this regression doesn't happen again. Thank you again for working on this!
,
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
,
Jan 29 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1d27b2eb8c431f0b6b5567d474de2e9fe47707b7 commit 1d27b2eb8c431f0b6b5567d474de2e9fe47707b7 Author: Troy Hildebrandt <thildebr@chromium.org> Date: Mon Jan 29 22:59:46 2018 Removes suppression of API Level 19 Lint warning for AssertionEror. Bug: 805509 Change-Id: Iff8178c0bc707da7c33b38f940cf6ca3fa19f994 Reviewed-on: https://chromium-review.googlesource.com/887293 Commit-Queue: Troy Hildebrandt <thildebr@chromium.org> Reviewed-by: Peter Wen <wnwen@chromium.org> Cr-Commit-Position: refs/heads/master@{#532644} [modify] https://crrev.com/1d27b2eb8c431f0b6b5567d474de2e9fe47707b7/build/android/lint/suppressions.xml |
|||
►
Sign in to add a comment |
|||
Comment 1 by thildebr@chromium.org
, Jan 24 2018