Leak Tracker should avoid flagging non-leaks in Flutter tests

[matthew-carroll]

The Leak Tracker currently identifies non-leaks as leaks in Flutter tests for disposable objects.

The following is an example of such a false-positive:

void main() {
  testWidgets("my widget text", (tester) async {
    final focusNode = FocusNode();

    await tester.pumpWidget(
      MaterialApp(...),
    );
  });
}

The Leak Tracker says that we should explicitly call dispose() on focusNode to avoid a leak. However, because the Flutter test runner resets the widget tree for every test, there shouldn't be any need at all to call dispose() within the test. There's no leak.

This same situation applies to things like ScrollController, TextEditingController, etc.

What to do about it

Option 1: Do nothing

The first option is to do nothing. The Leak Tracker will identify situations like the one above as leaks, and developers working on the Flutter framework will be forced to add a teardown for every such object, which calls dispose().

Pro's:

• All tests have a recipe for what to do, without any thinking. And adding teardowns shouldn't hurt anything.

Con's:

• Unknowing developers will read these teardowns, assume that they're necessary, and draw incorrect conclusions about how the Flutter test runner works. This will lead to a growing number of community members who spread incorrect information about the test runner (imagine Twitter posts telling all developers to always add teardowns with calls to dispose so they don't get memory leaks).
• When multiplied across all such tests, Flutter will accumulate something on the order of thousands of unnecessary lines of code.

Option 2: Don't identify these as leaks

The second option is to adjust the Leak Tracker so that it doesn't identify non-leaking disposable within tests as leaks.

The pro's and con's here are the inverse of those listed above.

How to avoid false positives in tests

In the general case, any disposable which is declared within a test() or testWidgets() method, should be safe from memory leaks. When that test ends, the declaration scope is gone, and the widget tree has also been released by the test runner. Therefore, GC is free to collect the disposable, without explicitly calling dispose.

Regex could probably be used to achieve this filtering:

1. Run the Leak Tracker as-is, and collect the memory leak candidates.
2. Inspect each memory leak candidate that exists in a test suite. Run regex against each of those test files and check whether the cause of the memory leak is inside a call to testWidgets() or outside. If it's inside, then throw away that leak candidate because it isn't a leak. If it's outside of testWidgets() then it might be a legitimate leak, report it.