Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove record/replay/bug report functionality from the tool #45999

Merged
merged 2 commits into from
Dec 3, 2019

Conversation

jonahwilliams
Copy link
Member

@jonahwilliams jonahwilliams commented Dec 3, 2019

Description

This functionality is neither actively used nor sufficiently tested. While record/replay functionality sounds useful, the fact that we haven't put it to any use in the past year suggests that it might not be.

Running with --bug-report or any recording crashes almost immediately with:

#0      FutureReference.value.<anonymous closure> (package:file/src/backends/record_replay/result_reference.dart:79:11)
#1      _rootRunUnary (dart:async/zone.dart:1134:38)
#2      _CustomZone.runUnary (dart:async/zone.dart:1031:19)
#3      _FutureListener.handleValue (dart:async/future_impl.dart:140:18)
#4      Future._propagateToListeners.handleValueCallback (dart:async/future_impl.dart:682:45)
#5      Future._propagateToListeners (dart:async/future_impl.dart:711:32)
#6      Future._completeWithValue (dart:async/future_impl.dart:526:5)
#7      Future._asyncComplete.<anonymous closure> (dart:async/future_impl.dart:556:7)
#8      _rootRun (dart:async/zone.dart:1126:13)
#9      _CustomZone.run (dart:async/zone.dart:1023:19)
#10     _CustomZone.runGuarded (dart:async/zone.dart:925:7)
#11     _CustomZone.bindCallbackGuarded.<anonymous closure> (dart:async/zone.dart:965:23)
#12     _microtaskLoop (dart:async/schedule_microtask.dart:43:21)
#13     _startMicrotaskLoop (dart:async/schedule_microtask.dart:52:5)
#14     _runPendingImmediateCallback (dart:isolate-patch/isolate_patch.dart:118:13)
#15     _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:175:5)

Instead of record/reply, we've gotten significant mileage in the tool out of mocks and fakes for testing

@fluttergithubbot fluttergithubbot added tool Affects the "flutter" command-line tool. See also t: labels. work in progress; do not review labels Dec 3, 2019
Copy link
Member

@zanderso zanderso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am supportive of removing things we don't use/need, but maybe @tvolkert can share an anecdote or two about times this was useful to have around.

@tvolkert
Copy link
Contributor

tvolkert commented Dec 3, 2019

It has indeed proved useful a small handful of times in the past - not for record/replay tests (which ended up being fraught with gotchas), but for user bug reports.

I'd support adding this back in a more fully fleshed out form to support a bug report capture in the flutter tool, but I agree that the current code has bit rotted to the point that it should be removed.

@jonahwilliams
Copy link
Member Author

Filled #46018 to track

@jonahwilliams jonahwilliams merged commit b96d818 into flutter:master Dec 3, 2019
@jonahwilliams jonahwilliams deleted the remove_record_replay branch December 3, 2019 21:24
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
tool Affects the "flutter" command-line tool. See also t: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants