Replace Chrome-specific VisibleForTesting with android.support.annotation.VisibleForTesting |
||
Issue description
Support Library has VisibleForTesting annotation that has a number of benefits over Chrome-specific one:
1. Lint checks:
chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java:358 This method should only be accessed from tests or within private scope: VisibleForTests [warning]
NavigationEntry entry = controller.getEntryAtIndex(index);
~~~~~~~~~~~~~~~
2. Possibility to specify visibility for usages outside of tests. This visibility is taken into account by Lint and AndroidStudio.
@VisibleForTesting(otherwise)
3. AndroidStudio integration (see attached screenshot).
,
Sep 26
+agrieve in case we special case chromium's VisibleForTesting in our build tool chain. I'm fully supportive to move over to a more consistent one. Historically, we might have not used the support library variant because we didn't always have access to it under the chrome/ layer. That problem might and probably is a thing of the past.
,
Sep 27
Nothing special that I'm aware of. Swapping this out sgtm.
,
Oct 2
It turns out Log methods are actually marked with @VisibleForTesting: https://cs.chromium.org/chromium/src/base/android/java/src/org/chromium/base/Log.java?l=120&rcl=2d7072f986a163c0f00d47dd5b93eebc6b8a99e1. Should we just remove @VisibleForTesting from these methods?
,
Oct 2
The reason isn't jumping out to me anyways. sgtm to remove from there. |
||
►
Sign in to add a comment |
||
Comment 1 by bsazonov@chromium.org
, Sep 26Labels: -Type-Bug OS-Android Type-Task