Stop serving Observatory by default by bkonyi · Pull Request #122419 · flutter/flutter

bkonyi · 2023-03-10T19:40:11Z

Observatory can still be enabled by providing --serve-observatory or invoking the _serveObservatory private service RPC via web socket or HTTP.

Related to dart-lang/sdk#50233

Observatory can still be enabled by providing `--serve-observatory` or invoking the `_serveObservatory` private service RPC via web socket or HTTP. Related to dart-lang/sdk#50233

zanderso · 2023-03-10T19:46:31Z

The test failures look related.

bkonyi · 2023-03-13T17:16:28Z

Test failures should be resolved now.

christopherfujino · 2023-03-13T22:12:02Z

packages/flutter_tools/test/integration.shard/flutter_attach_test.dart

-      await flutterRun.run(withDebugger: true, serveObservatory: false);
+      await flutterRun.run(withDebugger: true);
      // Bail out if Observatory is still served by default in the VM.
      if (await isObservatoryAvailable()) {


is observatory still served by default by the vm?

No, it's not.

christopherfujino · 2023-03-13T22:19:30Z

packages/flutter_tools/test/integration.shard/flutter_attach_test.dart

      if (await isObservatoryAvailable()) {
        return;
      }


Can we update this then to:

Suggested change

if (await isObservatoryAvailable()) {

return;

}

expect(await isObservatoryAvailable(), isFalse);

No, since the change in the VM hasn't rolled in yet.

Which change?

It was reverted in dart-lang/sdk@5a8ddc0, which will reland once this change lands.

This reverts commit edead9c. Reason for revert: We have some flutter framework tests that are breaking, "flutter test should respect --serve-observatory". This will apparently be fixed when flutter/flutter#122419 lands. Original change's description: > [ Observatory ] Disable serving Observatory by default > > Observatory can still be enabled by providing `--serve-observatory` or > invoking the `_serveObservatory` private service RPC via web socket or > HTTP. > > Related to #50233 > > TEST=pkg/dartdev/test/commands/run_test > > Change-Id: I89b000e69bb31c91a9a5386fed1ee590cdafa58c > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/287821 > Reviewed-by: Michael Thomsen <[email protected]> > Commit-Queue: Ben Konyi <[email protected]> > Reviewed-by: Siva Annamalai <[email protected]> Change-Id: I994c86cbca9d0eb25e1f9adddeede94c227acad9 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/288601 Commit-Queue: Siva Annamalai <[email protected]> Bot-Commit: Rubber Stamper <[email protected]> Reviewed-by: Zach Anderson <[email protected]>

christopherfujino

LGTM

zanderso · 2023-03-14T18:18:38Z

packages/flutter_tools/test/integration.shard/flutter_attach_test.dart

        expect(await isObservatoryAvailable(), true);
    });

    testWithoutContext('enables Observatory on attach', () async {


Should this test have a new name?

No, this test was named this way in anticipation of Observatory being disabled by default.

zanderso · 2023-03-23T17:15:30Z

The presub runs on this were a week old, I think? I would have suggested re-running them. Let's keep an eye on how this does on the tree.

* cbdee52 Roll Flutter Engine from 59acb5362098 to 12c822327825 (3 revisions) (flutter/flutter#123330) * 897e3db Inject the gstatic CanvasKit CDN URL by default in `flutter build web` (flutter/flutter#122772) * 5a36bdd Stop serving Observatory by default (flutter/flutter#122419) * b212e7b implement Iterator and Comparable instead of extending them (flutter/flutter#123282) * af6029c c0fbe5a53 Roll Dart SDK from 9256fffbd5af to e8e045620234 (1 revision) (flutter/engine#40561) (flutter/flutter#123336) * 9dec4fb FIX: NavigationDrawer hover/focus/pressed does not use indicatorShape (flutter/flutter#123325) * 4907464 20ab040cd Roll Skia from ce5ff5cc03ce to c42320d53714 (2 revisions) (flutter/engine#40565) (flutter/flutter#123340) * 28d40a4 Roll Packages from 75491e9 to 0826798 (5 revisions) (flutter/flutter#123342) * 674ff15 [macOS] Add platform_channel sample/test (flutter/flutter#123141) * 7b7af9f roll packages (flutter/flutter#123339) * 3179875 replace some ._() constructors with class modifiers (flutter/flutter#122765) * 7f41ab2 Fix (insert|move|remove)RenderObjectChild methods in base class (flutter/flutter#123276) * fccca49 Refactor buildOverscrollIndicator (flutter/flutter#123246) * 11bbce1 6b0933e74 [web] Add `dart:js_interop` to `_embedder.yaml`. (flutter/engine#40545) (flutter/flutter#123347) * 716d252 Remove prefer_const_constructors ignores (flutter/flutter#123284) * 100cf21 Prefer enum over class. (flutter/flutter#123312) * 5ef9b84 Expose toggle to textfield's opacity animation. (flutter/flutter#122474) * d79f3aa Roll Flutter Engine from 6b0933e74965 to bdce896fb64f (2 revisions) (flutter/flutter#123351)

This reverts commit 5a8ddc0. Reason for reland: fix for failing Flutter test landed upstream in flutter/flutter#122419 TEST=pkg/dartdev/test/commands/run_test.dart Change-Id: I1152296828428e118ccba11025f25f6b1dbbb0f3 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/290921 Reviewed-by: Zach Anderson <[email protected]> Commit-Queue: Ben Konyi <[email protected]>

This reverts commit fb1516c. Reason for revert: flutter/flutter#123516 Original change's description: > Reland "[ Observatory ] Disable serving Observatory by default" > > This reverts commit 5a8ddc0. > > Reason for reland: fix for failing Flutter test landed upstream > in flutter/flutter#122419 > > TEST=pkg/dartdev/test/commands/run_test.dart > > Change-Id: I1152296828428e118ccba11025f25f6b1dbbb0f3 > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/290921 > Reviewed-by: Zach Anderson <[email protected]> > Commit-Queue: Ben Konyi <[email protected]> Change-Id: I4e35f93ef4ac46c6dbd905903496a27107eb8329 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/291180 Bot-Commit: Rubber Stamper <[email protected]> Commit-Queue: Alexander Markov <[email protected]> Reviewed-by: Alexander Markov <[email protected]> Reviewed-by: Ben Konyi <[email protected]>

) * cbdee52 Roll Flutter Engine from 59acb5362098 to 12c822327825 (3 revisions) (flutter/flutter#123330) * 897e3db Inject the gstatic CanvasKit CDN URL by default in `flutter build web` (flutter/flutter#122772) * 5a36bdd Stop serving Observatory by default (flutter/flutter#122419) * b212e7b implement Iterator and Comparable instead of extending them (flutter/flutter#123282) * af6029c c0fbe5a53 Roll Dart SDK from 9256fffbd5af to e8e045620234 (1 revision) (flutter/engine#40561) (flutter/flutter#123336) * 9dec4fb FIX: NavigationDrawer hover/focus/pressed does not use indicatorShape (flutter/flutter#123325) * 4907464 20ab040cd Roll Skia from ce5ff5cc03ce to c42320d53714 (2 revisions) (flutter/engine#40565) (flutter/flutter#123340) * 28d40a4 Roll Packages from 75491e9 to 0826798 (5 revisions) (flutter/flutter#123342) * 674ff15 [macOS] Add platform_channel sample/test (flutter/flutter#123141) * 7b7af9f roll packages (flutter/flutter#123339) * 3179875 replace some ._() constructors with class modifiers (flutter/flutter#122765) * 7f41ab2 Fix (insert|move|remove)RenderObjectChild methods in base class (flutter/flutter#123276) * fccca49 Refactor buildOverscrollIndicator (flutter/flutter#123246) * 11bbce1 6b0933e74 [web] Add `dart:js_interop` to `_embedder.yaml`. (flutter/engine#40545) (flutter/flutter#123347) * 716d252 Remove prefer_const_constructors ignores (flutter/flutter#123284) * 100cf21 Prefer enum over class. (flutter/flutter#123312) * 5ef9b84 Expose toggle to textfield's opacity animation. (flutter/flutter#122474) * d79f3aa Roll Flutter Engine from 6b0933e74965 to bdce896fb64f (2 revisions) (flutter/flutter#123351)

Stop serving Observatory by default

4046b2a

Observatory can still be enabled by providing `--serve-observatory` or invoking the `_serveObservatory` private service RPC via web socket or HTTP. Related to dart-lang/sdk#50233

bkonyi requested review from christopherfujino and zanderso March 10, 2023 19:40

flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Mar 10, 2023

bkonyi added 2 commits March 13, 2023 12:28

Fix tests and analysis

c629973

Merge branch 'master' into disable_observatory

f89134a

Merge branch 'master' into disable_observatory

de5e95d

christopherfujino reviewed Mar 13, 2023

View reviewed changes

christopherfujino approved these changes Mar 14, 2023

View reviewed changes

Update flutter_attach_test.dart

81fec40

zanderso reviewed Mar 14, 2023

View reviewed changes

Merge branch 'master' into disable_observatory

e358ee7

bkonyi merged commit 5a36bdd into master Mar 23, 2023

bkonyi deleted the disable_observatory branch March 23, 2023 16:52

engine-flutter-autoroll mentioned this pull request Mar 23, 2023

Roll Flutter from 250a6c294264 to af6029c8fe5f (5 revisions) flutter/packages#3532

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 23, 2023

5a36bdd Stop serving Observatory by default (flutter/flutter#122419)

508dbdc

engine-flutter-autoroll mentioned this pull request Mar 23, 2023

Roll Flutter from 250a6c294264 to 28d40a4d2f87 (8 revisions) flutter/packages#3534

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 23, 2023

5a36bdd Stop serving Observatory by default (flutter/flutter#122419)

f2e046b

engine-flutter-autoroll mentioned this pull request Mar 23, 2023

Manual roll Flutter from 250a6c294264 to d79f3aab0fa8 (18 revisions) flutter/packages#3535

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 23, 2023

5a36bdd Stop serving Observatory by default (flutter/flutter#122419)

9cb97e8

This was referenced Mar 28, 2023

i123643 print java version gradle #123644

Merged

Add Java-Gradle-AGP validation to flutter analyze #123916

Merged

reidbaker mentioned this pull request Apr 5, 2023

Update getGradleVersion to ignore commented out lines #124260

Merged

8 tasks

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023

5a36bdd Stop serving Observatory by default (flutter/flutter#122419)

55b3f1d

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023

5a36bdd Stop serving Observatory by default (flutter/flutter#122419)

4c94379

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Stop serving Observatory by default#122419

Stop serving Observatory by default#122419
bkonyi merged 6 commits intomasterfrom
disable_observatory

bkonyi commented Mar 10, 2023

Uh oh!

zanderso commented Mar 10, 2023

Uh oh!

bkonyi commented Mar 13, 2023

Uh oh!

christopherfujino Mar 13, 2023

Uh oh!

bkonyi Mar 13, 2023

Uh oh!

christopherfujino Mar 13, 2023

Uh oh!

bkonyi Mar 13, 2023

Uh oh!

christopherfujino Mar 13, 2023

Uh oh!

bkonyi Mar 14, 2023

Uh oh!

christopherfujino left a comment

Uh oh!

zanderso Mar 14, 2023

Uh oh!

bkonyi Mar 14, 2023

Uh oh!

zanderso commented Mar 23, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

bkonyi commented Mar 10, 2023

Uh oh!

zanderso commented Mar 10, 2023

Uh oh!

bkonyi commented Mar 13, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

christopherfujino left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zanderso commented Mar 23, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants