android / android-test

An extensive framework for testing Android apps
https://android.github.io/android-test
Apache License 2.0
1.16k stars 314 forks source link

NoMatchingViewException leaks view hierarchies until test suite finishes #1661

Open pyricau opened 1 year ago

pyricau commented 1 year ago

Description

While a test suite is running, the AndroidJUnitRunner thread holds the test suite result in a local variable of type org.junit.runner.Result where Result.failures is a list of org.junit.runner.notification.Failure and each Failure has a Failure.fThrownException that points to the thrown exception, which here is a androidx.test.espresso.NoMatchingViewException. Unfortunately, NoMatchingViewException has a strong reference to the root view where the exception was thrown.

This means that while the test suite is running, any test failing due to a view matching error will leak to the corresponding root view being leaked for the duration of the test suite. This increases memory pressure. Developers who run LeakCanary in tests might see subsequent tests fail due to a leak being detected, when that leak is actually caused by the Espresso.

Steps to Reproduce

Expected Results

AndroidX Test and Android OS Versions

All versions of AndroidX Test and all versions of Android.

Link to a public git repo demonstrating the problem

https://github.com/square/leakcanary/issues/2297

ralf-at-android commented 1 year ago

Hi, We looked into it, and as you correctly pointed out in the description above, it looks like JUnit keeps references to all test failures and exceptions during the test run. So from an Espresso point of view, there's not much we can do here in the short term. Feel free to contribute a patch if you have a better way to handle this.

TWiStErRob commented 1 year ago

It is possible to solve this by making NoMatchingViewException.rootView a WeakReference. The only usage of getRootView() is in a FailureHandler, which should run immediately-ish after an exception happens and at that time the activity should be still visible (therefore the weak reference would still be strong). After this rendering of the view hierarchy is done the rootView is just leaking as it is now. The WeakReference would take care of this by "forgetting" the rootView after it's not used. The getRootView is nullable anyway so there's no breaking change here IMO.

pyricau commented 1 year ago

@ralf-at-android Would you be ok with the solution @TWiStErRob is describing? I do think it makes sense that you'd be able to access the view hierarchy as long as it's attached (i.e. essentially soon after the test fails) but once that UI is gone then Espresso should stop holding on to it.

To do this right, we'd probably need to also take care of the list of adapter views (=> List<WeakReference<View>>) and probably all the exceptions that implement RootViewException.

darnmason commented 1 year ago

Any development on this issue?

rcgonzalezf commented 4 months ago

It will be nice that now that 3.6.0 has been released to have this fixed.