I'm curious what the leak is here? Those FocusNodes are instantiated within each test. What's leaking outside of the tests?

I see this already got an approval, but without answering my question. Please don't merge this until someone explains what's being leaked in the tests.

I'm curious what the leak is here? Those FocusNodes are instantiated within each test. What's leaking outside of the tests?

Leak tracker watches that every disposable, created by the test's callback, is disposed. And there is no way for it to detect if the disposable is test-only. There are some huristics to detect a helper method in the testing library, but they are not applicable here.

The focusNode is created inside the test's callback and never disposed. So there is a question if it may be the tested feature created forgot to dispose a disposable.

Does this help?

So you're saying there's some kind of tracker that's identifying non-leaks as possible leaks?

If so, isn't the better answer to improve that tracker?

Did you look at the specific changes that you made? Were there any actual leaks, or were you just making changes so that the inspector stopped complaining?

Ah... Approved before seeing your comment @matthew-carroll. I approved because leak or not I personally think it's good practice to dispose of disposables.

I approved because leak or not I personally think it's good practice to dispose of disposables.

I think the question is - is there actually an opportunity for leaks? I don't generally think it's a good practice to add code that isn't needed and doesn't solve anything. Otherwise, that code tends to become untouchable because nobody is quite sure why it exists.

So I'm still trying to get an answer to my originally question. Does anyone know if there's actually a single leak in this code?

auto label is removed for flutter/flutter/150817, due to - The status or check suite Linux customer_testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

I approved because leak or not I personally think it's good practice to dispose of disposables.

I think the question is - is there actually an opportunity for leaks? I don't generally think it's a good practice to add code that isn't needed and doesn't solve anything. Otherwise, that code tends to become untouchable because nobody is quite sure why it exists.

So I'm still trying to get an answer to my originally question. Does anyone know if there's actually a single leak in this code?

The answer depends on definition of a leak.
For me it is clear why this code exist. There is de-facto rule: if a piece of code creates a disposable object, it should take care about disposing it.

And yes, there is a tool that auto-detects not-disposed objects.

The answer depends on definition of a leak.
For me it is clear why this code exist. There is de-facto rule: if a piece of code creates disposable object, it should take care about disposing it.

If you're enforcing a standard policy, and you want to keep enforcing it, that's up to you. But no, the definition of a leak isn't amorphous. Either dangling references exist in memory after the test completes, or they don't. Either there is a leak, or there isn't. I haven't seen any comment so far that identifies an actual leak. All I've heard is that there's some tool which automatically flags any objects with a dispose() method for which the dispose() method is not explicitly called in a test. That does not imply a leak.

And yes, there is a tool that auto-detect not-disposed objects.

I would suggest considering an improvement to that tool so that it doesn't refer to a bunch of non-leaks as leaks. And/or I would include a written policy about how every test is expected to work with disposables. If that policy already exists, please feel free to link that here.

The answer depends on definition of a leak.
For me it is clear why this code exist. There is de-facto rule: if a piece of code creates disposable object, it should take care about disposing it.

If you're enforcing a standard policy, and you want to keep enforcing it, that's up to you. But no, the definition of a leak isn't amorphous. Either dangling references exist in memory after the test completes, or they don't. Either there is a leak, or there isn't. I haven't seen any comment so far that identifies an actual leak. All I've heard is that there's some tool which automatically flags any objects with a dispose() method for which the dispose() method is not explicitly called in a test. That does not imply a leak.

And yes, there is a tool that auto-detect not-disposed objects.

I would suggest considering an improvement to that tool so that it doesn't refer to a bunch of non-leaks as leaks. And/or I would include a written policy about how every test is expected to work with disposables. If that policy already exists, please feel free to link that here.

Yes, leak tracker catches not leaks, but risks of a leak, we are just shortening the phrasing. Even if a test is not leaking, but some objects are not disposed, there can be code paths that create actual leaks.

Leak tracking is configured in the testing root:

For now leak tracking is not blocking. When it gets blocking, failures will give link to the tool with explanation: https://github.com/dart-lang/leak_tracker/blob/main/doc/leak_tracking/OVERVIEW.md