Skip to content

Conversation

@camsim99
Copy link
Contributor

@camsim99 camsim99 commented Nov 1, 2024

Adds whether or not the plugins in .flutter-plugins-dependencies are dev dependencies or not (as the dev_dependency attribute).

Fixes #157948.

Pre-launch Checklist

@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Nov 1, 2024
@camsim99
Copy link
Contributor Author

Looks like I need to fix some tests that don't have the dev_dependency attribute as well.

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.

The overall testing approach SGTM, but I think we should be careful about over-testing or using version solving in a fragile way in this integration test. See comments below!

], workingDirectory: tempDir.path);

final File pubspecFile = tempDir.childFile('pubspec.yaml');
expect(pubspecFile.existsSync(), true);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Try using the exists matcher instead (there should be other examples of this in the flutter_tools/test directory, look for package:file_testing I believe?).

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 couldn't find such a matcher. There are convenience methods I've seen but they just call this or similar methods.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh oops I see! I was looking in the wrong place :/


final File pubspecFile = tempDir.childFile('pubspec.yaml');
expect(pubspecFile.existsSync(), true);
String pubspecFileAsString = pubspecFile.readAsStringSync();
Copy link
Contributor

Choose a reason for hiding this comment

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

This might be over-testing for an integration test or fragile if the templates change. I might consider instead shelling out and using flutter pub add, i.e. flutter pub add camera:0.11.0.

The other problem though is I'd be worried that eventually when the current Flutter SDK moves past these versions, this will cause a breakage (cannot resolve X for Y).

A potentially cleaner, but slightly more cumbersome path might be to have the integration test create two (blank) plugins instead, i.e. a plugin_a_real_dependency and plugin_b_dev_dependency; then we could use path: ... dependencies and not worry about version solving in this integration test.

(Let me know if you could use more of an explanation or help doing this)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gotcha this approach simplifies things a lot so thank you! I'll make this change and then see if I'm still having trouble getting the plugins marked as dev dependencies.

@camsim99 camsim99 marked this pull request as ready for review November 14, 2024 15:30
@camsim99 camsim99 requested a review from matanlurey November 14, 2024 15:30
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.

Looks great, love this approach!

], workingDirectory: tempDir.path);

final File pubspecFile = tempDir.childFile('pubspec.yaml');
expect(pubspecFile.existsSync(), true);
Copy link
Contributor

Choose a reason for hiding this comment

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

@camsim99 camsim99 added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 14, 2024
@auto-submit auto-submit bot merged commit 46a4158 into flutter:master Nov 14, 2024
145 checks passed
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 14, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 14, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 14, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 15, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 15, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 15, 2024
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Nov 15, 2024
Roll Flutter from ed553d1 to 0e2d55e (40 revisions)

flutter/flutter@ed553d1...0e2d55e

