-
Notifications
You must be signed in to change notification settings - Fork 3.5k
[tools] Fix OOB test error #4144
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
[tools] Fix OOB test error #4144
Conversation
An update to `args` changed the failure mode for a missing flag; this updates the tests accordingly to fix the tree.
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.
What is different between --changelog and --version?
How come this test is not failing in a similar fashion? fails if --version is missing (L78)
|
Replying to myself, here's the relevant diff:
(According to pub, |
Ah, good catch. It turns out that test is subtly wrong, because changelog is blank, and that's also a failure case. So the test is failing with a |
|
Linux/Window repo_tools_tests fails with args: 2.4.2. Is CI engine uses tools different from flutter/packages main? So i suggest for universal: Error? commandError;
Exception? commandException;
await runCapturingPrint(runner, <String>[
'update-release-info',
'--version=next',
], errorHandler: (Error e) {
commandError = e;
}, exceptionHandler: (Exception e) {
commandException = e;
});
expect(
commandError is ArgumentError || commandException is UsageException,
isTrue,
); |
Can we update the dependency to |
I considered that when writing the PR, but it requires Dart 2.19, so we'd have to drop the 3.3 legacy tests. I was just going to ignore this since we unit test the tools on |
That is what this PR fixes, as discussed in previous comments.
I don't understand the question.
Why? Who is running the flutter/packages repo tools' unit tests with Flutter 3.3 or earlier? |
|
LUCI had some kind of meltdown so everything is red, but I re-ran the |
The only way to deal with out-of-band failures is to wait for them to be fixed (as has now happened), then rebase/merge the latest main.
Yes,
I don't understand how you reached that conclusion, but it is not correct. |
flutter/packages@db4e5c2...da72219 2023-06-06 42692766+dimabran@users.noreply.github.com remove unnecessary typed_data imports (flutter/packages#4136) 2023-06-06 43054281+camsim99@users.noreply.github.com [local_auth_android] Bump androidx.fragment to 1.5.7 and androidx.core to 1.10.1 (flutter/packages#4142) 2023-06-05 ditman@gmail.com [google_maps] Endorses package:google_maps_flutter_web. (flutter/packages#4124) 2023-06-05 ditman@gmail.com [ci] Removes bespoke web scripts. (flutter/packages#4129) 2023-06-05 arteevraina@gmail.com [webview_flutter]: fix typo (flutter/packages#4070) 2023-06-05 49699333+dependabot[bot]@users.noreply.github.com [camera]: Bump com.google.guava:guava from 31.1-android to 32.0.0-android in /packages/camera/camera_android_camerax/android (flutter/packages#4116) 2023-06-05 stuartmorgan@google.com [tools] Fix OOB test error (flutter/packages#4144) 2023-06-05 46547604+dsambuk@users.noreply.github.com [path_provider] Allow win32 up to version 5.x (flutter/packages#4125) 2023-06-05 49699333+dependabot[bot]@users.noreply.github.com [sign_in]: Bump com.google.guava:guava from 31.1-android to 32.0.0-android in /packages/google_sign_in/google_sign_in_android/android (flutter/packages#4112) 2023-06-05 49699333+dependabot[bot]@users.noreply.github.com [video_player]: Bump exoplayer_version from 2.18.6 to 2.18.7 in /packages/video_player/video_player_android/android (flutter/packages#4053) 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 flutter-ecosystem@google.com,rmistry@google.com 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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
An update to
argschanged the failure mode for a missing flag; this updates the tests accordingly to fix the tree.