Skip to content

Conversation

@jmagman
Copy link
Member

@jmagman jmagman commented Oct 31, 2019

Description

IOSDeviceLogReader observes Stdout stream from VMService if version is iOS 13+ to print dart and engine logs. Keep using syslog on older versions since that will additionally provide logs if the app crashes.

Similar to resident_web_runner behavior:
https://github.com/flutter/flutter/blob/master/packages/flutter_tools/lib/src/build_runner/resident_web_runner.dart#L295

Related Issues

Fixes #41133.

Tests

New resident_runner_test and devices_test.

Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I signed the [CLA].
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

  • Yes, this is a breaking change (Please read [Handling breaking changes]). Replace this with a link to the e-mail where you asked for input on this proposed change.
  • No, this is not a breaking change.

@jmagman jmagman added platform-ios iOS applications specifically tool Affects the "flutter" command-line tool. See also t: labels. a: debugging Debugging, breakpoints, expression evaluation labels Oct 31, 2019
Copy link
Member Author

Choose a reason for hiding this comment

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

This needs a test, working on it...

Copy link
Contributor

Choose a reason for hiding this comment

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

won't this fail if device.majorSdkVersion == null?

Copy link
Member Author

Choose a reason for hiding this comment

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

🤦‍♀

Copy link
Contributor

Choose a reason for hiding this comment

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

NNBD soon!

Copy link
Member Author

@jmagman jmagman Oct 31, 2019

Choose a reason for hiding this comment

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

This part is a little over-engineered in the dance between _linesController starting and setting connectedVMServices. I'll simplify.

Copy link
Contributor

Choose a reason for hiding this comment

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

wow, does this verify that an assignment happened? I didn't know you could do that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems to. From the mockito docs:

// You can verify setters.
cat.lives = 9;
verify(cat.lives=9);

Copy link
Contributor

Choose a reason for hiding this comment

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

I've skimmed that README many times and yet seem to have missed that. Cool.

@jmagman
Copy link
Member Author

jmagman commented Oct 31, 2019

This doesn't fix the driver tests, that's a DIFFERENT VM service interface.
☹️

Copy link
Contributor

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

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

LGTM

@jmagman jmagman merged commit 2e7d913 into flutter:master Nov 1, 2019
@jmagman jmagman deleted the logging branch November 1, 2019 21:37
Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Nov 26, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

a: debugging Debugging, breakpoints, expression evaluation platform-ios iOS applications specifically tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No exceptions or logs shown in flutter run on physical iOS 13 phone

4 participants