2024-11-15 [email protected] Roll Packages from b9ac917 to b164be3 (4 revisions) (flutter/flutter#158986)
2024-11-15 [email protected] Label PRs with gradle and Android paths "platform-android" (flutter/flutter#158970)
2024-11-15 [email protected] Fix code asset copying logic in native asset code (flutter/flutter#158984)
2024-11-15 [email protected] Fix duplicate work in native assets release builds (flutter/flutter#158980)
2024-11-15 [email protected] Stop generate both `.kts` and non-`.kts` gradle files for a test project. (flutter/flutter#158965)
2024-11-15 [email protected] Further skip `native_assets_test`(s) that runs `flutter build apk`. (flutter/flutter#158966)
2024-11-15 49699333+dependabot[bot]@users.noreply.github.com Bump actions/checkout from 4.2.1 to 4.2.2 (flutter/flutter#157473)
2024-11-15 [email protected] Try running historically flaky tests first to make `flutter build apk` health tests time out more often? (flutter/flutter#158967)
2024-11-15 [email protected] [ tool ] Don't throw StateError when DDS fails to start (flutter/flutter#158744)
2024-11-14 [email protected] Create merge_queue.md (flutter/flutter#158959)
2024-11-14 [email protected] Marks Mac_x64 hot_mode_dev_cycle_macos_target__benchmark to be flaky (flutter/flutter#158569)
2024-11-14 [email protected] Marks Mac_arm64 mac_desktop_impeller to be unflaky (flutter/flutter#158564)
2024-11-14 [email protected] [gen_l10n] When localizing a message, prefer placeholder definitions defined by the current locale rather than the template locale (flutter/flutter#153459)
2024-11-14 [email protected] feat: Include web 1.x.x in plugin template (flutter/flutter#156947)
2024-11-14 [email protected] hide members where possible (flutter/flutter#158492)
2024-11-14 [email protected] Move platform-specific log-reading implementation details from `ResidentRunner`/`FlutterDevice` to `DeviceLogReader` implementations (flutter/flutter#156181)
2024-11-14 [email protected] Updated document to clarify Clip Behaviour (flutter/flutter#157719)
2024-11-14 [email protected] Enable --verbose for android_plugin_skip_unsupported_test tests (flutter/flutter#158933)
2024-11-14 [email protected] Roll Packages from 26e123a to b9ac917 (5 revisions) (flutter/flutter#158938)
2024-11-14 [email protected] Add `dev_dependency` attribute to plugins in `.flutter-plugins-dependencies` (flutter/flutter#158009)
2024-11-14 [email protected] No longer pass `--verbose` to implicit `pub` calls when `flutter --verbose` is set. (flutter/flutter#158898)
2024-11-14 [email protected] Update triage flow chart for SVG packages (flutter/flutter#158670)
2024-11-14 [email protected] Add one MenuAnchor alignment test (flutter/flutter#158915)
2024-11-14 [email protected] Reland Add test for dynamic_content_color.0.dart (flutter/flutter#158547)
2024-11-14 [email protected] Roll Flutter Engine from e97b148e796d to 619804c0fbb7 (1 revision) (flutter/flutter#158905)
2024-11-14 [email protected] Roll Flutter Engine from 82940a9aa714 to e97b148e796d (1 revision) (flutter/flutter#158901)
2024-11-14 [email protected] Move explicit package dependencies to a feature flag (flutter/flutter#158016)
2024-11-14 [email protected] Try with `bringup: true` debugging why `flutter build apk` often times out. (flutter/flutter#158895)
2024-11-14 [email protected] Add constraint options to `SearchAnchor` suggestions builder (flutter/flutter#148856)
2024-11-14 [email protected] Adjusts the Hindi TimeOfDayFormat to display in a LTR orientation in localizations. (flutter/flutter#157998)
2024-11-14 [email protected] Fix update order of SliverAppBar (flutter/flutter#158159)
2024-11-13 [email protected] #154792 - CupertinoActionSheetAction cursor doesn't change to clickable on desktop (flutter/flutter#158470)
2024-11-13 [email protected] Adds a skip message for analyzer (flutter/flutter#158890)
2024-11-13 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Marks Mac_arm64_ios hot_mode_dev_cycle_ios__benchmark to be flaky (#158242)" (flutter/flutter#158891)
2024-11-13 [email protected] [SwiftPM] Move the logic for SwiftPM enablement to the platform project (flutter/flutter#158213)
2024-11-13 [email protected] Temporarily skip flutter build apk for native_assets tests. (flutter/flutter#158880)
2024-11-13 [email protected] docs: include Human Interface haptic information in HapticFeedback (flutter/flutter#158587)
2024-11-13 [email protected] Roll Flutter Engine from db3e5af2ca22 to 82940a9aa714 (2 revisions) (flutter/flutter#158799)
2024-11-13 [email protected] Stream the output of `flutter build` for debugging. (flutter/flutter#158757)
2024-11-13 [email protected] "Fix failing checks" wiki page for new contributors (flutter/flutter#154629)

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] on the revert to ensure that a human
is aware of the problem.
...
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UsePlugin.isDevDependency to influence .flutter-plugins-dependencies

2 participants