oppia / oppia-android

A free, online & offline learning platform to make quality education accessible for all.
https://www.oppia.org
Apache License 2.0
322 stars 523 forks source link

Introduce test or static analysis check to prevent potential deadlocks when misusing Espresso's API #3150

Open BenHenning opened 3 years ago

BenHenning commented 3 years ago

Is your feature request related to a problem? Please describe. See #3043 for context. When using Espresso functions (like onView) within a runOnUiThread block, Espresso may deadlock. This is not only ideal, it's also unexpected and difficult to debug. When developers hit these situations, they're unsure how to proceed or investigate.

Describe the solution you'd like We should introduce some sort of check that detects when developers are using Espresso methods within runOnUiThread & triggering a failure. A static analysis check is probably the most straightforward with the current codebase, but it's challenging because tests might call helpers or test utilities that in turn call Espresso methods.

Instead, we should consider wrapping Espresso's API so that we can introduce custom detection mechanisms (e.g. we could add a check to verify if an Espresso method is being called on the main thread not on Robolectric as that should always cause a fast failure). Note that while this part of the solution seems straightforward, what's unclear is how best to abstract away Espresso functionality. It may actually be advantageous to take this as an opportunity to design a clean common testing interface for Android that leverages both Espresso & Robolectric-specific functionality in a clean way. This requires a design document.

Describe alternatives you've considered As mentioned above, the primary alternative would be a static analysis check but it's hard to get right & the team benefits broadly from a custom design testing interface.

BenHenning commented 3 years ago

NB: this might be more like a full project actually than a mini project.