Leakcanary makes test always pass when using androidx.test >=1.3.1-alpha02 #1986
Comments
|
I suggest to make |
|
@pyricau unless this proposal is considered too defensive and this state isn't expected when executing Is it something possible to have |
|
@pyricau do you have a recipe somewhere to use |
|
The real issue causing trouble is the following exception, not I protected such error and the test is now correctly marked as failed as expected. I'll push a PR with my changes even if I don't think it will be the end of the story. |
…TestOnLeakRunListener .testResultPublisher` (relates to square#1986) This seems to be the cause of square#1986 but even without this fix, the issue example worked by fixing the root cause of this issue.
…Listener` field (fixes square#1986) Such field might be missing, observed with `android-test 1.3.1-alpha02` at least.
|
It seems this commit android/android-test/e998fe63c1870e79548d2b6386db81ca07010c5c introduced the issue.
|
|
Catching up, sorry it took me so long to take a look. |
The LeakCanary example app has UI tests that fail (and don't run on CI). You could add a test there (and update the espresso version temporarily). Based on what you're telling me, it sounds like we need a two different impls. We could check for whether the |
|
FYI: The issue persiststs in LeakCanary 2.7 and androidx.test 1.4.0 as well. Surprisingly, the behavior I observe in my default project is a bit different. It doesn't make the whole test suite to pass, but it shows Leakcanary Failure as a separate but yeah, basically Leakcanary Espresso intergration is broken utterly. |
mateuszkwiecinski
changed the title
|
I face the same issue when updating to |
@dawidhyzy You had to explicitly enable it, soo the way to disable the integration is to comment out lines responsible for enabling it
Source: https://square.github.io/leakcanary/faq/#how-do-i-know-if-leakcanary-is-running |
|
It isn't linked but it seems like this issue will be fixed by #2147. |
|
androidx.test maintainer here. Please let us know how we could help migrate LeakCanary from depending on androidx.test implementation details. #2147 will only 'fix' this issue until the next time the implementation is changed again. Anything in androidx.test.internal is not a stable API surface and no external libraries should depend on it. |
|
Thanks everybody, sorry this is taking a while.
@brettchabot I hear you, but also: https://en.wikipedia.org/wiki/Desire_path In other words, when an API is missing, users will find a way around it. The missing API was "I need a way to fail an otherwise successful test once it's done running". A bit like a rule but that applies to all tests There is one API that's available: Junit rules. The problem with those is two fold: 1) they're not applied to all tests, so you have to manually add to each test. 2) Espresso itself had leaks within its outer rules code (it keeps activities after they're destroyed) and that's meant leak detection would trigger due to Espresso. I believe that's fixed now, and we could likely use a rule, but it's definitely way less convenient. I might take a spike at migrating this and the documentation to rely on rules instead, but that's a pretty significant change. |
|
Would a custom runner work instead? https://stackoverflow.com/questions/9034812/how-to-apply-a-junit-rule-for-all-test-cases-in-a-suite AndroidJUnitRunner supports specifying a custom runner builder via command line. We could also consider enhancing AndroidJUnitRunner to support specifying RunnerBuilders via ServiceLoader if that would be easier. It already does this for RunListener's. That way if leak canary is on the classpath, users would automatically get its custom runner. |
|
Interesting suggestion, I need to try this out! |
This commit deprecates FailTestOnLeakRunListener, FailAnnotatedTestOnLeakRunListener, FailTestOnLeak and InstrumentationLeakDetector. - `FailTestOnLeakRunListener` is replaced by the `DetectLeaksAfterTestSuccess` test rule. - Test code and test utilities can also call `LeakAssertions.assertNoLeak()`, which by default throws an exception if leaks are found. - Specific tests can make calls to `LeakAssertions.assertNoLeak()` a no-op by using the `@SkipLeakDetection` annotation (and setting up the TestDescriptionHolder test rule) - The leak detection and reporting behavior can be customized with `DetectLeaksAssert.update()` Fixes #1986
|
I'm working on deprecating the run listener, providing more fine grained APIs and building back on top with rules etc. See #2244 |
This commit deprecates FailTestOnLeakRunListener, FailAnnotatedTestOnLeakRunListener, FailTestOnLeak and InstrumentationLeakDetector. - `FailTestOnLeakRunListener` is replaced by the `DetectLeaksAfterTestSuccess` test rule. - Test code and test utilities can also call `LeakAssertions.assertNoLeak()`, which by default throws an exception if leaks are found. - Specific tests can make calls to `LeakAssertions.assertNoLeak()` a no-op by using the `@SkipLeakDetection` annotation (and setting up the TestDescriptionHolder test rule) - The leak detection and reporting behavior can be customized with `DetectLeaksAssert.update()` Fixes #1986 Co-authored-by: Preetha Suraj <preetha@squareup.com>
Description
When using leakcanary with android-test 1.3.1-alpha02 ui test suite always pass.😞
Using alpha version is required due to android/android-test#743 but in case when it gets into stable release more people will face the issue
Steps to Reproduce
Sample project that has test suite passing despite it shouldn't
mateuszkwiecinski/orchestrator_doesnt_work#7
The repository contains simple ui tests setup with orchestrator and leakcananry enabled
Expected behavior: I expect to see test failure and have failing workflow (the test
shouldFailshould fail)Version Information
Additional Information
When looking at logcat it points at leakcanary:
The text was updated successfully, but these errors were encountered: