android / android/android-test

New feature: annotate all factory methods with @CheckResult

Open
#1,325 19 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
1.2k
Forks
342
Avg merge
11h 29m
Merged PRs (30d)
2

Description

### Description
To prevent situations like: https://twitter.com/yogurtearl/status/1347344454668062722

We could leverage a best effort annotation that tools could detect.
Using `androidx.annotation.CheckResult` would be ideal, because (hopefully) lint would pick up on that.
If lint doesn't [Errorprone can](https://errorprone.info/bugpattern/CheckReturnValue), so an alternative is JSR305's `javax.annotation.CheckReturnValue`. Best is both, actually this already exists on [Espresso.onView](https://github.com/android/android-test/blob/master/espresso/core/java/androidx/test/espresso/Espresso.java#L85-L87):

https://github.com/android/android-test/blob/41855232b54fb77af11744e446d11204749b789c/espresso/core/java/androidx/test/espresso/Espresso.java#L85-L87

### Steps to Reproduce
In a sudden lapse of judgement, import the wrong thing:
```java
import static androidx.test.espresso.action.ViewActions.closeSoftKeyboard;

@Test public void test() {
...
closeSoftKeyboard();
...
}
```
of course the right one is:
```java
import static androidx.test.espresso.Espresso.closeSoftKeyboard;
```

### Expected Results
Some kind of warning when using Espresso wrong.

### Actual Results
Silent nothing, many hours of dev time wasted.

### AndroidX Test and Android OS Versions
N/A

### Link to a public git repo demonstrating the problem:
N/A

### Proposal

Annotate everything in:
* *View*Actions.*
* androidx.test.espresso.matcher.*
* androidx.test.espresso.intent.Intents.intending
* potentially others

I would be happy to contribute the adding of annotations on the Espresso public surface, but I have a feeling internally in Google you might require some cleanup in the monorepo if this ever happened (i.e. remove side-effect-less calls, if any). Although those can be discovered on this branch, and committed before merge.

Contributor guide

Open the contributing guide

Research direction

Start by reviewing the existing CheckResult usage in Espresso.java, then inspect the public APIs under ViewActions.*, androidx.test.espresso.matcher.*, and androidx.test.espresso.intent.Intents.intending. Determine the annotation coverage needed across these surfaces and verify that the reported incorrect static import produces a warning from the intended tooling.

Written by the indexing model from the issue text.

Assessment

Tech stack
android, java
Domain
mobile, testing-qa
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.