-
Notifications
You must be signed in to change notification settings - Fork 29.8k
Add dev_dependency attribute to plugins in .flutter-plugins-dependencies
#158009
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Looks like I need to fix some tests that don't have the |
matanlurey
left a comment
There was a problem hiding this 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!
packages/flutter_tools/test/integration.shard/flutter_plugins_dependencies_test.dart
Outdated
Show resolved
Hide resolved
| ], workingDirectory: tempDir.path); | ||
|
|
||
| final File pubspecFile = tempDir.childFile('pubspec.yaml'); | ||
| expect(pubspecFile.existsSync(), true); |
There was a problem hiding this comment.
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?).
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://pub.dev/documentation/file_testing/latest/file_testing/exists-constant.html
No worries, this isn't critical. Thanks!
There was a problem hiding this comment.
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(); |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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.
matanlurey
left a comment
There was a problem hiding this 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); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://pub.dev/documentation/file_testing/latest/file_testing/exists-constant.html
No worries, this isn't critical. Thanks!
…s-dependencies` (flutter/flutter#158009)
…s-dependencies` (flutter/flutter#158009)
…s-dependencies` (flutter/flutter#158009)
…s-dependencies` (flutter/flutter#158009)
…s-dependencies` (flutter/flutter#158009)
Roll Flutter from ed553d1 to 0e2d55e (40 revisions) flutter/flutter@ed553d1...0e2d55e 2024-11-15 engine-flutter-autoroll@skia.org Roll Packages from b9ac917 to b164be3 (4 revisions) (flutter/flutter#158986) 2024-11-15 magder@google.com Label PRs with gradle and Android paths "platform-android" (flutter/flutter#158970) 2024-11-15 kustermann@google.com Fix code asset copying logic in native asset code (flutter/flutter#158984) 2024-11-15 kustermann@google.com Fix duplicate work in native assets release builds (flutter/flutter#158980) 2024-11-15 matanlurey@users.noreply.github.com Stop generate both `.kts` and non-`.kts` gradle files for a test project. (flutter/flutter#158965) 2024-11-15 matanlurey@users.noreply.github.com 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 matanlurey@users.noreply.github.com Try running historically flaky tests first to make `flutter build apk` health tests time out more often? (flutter/flutter#158967) 2024-11-15 bkonyi@google.com [ tool ] Don't throw StateError when DDS fails to start (flutter/flutter#158744) 2024-11-14 katelovett@google.com Create merge_queue.md (flutter/flutter#158959) 2024-11-14 fluttergithubbot@gmail.com Marks Mac_x64 hot_mode_dev_cycle_macos_target__benchmark to be flaky (flutter/flutter#158569) 2024-11-14 fluttergithubbot@gmail.com Marks Mac_arm64 mac_desktop_impeller to be unflaky (flutter/flutter#158564) 2024-11-14 gengesa@gmail.com [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 august.oberhauser@swissinfo.ch feat: Include web 1.x.x in plugin template (flutter/flutter#156947) 2024-11-14 andrewrkolos@gmail.com hide members where possible (flutter/flutter#158492) 2024-11-14 andrewrkolos@gmail.com Move platform-specific log-reading implementation details from `ResidentRunner`/`FlutterDevice` to `DeviceLogReader` implementations (flutter/flutter#156181) 2024-11-14 98221114+Neutrino2711@users.noreply.github.com Updated document to clarify Clip Behaviour (flutter/flutter#157719) 2024-11-14 bkonyi@google.com Enable --verbose for android_plugin_skip_unsupported_test tests (flutter/flutter#158933) 2024-11-14 engine-flutter-autoroll@skia.org Roll Packages from 26e123a to b9ac917 (5 revisions) (flutter/flutter#158938) 2024-11-14 43054281+camsim99@users.noreply.github.com Add `dev_dependency` attribute to plugins in `.flutter-plugins-dependencies` (flutter/flutter#158009) 2024-11-14 matanlurey@users.noreply.github.com No longer pass `--verbose` to implicit `pub` calls when `flutter --verbose` is set. (flutter/flutter#158898) 2024-11-14 stuartmorgan@google.com Update triage flow chart for SVG packages (flutter/flutter#158670) 2024-11-14 bruno.leroux@gmail.com Add one MenuAnchor alignment test (flutter/flutter#158915) 2024-11-14 32538273+ValentinVignal@users.noreply.github.com Reland Add test for dynamic_content_color.0.dart (flutter/flutter#158547) 2024-11-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from e97b148e796d to 619804c0fbb7 (1 revision) (flutter/flutter#158905) 2024-11-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from 82940a9aa714 to e97b148e796d (1 revision) (flutter/flutter#158901) 2024-11-14 matanlurey@users.noreply.github.com Move explicit package dependencies to a feature flag (flutter/flutter#158016) 2024-11-14 matanlurey@users.noreply.github.com Try with `bringup: true` debugging why `flutter build apk` often times out. (flutter/flutter#158895) 2024-11-14 rexios@rexios.dev Add constraint options to `SearchAnchor` suggestions builder (flutter/flutter#148856) 2024-11-14 50643541+Mairramer@users.noreply.github.com Adjusts the Hindi TimeOfDayFormat to display in a LTR orientation in localizations. (flutter/flutter#157998) 2024-11-14 38378650+hgraceb@users.noreply.github.com Fix update order of SliverAppBar (flutter/flutter#158159) 2024-11-13 42980667+srivats22@users.noreply.github.com #154792 - CupertinoActionSheetAction cursor doesn't change to clickable on desktop (flutter/flutter#158470) 2024-11-13 58190796+MitchellGoodwin@users.noreply.github.com 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 737941+loic-sharma@users.noreply.github.com [SwiftPM] Move the logic for SwiftPM enablement to the platform project (flutter/flutter#158213) 2024-11-13 matanlurey@users.noreply.github.com Temporarily skip flutter build apk for native_assets tests. (flutter/flutter#158880) 2024-11-13 me@alestiago.com docs: include Human Interface haptic information in HapticFeedback (flutter/flutter#158587) 2024-11-13 engine-flutter-autoroll@skia.org Roll Flutter Engine from db3e5af2ca22 to 82940a9aa714 (2 revisions) (flutter/flutter#158799) 2024-11-13 matanlurey@users.noreply.github.com Stream the output of `flutter build` for debugging. (flutter/flutter#158757) 2024-11-13 nate.w5687@gmail.com "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 dit@google.com,stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. ...
…s-dependencies` (flutter/flutter#158009)
…s-dependencies` (flutter/flutter#158009)
…s-dependencies` (flutter/flutter#158009)
…s-dependencies` (flutter/flutter#158009)
…s-dependencies` (flutter/flutter#158009)
Adds whether or not the plugins in
.flutter-plugins-dependenciesare dev dependencies or not (as thedev_dependencyattribute).Fixes #157948.
Pre-launch Checklist
///).