Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Provide a subset of adb logcat logs streaming during scenario_app testing #50886

Merged
merged 7 commits into from
Feb 23, 2024

Conversation

matanlurey
Copy link
Contributor

Work towards flutter/flutter#143458.

What this doesn't change:

  1. The full output of adb logcat is still written to $FLUTTER_LOGS_DIR/logcat.txt

Example output:

Screenshot 2024-02-22 at 2 53 51 PM

/cc @dnfield @gaaclarke

// 4. The message (after the colon).
//
// This regex is simple versus being more precise. Feel free to improve it.
static final RegExp _pattern = RegExp(r'([^A-Z]*)([A-Z])\s([^:]*)\:\s(.*)');
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this format documented? Not that this is a requirement, just curious.

Copy link
Contributor

Choose a reason for hiding this comment

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

This looks fine. My only thought would be if this is TOO specific, and thus brittle to changes. But I'm inclined to say LGTM

Copy link
Member

Choose a reason for hiding this comment

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

LGTM too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to make it fairly resilient, but it's not the best

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

// 4. The message (after the colon).
//
// This regex is simple versus being more precise. Feel free to improve it.
static final RegExp _pattern = RegExp(r'([^A-Z]*)([A-Z])\s([^:]*)\:\s(.*)');
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks fine. My only thought would be if this is TOO specific, and thus brittle to changes. But I'm inclined to say LGTM

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.

wait a minute...streams...

Copy link
Member

@gaaclarke gaaclarke left a comment

Choose a reason for hiding this comment

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

lgtm, I'll defer to chris on the safety of the streams. I've been in c++ land so long my dart minutia is rusty.

echo "Exit code: $?"
echo "Done"
# Optionally skip installation if -s is passed.
if [ "$1" != "-s" ]; then
Copy link
Member

Choose a reason for hiding this comment

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

nit: Since we don't have a nice help screen, documenting this at the top in a usage section would be helpful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah sorry, this is a debug only script that's not actually used (I was just using it for my own local hacking)


# Reverse port 3000 to the device.
echo "Reversing port 3000 to the device..."
$ADB reverse tcp:3000 tcp:3000
Copy link
Member

Choose a reason for hiding this comment

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

Where are these coming from? Is this pulling logic from the android_integration_tests.dart, should there be a removal in that script?

@matanlurey matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 23, 2024
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Feb 23, 2024
Copy link
Contributor

auto-submit bot commented Feb 23, 2024

auto label is removed for flutter/engine/50886, due to This PR has not met approval requirements for merging. Changes were requested by {christopherfujino}, please make the needed changes and resubmit this PR.
The PR author is a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.

  • Merge guidelines: A PR needs at least one approved review if the author is already part of flutter-hackers or two member reviews if the author is not a flutter-hacker before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

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

@christopherfujino christopherfujino added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 23, 2024
@auto-submit auto-submit bot merged commit 6a8d888 into flutter:main Feb 23, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 23, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Feb 23, 2024
…144028)

flutter/engine@0f3ad23...5f99a6c

2024-02-23 skia-flutter-autoroll@skia.org Roll Skia from bb8a7832a6f4 to c7f8e0e6f3c2 (3 revisions) (flutter/engine#50911)
2024-02-23 brianosman@google.com Replace SkColorSpace::filterColor with filterColor4f (flutter/engine#50821)
2024-02-23 skia-flutter-autoroll@skia.org Roll Dart SDK from 52111952d91b to 7f6eccd65255 (1 revision) (flutter/engine#50910)
2024-02-23 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from j9cJ94K-T1i3u5xGh... to sO-oG6KoeFlPK2WLR... (flutter/engine#50907)
2024-02-23 bdero@google.com [Impeller] Make StC work for Position+UV buffers. (flutter/engine#50900)
2024-02-23 skia-flutter-autoroll@skia.org Roll Dart SDK from 957130770e7d to 52111952d91b (1 revision) (flutter/engine#50906)
2024-02-23 skia-flutter-autoroll@skia.org Roll Skia from 14b5c1a4627a to bb8a7832a6f4 (1 revision) (flutter/engine#50905)
2024-02-23 skia-flutter-autoroll@skia.org Roll Skia from 5a0a82af7796 to 14b5c1a4627a (1 revision) (flutter/engine#50904)
2024-02-23 skia-flutter-autoroll@skia.org Roll Dart SDK from e2f2d9b464e9 to 957130770e7d (2 revisions) (flutter/engine#50903)
2024-02-23 skia-flutter-autoroll@skia.org Roll Skia from 254b27e90e3c to 5a0a82af7796 (1 revision) (flutter/engine#50902)
2024-02-23 matanlurey@users.noreply.github.com Move ban-plugin-java script into separate file and improve testing. (flutter/engine#50875)
2024-02-23 matanlurey@users.noreply.github.com Provide a subset of `adb logcat` logs streaming during scenario_app testing (flutter/engine#50886)
2024-02-23 54558023+keyonghan@users.noreply.github.com Remove obsolete `cache_root` property (flutter/engine#50894)
2024-02-23 skia-flutter-autoroll@skia.org Roll Skia from 58772db6bc46 to 254b27e90e3c (1 revision) (flutter/engine#50896)
2024-02-23 54558023+keyonghan@users.noreply.github.com Remove unused drone_dimension field (flutter/engine#50893)
2024-02-22 54558023+keyonghan@users.noreply.github.com Remove unused property from `Linux Web Framework tests` (flutter/engine#50891)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from j9cJ94K-T1i3 to sO-oG6KoeFlP

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC jimgraham@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants