[tool] Add initial file-based command skipping #8928
Conversation
|
(Closing was me pressing the wrong button; this is still ready for review.) |
| bool shouldIgnoreFile(String path) { | ||
| return repoLevelNonCodeImpactingFiles.contains(path) || | ||
| // Native code, which is not part of Dart analysis. | ||
| path.endsWith('.c') || |
There was a problem hiding this comment.
nit: I think this could also be a list named something like sourceFileExtensions.
There was a problem hiding this comment.
Good idea; extracted into a helper called isNativeCodeFile. I also changed the repoLevelNonCodeImpactingFiles list into a helper method for consistency, so everything can just call a set of helpers without it mattering whether it's an exact-match list or a more complicated pattern.
| path.endsWith('.java') || | ||
| path.endsWith('.kt') || | ||
| // Package metadata that doesn't impact analysis. | ||
| path.endsWith('/AUTHORS') || |
There was a problem hiding this comment.
nit: Same for here. Its repeated in other commands, so this could be list like packageMetadataFiles.
There was a problem hiding this comment.
I called it isPackageSupportFile; "metadata" to me suggests it would include pubspec.yaml, but pubspec.yaml can affect a bunch of things, including Dart analysis, so I don't generally want to include it.
| 'foo.h', | ||
| ]; | ||
| for (final String file in files) { | ||
| test('skips command for changes to non-Dart source $file', () async { |
There was a problem hiding this comment.
I haven't seen running test() in a for loop before. Does this pattern work well in IDEs? For example if I try to rerun a failing test, it typically passes the name of the test to flutter test.
Also, this include the $, which I believe doesn't work well with Visual Studio I think.
Same for the command tests below.
There was a problem hiding this comment.
I'm pretty sure we do it in a few other places already. VS Code supports it well; it looks like this before you run it:
and then this after:
For example if I try to rerun a failing test, it typically passes the name of the test to flutter test.
Re-running individual entries from the IDE works. (IIRC, the way Dart test works is that it runs through the whole program and handles all test calls by collecting the names they are called with, and then running tests uses that collected set, so if you pass a name matching a generated name, by the time it tries to do the match the generated name is guaranteed to exist.)
Also, this include the
$, which I believe doesn't work well with Visual Studio I think.
The problem we had was with group ('test $SomeClass'), which confused the tree in VS. I'm not sure if that still happens or has been fixed, but it also wasn't buying us anything because we didn't actually want a dynamic name, unlike the loop case.
|
stuartmorgan-g
added
the
autosubmit
auto-submit
bot
removed
the
autosubmit
|
autosubmit label was removed for flutter/packages/8928, because - The status or check suite Linux_android_legacy android_platform_tests_legacy_api_shard_1 master has failed. Please fix the issues identified (or deflake) before re-applying this label. |
stuartmorgan-g
added
the
autosubmit
flutter/packages@2fcc403...ac21f53 2025-04-20 [email protected] Roll Flutter from 3ed38e2 to cfb887c (17 revisions) (flutter/packages#9118) 2025-04-19 [email protected] [various] Scrubs pre-SDK-21 Android code (flutter/packages#9112) 2025-04-18 [email protected] Roll Flutter from ecabb1a to 3ed38e2 (23 revisions) (flutter/packages#9114) 2025-04-18 [email protected] [flutter_svg] feat: Expose the `colorMapper` property in `SvgPicture` (flutter/packages#9043) 2025-04-18 [email protected] [tool] Add initial file-based command skipping (flutter/packages#8928) 2025-04-18 [email protected] [pigeon] Convert test plugins to SPM (flutter/packages#9105) 2025-04-18 [email protected] [webview_flutter] Adds support to control overscrolling (flutter/packages#8451) 2025-04-17 [email protected] [in_app_purchase] add Storefront.countryCode() and AppStore.sync() (flutter/packages#8900) 2025-04-17 [email protected] [webview_flutter_wkwebview] Expose the allowsLinkPreview property in WKWebView for iOS (flutter/packages#5029) 2025-04-17 [email protected] [webview_flutter_android][webview_flutter_wkwebview] Adds platform implementations to set over-scroll mode (flutter/packages#9101) 2025-04-17 [email protected] [shared_preferences] Update AGP to 8.9.1 (flutter/packages#9106) 2025-04-17 [email protected] [pigeon] Adds Kotlin lint tests to example code and fix lints (flutter/packages#9034) 2025-04-17 [email protected] [video_player_avfoundation] enable more than 30 fps (flutter/packages#7466) 2025-04-17 [email protected] Roll Flutter from aef4718 to ecabb1a (25 revisions) (flutter/packages#9104) 2025-04-16 [email protected] [pigeon] Unify iOS and macOS test plugins (flutter/packages#9100) 2025-04-16 [email protected] Roll Flutter from db68c95 to aef4718 (7 revisions) (flutter/packages#9098) 2025-04-16 [email protected] [webview_flutter_platform_interface] Adds method to set overscroll mode (flutter/packages#9099) 2025-04-16 [email protected] Update `CODEOWNERS` (flutter/packages#8984) 2025-04-16 [email protected] [google_sign_is] Update iOS SDK to 8.0 (flutter/packages#9081) 2025-04-16 [email protected] [camera_avfoundation] Implementation swift migration (flutter/packages#8988) 2025-04-16 [email protected] [go_router] Adds `caseSensitive` to `GoRoute` (flutter/packages#8992) 2025-04-16 [email protected] Manual roll Flutter from 30e53b0 to db68c95 (98 revisions) (flutter/packages#9092) 2025-04-15 [email protected] [tool] Run config-only build for iOS/macOS native-test (flutter/packages#9080) 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-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: 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
flutter/packages@2fcc403...ac21f53 2025-04-20 [email protected] Roll Flutter from 409a8ac to cd51fa3 (17 revisions) (flutter/packages#9118) 2025-04-19 [email protected] [various] Scrubs pre-SDK-21 Android code (flutter/packages#9112) 2025-04-18 [email protected] Roll Flutter from d0741df to 409a8ac (23 revisions) (flutter/packages#9114) 2025-04-18 [email protected] [flutter_svg] feat: Expose the `colorMapper` property in `SvgPicture` (flutter/packages#9043) 2025-04-18 [email protected] [tool] Add initial file-based command skipping (flutter/packages#8928) 2025-04-18 [email protected] [pigeon] Convert test plugins to SPM (flutter/packages#9105) 2025-04-18 [email protected] [webview_flutter] Adds support to control overscrolling (flutter/packages#8451) 2025-04-17 [email protected] [in_app_purchase] add Storefront.countryCode() and AppStore.sync() (flutter/packages#8900) 2025-04-17 [email protected] [webview_flutter_wkwebview] Expose the allowsLinkPreview property in WKWebView for iOS (flutter/packages#5029) 2025-04-17 [email protected] [webview_flutter_android][webview_flutter_wkwebview] Adds platform implementations to set over-scroll mode (flutter/packages#9101) 2025-04-17 [email protected] [shared_preferences] Update AGP to 8.9.1 (flutter/packages#9106) 2025-04-17 [email protected] [pigeon] Adds Kotlin lint tests to example code and fix lints (flutter/packages#9034) 2025-04-17 [email protected] [video_player_avfoundation] enable more than 30 fps (flutter/packages#7466) 2025-04-17 [email protected] Roll Flutter from 9616f9c to d0741df (25 revisions) (flutter/packages#9104) 2025-04-16 [email protected] [pigeon] Unify iOS and macOS test plugins (flutter/packages#9100) 2025-04-16 [email protected] Roll Flutter from a7ce7ff to 9616f9c (7 revisions) (flutter/packages#9098) 2025-04-16 [email protected] [webview_flutter_platform_interface] Adds method to set overscroll mode (flutter/packages#9099) 2025-04-16 [email protected] Update `CODEOWNERS` (flutter/packages#8984) 2025-04-16 [email protected] [google_sign_is] Update iOS SDK to 8.0 (flutter/packages#9081) 2025-04-16 [email protected] [camera_avfoundation] Implementation swift migration (flutter/packages#8988) 2025-04-16 [email protected] [go_router] Adds `caseSensitive` to `GoRoute` (flutter/packages#8992) 2025-04-16 [email protected] Manual roll Flutter from f2d54fd to a7ce7ff (98 revisions) (flutter/packages#9092) 2025-04-15 [email protected] [tool] Run config-only build for iOS/macOS native-test (flutter/packages#9080) 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-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: 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
Adds initial file-based filtering. This does not attempt to be comprehensive, just to get some low-hanging fruit, and to create a blueprint for anyone to follow in the future when adding more filtering. I expect that once this is in place, what will happen is that as we notice cases where PRs are hitting slow or flaky tests that they clearly don't need to, we can incrementally improve the filtering on demand.
Fixes flutter/flutter#136394