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.

Sign up for GitHub

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

Jump to bottom

Enable the whole logcat log #143119

Merged
merged 2 commits into from
Feb 8, 2024
Merged

Enable the whole logcat log #143119

merged 2 commits into from
Feb 8, 2024

Conversation

keyonghan
Copy link
Contributor

Help with #142572.

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@keyonghan keyonghan marked this pull request as draft February 7, 2024 21:47
@keyonghan keyonghan mentioned this pull request Feb 7, 2024
Closed
@keyonghan keyonghan marked this pull request as ready for review February 7, 2024 23:58
matanlurey
matanlurey requested changes Feb 8, 2024
View reviewed changes
Copy link
Contributor

@matanlurey matanlurey left a comment

Choose a reason for hiding this comment

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

There is a test folder that includes commands that run against a fake ADB, is there a reason this can't be tested?

dev/devicelab/lib/framework/devices.dart
@@ -748,7 +748,7 @@ class AndroidDevice extends Device {
// For local testing, this can just be:
// <String>['-s', deviceId, 'logcat']
// to view the whole log, or just run logcat alongside this.
<String>['-s', deviceId, 'logcat', 'ActivityManager:I', 'flutter:V', '*:F'],
<String>['-s', deviceId, 'logcat'],
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the entire comment stream above stale now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! Updated.

@keyonghan
Copy link
Contributor Author

There is a test folder that includes commands that run against a fake ADB, is there a reason this can't be tested?

This is simply an adb command change (less args). Not changing any logics but include more logs in the existing log file.
I can try adding a mock process manager and a mock process, mocking that the process does get executed. But I am not sure if we are really testing any logic. Please advise if you think we need to test from a different perspective.

@matanlurey
Copy link
Contributor

Ok, I'm convinced. LGTM and test-exempt.

@keyonghan keyonghan added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 8, 2024
@auto-submit auto-submit bot merged commit e64a372 into flutter:master Feb 8, 2024
133 checks passed
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 9, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 9, 2024
@engine-flutter-autoroll
e64a372d6 Enable the whole logcat log (flutter/flutter#143119)
1fdf23a
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 9, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 9, 2024
@engine-flutter-autoroll
e64a372d6 Enable the whole logcat log (flutter/flutter#143119)
df0be98
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 9, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 9, 2024
@engine-flutter-autoroll
e64a372d6 Enable the whole logcat log (flutter/flutter#143119)
1753c0f
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 9, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 9, 2024
@engine-flutter-autoroll
e64a372d6 Enable the whole logcat log (flutter/flutter#143119)
13da6fb
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 9, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 9, 2024
@engine-flutter-autoroll
e64a372d6 Enable the whole logcat log (flutter/flutter#143119)
4703bb3
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 9, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 9, 2024
@engine-flutter-autoroll
e64a372d6 Enable the whole logcat log (flutter/flutter#143119)
12177fc
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 9, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 9, 2024
@engine-flutter-autoroll
e64a372d6 Enable the whole logcat log (flutter/flutter#143119)
a891b4a
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 10, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 10, 2024
@engine-flutter-autoroll
e64a372d6 Enable the whole logcat log (flutter/flutter#143119)
8dfe9d4
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 10, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 10, 2024
@engine-flutter-autoroll
e64a372d6 Enable the whole logcat log (flutter/flutter#143119)
5310832
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 10, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 10, 2024
@engine-flutter-autoroll
e64a372d6 Enable the whole logcat log (flutter/flutter#143119)
3facb00
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 10, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 10, 2024
@engine-flutter-autoroll
e64a372d6 Enable the whole logcat log (flutter/flutter#143119)
9c0f7c1
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 11, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 11, 2024
@engine-flutter-autoroll
e64a372d6 Enable the whole logcat log (flutter/flutter#143119)
0aad39a
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 11, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 11, 2024
@engine-flutter-autoroll
e64a372d6 Enable the whole logcat log (flutter/flutter#143119)
a483278
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2024
@engine-flutter-autoroll
e64a372d6 Enable the whole logcat log (flutter/flutter#143119)
328c17c
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2024
@engine-flutter-autoroll
e64a372d6 Enable the whole logcat log (flutter/flutter#143119)
c77a4fc
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2024
@engine-flutter-autoroll
e64a372d6 Enable the whole logcat log (flutter/flutter#143119)
f86ce99
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2024
@engine-flutter-autoroll
e64a372d6 Enable the whole logcat log (flutter/flutter#143119)
827783a
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2024
@engine-flutter-autoroll
e64a372d6 Enable the whole logcat log (flutter/flutter#143119)
b6d78ee
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2024
@engine-flutter-autoroll
e64a372d6 Enable the whole logcat log (flutter/flutter#143119)
d5cc766
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2024
@engine-flutter-autoroll
e64a372d6 Enable the whole logcat log (flutter/flutter#143119)
5e212d1
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2024
@engine-flutter-autoroll
e64a372d6 Enable the whole logcat log (flutter/flutter#143119)
f2d6f1e
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2024
@engine-flutter-autoroll
e64a372d6 Enable the whole logcat log (flutter/flutter#143119)
f4098eb
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2024
@engine-flutter-autoroll
e64a372d6 Enable the whole logcat log (flutter/flutter#143119)
f5506af
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2024
@engine-flutter-autoroll
e64a372d6 Enable the whole logcat log (flutter/flutter#143119)
d1c41a2
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2024
@engine-flutter-autoroll
e64a372d6 Enable the whole logcat log (flutter/flutter#143119)
c617619
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 14, 2024
@engine-flutter-autoroll
e64a372d6 Enable the whole logcat log (flutter/flutter#143119)
09d20e3
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 14, 2024
@engine-flutter-autoroll
e64a372d6 Enable the whole logcat log (flutter/flutter#143119)
5ee13f6
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 14, 2024
@engine-flutter-autoroll
e64a372d6 Enable the whole logcat log (flutter/flutter#143119)
2ffc172
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 14, 2024
@engine-flutter-autoroll
e64a372d6 Enable the whole logcat log (flutter/flutter#143119)
9654e73
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matanlurey matanlurey matanlurey approved these changes

@zanderso zanderso zanderso approved these changes

Assignees
No one assigned
Labels
autosubmit Merge PR when tree becomes green via auto submit App
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@keyonghan @matanlurey @zanderso