Use ErrorHandlingFileSystem.deleteIfExists when deleting .plugin_symlinks
#151073
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ErrorHandlingFileSystem.deleteIfExists when deleting .plugin_symlinks
andrewkolos
commented
Jul 1, 2024
| }); | ||
| }); | ||
|
|
||
| testUsingContext('exits tool when deleting .plugin_symlinks fails', () async { |
Contributor
Author
There was a problem hiding this comment.
I'm somewhat concerned about having tests like these. The existence of this test implies that every single file operation throughout the entire tool should have one or more tests for possibility of the operation failing. Do tests like these add more value than they produce noise?
More pointedly, I only wrote this test to satisfy our testing requirement, not because I think it's a good test to have.
Contributor
There was a problem hiding this comment.
The existence of this test implies that every single file operation throughout the entire tool should have one or more tests for possibility of the operation failing. Do tests like these add more value than they produce noise?
I think there's a difference between testing every single operation to verify it behaves as expected and adding a test for a specific operation that was known to be problematic to verify someone doesn't undo it.
That having been said, I don't care that much, and your cache_test update would have made the bot happy.
Contributor
Author
There was a problem hiding this comment.
I think there's a difference between testing every single operation to verify it behaves as expected and adding a test for a specific operation that was known to be problematic to verify someone doesn't undo it.
I suppose.
🤷 I do think the test was worth writing just to verify the change worked, and I therefore think it's useful for future archeologists to verify that the change was tested before being landed. So I think I'll keep writing these.
| "unable to. This may be due to the file and/or project's location on a read-only " | ||
| 'volume. Consider relocating the project and trying again', | ||
| 'Unable to delete file or directory at "${entity.path}". ' | ||
| 'This may be due it and/or the project being in a read-only ' |
Contributor
There was a problem hiding this comment.
s/due it and/or the project/due to the file and/or project
christopherfujino
approved these changes
Jul 1, 2024
Contributor
christopherfujino
left a comment
There was a problem hiding this comment.
nit about awkward grammar in the toolexit, but otherwise LGTM
| // found in the LICENSE file. | ||
|
|
||
| import 'dart:convert'; | ||
| import 'dart:io'; |
Contributor
There was a problem hiding this comment.
nit. do you really need to import dart:io here, or would package:flutter_tools/src/base/io.dart be sufficient?
Contributor
Author
There was a problem hiding this comment.
Tried to fix this locally, but it appears io.dart doesn't expose PathNotFoundException?
Contributor
There was a problem hiding this comment.
Ahh, ok, this LGTM then
flutteractionsbot
pushed a commit
to flutteractionsbot/flutter
that referenced
this pull request
Jul 2, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 2, 2024
hello-coder-xu
added a commit
to hello-coder-xu/flutter
that referenced
this pull request
Jul 3, 2024
* master: (88 commits) Fix scheduler event loop being stuck due to task with Priority.idle (flutter#151168) Fix result propagation in RenderSliverEdgeInsetsPadding.hitTestChildren (flutter#149825) docImports for flutter_test (flutter#151189) Interactable ScrollView content when settling a scroll activity (flutter#145848) [flutter_tools] Update the mapping for the Dart SDK internal URI (flutter#151170) Roll pub packages (flutter#151129) Fix typo (flutter#151192) [tool] Fix `stdin.flush` calls on processes started by `FakeProcessManager` (flutter#151183) Roll Flutter Engine from 433d360 to 4427894 (4 revisions) (flutter#151186) Use `ErrorHandlingFileSystem.deleteIfExists` when deleting .plugin_symlinks (flutter#151073) ScrollEndNotification example: auto-scroll based on RenderSliver constraints and geometry (flutter#143538) Roll Packages from 412ec46 to d2705fb (13 revisions) (flutter#151169) docimports for painting (flutter#151143) docimports for scheduler (flutter#151126) `dismissible.dart` code cleanup (flutter#150276) docimports for physics (flutter#151125) docimports for services (flutter#151134) docimports for cupertino (flutter#151149) docimports for gestures (flutter#151123) Docimports for foundation (flutter#151119) ...
auto-submit bot
pushed a commit
to flutter/packages
that referenced
this pull request
Jul 3, 2024
flutter/flutter@99bb2ff...af913a7 2024-07-02 andrewrkolos@gmail.com Use `ErrorHandlingFileSystem.deleteIfExists` when deleting .plugin_symlinks (flutter/flutter#151073) 2024-07-02 hans.muller@gmail.com ScrollEndNotification example: auto-scroll based on RenderSliver constraints and geometry (flutter/flutter#143538) 2024-07-02 engine-flutter-autoroll@skia.org Roll Packages from 412ec46 to d2705fb (13 revisions) (flutter/flutter#151169) 2024-07-02 goderbauer@google.com docimports for painting (flutter/flutter#151143) 2024-07-02 goderbauer@google.com docimports for scheduler (flutter/flutter#151126) 2024-07-02 nate.w5687@gmail.com `dismissible.dart` code cleanup (flutter/flutter#150276) 2024-07-02 goderbauer@google.com docimports for physics (flutter/flutter#151125) 2024-07-02 goderbauer@google.com docimports for services (flutter/flutter#151134) 2024-07-02 goderbauer@google.com docimports for cupertino (flutter/flutter#151149) 2024-07-02 goderbauer@google.com docimports for gestures (flutter/flutter#151123) 2024-07-02 goderbauer@google.com Docimports for foundation (flutter/flutter#151119) 2024-07-02 goderbauer@google.com docimports for semantics (flutter/flutter#151132) 2024-07-02 jonahwilliams@google.com [flutter_driver] add allocator mtl to memory event allowlist. (flutter/flutter#151153) 2024-07-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from 40c087b31515 to 433d360eee11 (7 revisions) (flutter/flutter#151165) 2024-07-02 sigurdm@google.com Refactor BuildInfo to always require packageConfigPath (flutter/flutter#150559) 2024-07-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from d3c5bd66a78f to 40c087b31515 (1 revision) (flutter/flutter#151156) 2024-07-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from fc5bc14e6091 to d3c5bd66a78f (1 revision) (flutter/flutter#151155) 2024-07-02 dkwingsmt@users.noreply.github.com Fix: `CupertinoActionSheet` should take up max height when actions section is short (flutter/flutter#150708) 2024-07-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from 3456fee1a6b9 to fc5bc14e6091 (8 revisions) (flutter/flutter#151150) 2024-07-02 andrewrkolos@gmail.com [tool] remove some temporary printTrace calls (flutter/flutter#151074) 2024-07-01 nate.w5687@gmail.com Implementing a few switch statements (flutter/flutter#150946) 2024-07-01 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Upgrade template Gradle, App AGP, Module AGP, and Kotlin versions, and tests (#150969)" (flutter/flutter#151147) 2024-07-01 34871572+gmackall@users.noreply.github.com Upgrade template Gradle, App AGP, Module AGP, and Kotlin versions, and tests (flutter/flutter#150969) 2024-07-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from b57a044ed10f to 3456fee1a6b9 (5 revisions) (flutter/flutter#151127) 2024-07-01 codefu@google.com Read AndroidManifest.xml and emit manifest-aar-impeller-(enabled|disabled) analytics (flutter/flutter#150970) 2024-07-01 goderbauer@google.com More docimports for animation library (flutter/flutter#151011) 2024-07-01 goderbauer@google.com Bump dartdoc to 8.0.10 (flutter/flutter#151107) 2024-07-01 matthiasngeozhongsim@yahoo.com Fix missing `[` in docs (flutter/flutter#151091) 2024-07-01 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#151028) 2024-07-01 jason-simmons@users.noreply.github.com Fix teardown of a FocusScopeNode in material/app_test (flutter/flutter#151115) 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 camillesimon@google.com,rmistry@google.com,stuartmorgan@google.com 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
victorsanni
pushed a commit
to victorsanni/flutter
that referenced
this pull request
Jul 8, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 6, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #137168.
Pre-launch Checklist
///).