Reorganize leak tracker for better performance. #106
Conversation
polina-c
changed the title
...leak_tracker_flutter_test/test/end_to_end/leak_tracking_config_test/flutter_test_config.dart
Outdated
Show resolved
Hide resolved
...leak_tracker_flutter_test/test/end_to_end/leak_tracking_config_test/flutter_test_config.dart
Show resolved
Hide resolved
...racker_flutter_test/test/end_to_end/leak_tracking_config_test/leak_tracking_config_test.dart
Outdated
Show resolved
Hide resolved
|
|
||
| /// For these tests `expect` for found leaks happens in flutter_test_config.dart. | ||
| void main() { | ||
| testWidgetsWithLeakTracking(test1TrackingOn, (widgetTester) async { |
There was a problem hiding this comment.
It might be better to have the test names here in strings instead of in variables. That way it will be easier to read what is being tested without needing to jump around.
Example: "starting phases with tracking-on"
There was a problem hiding this comment.
+1, if we're not generating the names programmatically, we should just use string literals in testWidgets(...)
There was a problem hiding this comment.
I need to reference the test name in other places, so I need them to be variables.
I named the variables to mimic their value to make it more readable.
| @@ -96,6 +110,8 @@ abstract class LeakTracking { | |||
| Map<String, dynamic>? context, | |||
| }) { | |||
| assert(() { | |||
| if (phase.isPaused) return true; | |||
There was a problem hiding this comment.
I completely agree with the idea in principle, but most of the examples there are quite a bit more complex than these closures. The return value in these cases is boilerplate, so this just reads weird to me. I'll leave it up to you :)
|
|
||
| /// For these tests `expect` for found leaks happens in flutter_test_config.dart. | ||
| void main() { | ||
| testWidgetsWithLeakTracking(test1TrackingOn, (widgetTester) async { |
There was a problem hiding this comment.
+1, if we're not generating the names programmatically, we should just use string literals in testWidgets(...)
This PR introduces notion of phase of leak tracking. Different phases may have different names and settings.
testWidgetsWithLeakTrackingnow:tearDownAllto collect leaks for all tests and the throw exception if leaks are foundAlso this PR removes APIs that are not used in Flutter Framework.
As result, leak tracking stopped being noticeable from performance perspective.