Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Automate LeakCanary checking in existing UI tests #4506

Closed
colintheshots opened this issue Aug 2, 2019 · 9 comments
Closed

Automate LeakCanary checking in existing UI tests #4506

colintheshots opened this issue Aug 2, 2019 · 9 comments
Labels
E3 Estimation Point: average, 2 - 3 days eng:automation Build automation, Continuous integration, .. eng:health Improve code health P2 Upcoming release perf:resource-use https://wiki.mozilla.org/Performance/Bugzilla#Keywords performance Possible performance wins pin Issues, features, improvements that are still valid

Comments

@colintheshots
Copy link
Contributor

colintheshots commented Aug 2, 2019

LeakCanary has the capacity to fail UI tests when a leak occurs during the test. We should add memory leak checking to our existing UI tests (when not actively being used for performance testing, since it slows them down).

┆Issue is synchronized with this Jira Task

@colintheshots colintheshots added eng:automation Build automation, Continuous integration, .. eng:health Improve code health P2 Upcoming release labels Aug 2, 2019
@colintheshots colintheshots added this to To be Triaged in Fenix Sprint Kanban via automation Aug 2, 2019
@colintheshots colintheshots added the E3 Estimation Point: average, 2 - 3 days label Aug 2, 2019
@colintheshots colintheshots moved this from To be Triaged to Prioritized Eng Backlog in Fenix Sprint Kanban Aug 2, 2019
@rpappalax rpappalax self-assigned this Aug 22, 2019
@sblatz sblatz removed this from Prioritized Eng Backlog in Fenix Sprint Kanban Sep 27, 2019
@sblatz
Copy link
Contributor

sblatz commented Sep 27, 2019

Removing this from our Kanban since it looks like @rpappalax is digging into this.

@mcomella mcomella added the performance Possible performance wins label Nov 22, 2019
@mcomella mcomella added this to Needs prioritization in Performance, front-end roadmap via automation Nov 22, 2019
@mcomella mcomella moved this from Needs prioritization to Low impact (unprioritized) in Performance, front-end roadmap Dec 3, 2019
@mcomella
Copy link
Contributor

mcomella commented Aug 4, 2020

@rpappalax Is this still something you're looking into? There are several memory leaks already in the product and some users are reporting high memory usage #12374 and stuttering that seem related #12302 so it'd be great to prevent additional memory leaks from being introduced!


I found the documentation and it mentions:

Running leak detection in UI tests means you can detect memory leaks automatically in Continuous Integration prior to those leaks being merged into the codebase. However, as LeakCanary runs with a 5 seconds delay and freezes the VM to take a heap dump, this can introduce flakiness to the UI tests. Therefore it is automatically disabled by setting LeakCanary.config.dumpHeap to false when JUnit is on the runtime classpath.

It sounds like you can still run the memory leak checks without doing the heap dump that creates flakiness but I suppose the documentation is unclear.

In any case, given the possibility of disrupting the stability of the tests (and some tests failing due to the existing memory leaks), I was thinking it might be less problematic for your team to introduce the change but let me know if not!

@mcomella mcomella moved this from Low impact (unprioritized) to Needs prioritization in Performance, front-end roadmap Aug 4, 2020
@mcomella
Copy link
Contributor

mcomella commented Aug 4, 2020

Moving to triage: we may want to add this to our top 10 inter team bug list.

@hwinnemoe
Copy link

hwinnemoe commented Aug 4, 2020

Could this detect leaks that originate from GV? BZ-1655580 / #9065 was only caught by manual obversation of the memory usage during specific actions. And it was specific to Fenix, the issue was not reproducible on GeckoView Example.

@mcomella
Copy link
Contributor

mcomella commented Aug 4, 2020

@hwinnemoe No, the tooling only works for the JVM. That's a good point though – I'll check with folks on how we catch memory leaks in GV embedded in fenix.

@mcomella mcomella moved this from Needs prioritization to Top 10 Inter-Team bugs in Performance, front-end roadmap Aug 12, 2020
@stale
Copy link

stale bot commented Feb 4, 2021

See: #17373 This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Feb 4, 2021
@st3fan st3fan added this to Backlog (Untriaged) in Mobile CI/Build Automation Feb 8, 2021
@stale stale bot closed this as completed Feb 11, 2021
Performance, front-end roadmap automation moved this from Top 10 Inter-Team bugs to Done Feb 11, 2021
@mcomella mcomella reopened this Aug 19, 2021
@stale stale bot removed the wontfix label Aug 19, 2021
@mcomella
Copy link
Contributor

We want to catch memory leaks in testing so let's keep this open.

@mcomella mcomella added the pin Issues, features, improvements that are still valid label Aug 19, 2021
@mcomella mcomella added this to Needs prioritization in Performance, front-end roadmap via automation Aug 19, 2021
@MarcLeclair MarcLeclair moved this from Needs prioritization to Low impact (unprioritized) in Performance, front-end roadmap Aug 25, 2021
@rpappalax rpappalax moved this from Backlog (Untriaged) to In Progress in Mobile CI/Build Automation Feb 8, 2022
@mcomella mcomella added the perf:resource-use https://wiki.mozilla.org/Performance/Bugzilla#Keywords label Mar 30, 2022
@mcomella
Copy link
Contributor

Triage: we're concerned this could introduce additional flakiness to our tests. If it's flaky or it significantly slows down UI tests, we could introduce this as a Tier 2 test that doesn't run on every PR.

@cpeterso
Copy link

Moved to bugzilla: https://bugzilla.mozilla.org/show_bug.cgi?id=1807307

Change performed by the Move to Bugzilla add-on.

Performance, front-end roadmap automation moved this from Triaged (unordered) to Done Dec 23, 2022
Mobile CI/Build Automation automation moved this from In Progress to Review Dec 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
E3 Estimation Point: average, 2 - 3 days eng:automation Build automation, Continuous integration, .. eng:health Improve code health P2 Upcoming release perf:resource-use https://wiki.mozilla.org/Performance/Bugzilla#Keywords performance Possible performance wins pin Issues, features, improvements that are still valid
Development

No branches or pull requests

6 participants