Stop serving Observatory by default #122419

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

Stop serving Observatory by default #122419

Stop serving Observatory by default #122419

Uh oh!

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