Skip to content

Conversation

@zanderso
Copy link
Member

@zanderso zanderso commented Aug 5, 2024

As in #152696, It's not clear that DDS initialization is necessary for any of these tests.

In particular, from the comments on the tool flag:

            'It may be necessary to disable this when attaching to an application with '
            'an existing DDS instance (e.g., attaching to an application currently '
            'connected to by "flutter run"), or when running certain tests.\n'
            'Disabling this feature may degrade IDE functionality if a DDS instance is '
            'not already connected to the target application.'

These tests are unlikely to need IDE functionality. I have initially modified the Linux_android_emu tests in the ci.yaml so that they run in presubmit to verify that this change is safe. Many other postsubmit-only tests that will now be passed --no-dds require physical devices, and if there is an issue in post-submit, this change will need to be reverted.

@zanderso
Copy link
Member Author

zanderso commented Aug 5, 2024

I'll remove the ci.yaml changes before submitting.

Copy link
Member

@jmagman jmagman left a comment

Choose a reason for hiding this comment

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

Would it make more sense to always pass in that flag to drive?

List<String> _flutterCommandArgs(String command, List<String> options) {

@zanderso zanderso force-pushed the no-dds-in-driver-tests branch from 163dfef to 38a8b4d Compare August 5, 2024 23:52
@zanderso
Copy link
Member Author

zanderso commented Aug 5, 2024

Would it make more sense to always pass in that flag to drive?

List<String> _flutterCommandArgs(String command, List<String> options) {

Good idea. Updated to do that.

@zanderso zanderso merged commit fc46534 into flutter:master Aug 6, 2024
@zanderso zanderso deleted the no-dds-in-driver-tests branch August 6, 2024 04:11
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 6, 2024
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Aug 6, 2024
Manual roll requested by [email protected]

flutter/flutter@3832823...1dd7141

2024-08-06 [email protected] Roll Flutter Engine from 376a853846ce to f5f6e966e7e7 (1 revision) (flutter/flutter#152911)
2024-08-06 [email protected] Pass --no-dds to some integration tests driven by flutter drive (flutter/flutter#152898)
2024-08-06 [email protected] Manual dependency bump (flutter/flutter#152881)
2024-08-06 [email protected] Add tests for ordered_traversal_group.0.dart (flutter/flutter#152849)
2024-08-06 [email protected] Implement `on` clauses (flutter/flutter#152706)
2024-08-06 [email protected] Roll Flutter Engine from bc140a0124b7 to 376a853846ce (2 revisions) (flutter/flutter#152905)
2024-08-06 [email protected] Roll Flutter Engine from e073ad2e3ad4 to bc140a0124b7 (1 revision) (flutter/flutter#152895)
2024-08-05 [email protected] added functionality to where SR will communicate button clicked (flutter/flutter#152185)
2024-08-05 [email protected] more docImports (flutter/flutter#151951)
2024-08-05 [email protected] Bump dartdoc to 8.0.13 (flutter/flutter#152896)
2024-08-05 [email protected] [Reland] Introduce `double` `Flex.spacing` parameter for `Row`/`Column` spacing (flutter/flutter#152890)
2024-08-05 [email protected] Fix CarouselView rebuild (flutter/flutter#152791)
2024-08-05 [email protected] Roll Flutter Engine from 1bfd06cda313 to e073ad2e3ad4 (4 revisions) (flutter/flutter#152889)
2024-08-05 [email protected] Add migration to git ignore SwiftPM build directories (flutter/flutter#152766)
2024-08-05 [email protected] Roll pub packages (flutter/flutter#152127)
2024-08-05 [email protected] Roll Flutter Engine from 0aac60342005 to 1bfd06cda313 (2 revisions) (flutter/flutter#152868)
2024-08-05 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Introduce `double` `Flex.spacing` parameter for `Row`/`Column` spacing (#152472)" (flutter/flutter#152885)
2024-08-05 [email protected] [Docs] DeviceOrientation Enum Correction (flutter/flutter#152876)
2024-08-05 [email protected] Move Linux_build_test tests from staging to prod (flutter/flutter#152877)
2024-08-05 [email protected] Introduce `double` `Flex.spacing` parameter for `Row`/`Column` spacing (flutter/flutter#152472)
2024-08-05 [email protected] Mark Linux_android_emu tests bringup: true (flutter/flutter#152867)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: 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
TytaniumDev pushed a commit to TytaniumDev/flutter that referenced this pull request Aug 7, 2024
…ter#152898)

As in flutter#152696, It's not clear
that DDS initialization is necessary for any of these tests.

In particular, from the comments on the tool flag:
```
            'It may be necessary to disable this when attaching to an application with '
            'an existing DDS instance (e.g., attaching to an application currently '
            'connected to by "flutter run"), or when running certain tests.\n'
            'Disabling this feature may degrade IDE functionality if a DDS instance is '
            'not already connected to the target application.'
```

These tests are unlikely to need IDE functionality. I have initially
modified the Linux_android_emu tests in the ci.yaml so that they run in
presubmit to verify that this change is safe. Many other postsubmit-only
tests that will now be passed `--no-dds` require physical devices, and
if there is an issue in post-submit, this change will need to be
reverted.
Buchimi pushed a commit to Buchimi/flutter that referenced this pull request Sep 2, 2024
…ter#152898)

As in flutter#152696, It's not clear
that DDS initialization is necessary for any of these tests.

In particular, from the comments on the tool flag:
```
            'It may be necessary to disable this when attaching to an application with '
            'an existing DDS instance (e.g., attaching to an application currently '
            'connected to by "flutter run"), or when running certain tests.\n'
            'Disabling this feature may degrade IDE functionality if a DDS instance is '
            'not already connected to the target application.'
```

These tests are unlikely to need IDE functionality. I have initially
modified the Linux_android_emu tests in the ci.yaml so that they run in
presubmit to verify that this change is safe. Many other postsubmit-only
tests that will now be passed `--no-dds` require physical devices, and
if there is an issue in post-submit, this change will need to be
reverted.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 11, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants