Use ErrorHandlingFileSystem.deleteIfExists when deleting .plugin_symlinks#151073
Merged
auto-submit[bot] merged 4 commits intoflutter:masterfrom Jul 2, 2024
Merged
Conversation
andrewkolos
added
the
tool
andrewkolos
changed the title
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
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
andrewkolos
added
the
autosubmit
andrewkolos
added
the
cp: stable
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 [email protected] Use `ErrorHandlingFileSystem.deleteIfExists` when deleting .plugin_symlinks (flutter/flutter#151073) 2024-07-02 [email protected] ScrollEndNotification example: auto-scroll based on RenderSliver constraints and geometry (flutter/flutter#143538) 2024-07-02 [email protected] Roll Packages from 412ec46 to d2705fb (13 revisions) (flutter/flutter#151169) 2024-07-02 [email protected] docimports for painting (flutter/flutter#151143) 2024-07-02 [email protected] docimports for scheduler (flutter/flutter#151126) 2024-07-02 [email protected] `dismissible.dart` code cleanup (flutter/flutter#150276) 2024-07-02 [email protected] docimports for physics (flutter/flutter#151125) 2024-07-02 [email protected] docimports for services (flutter/flutter#151134) 2024-07-02 [email protected] docimports for cupertino (flutter/flutter#151149) 2024-07-02 [email protected] docimports for gestures (flutter/flutter#151123) 2024-07-02 [email protected] Docimports for foundation (flutter/flutter#151119) 2024-07-02 [email protected] docimports for semantics (flutter/flutter#151132) 2024-07-02 [email protected] [flutter_driver] add allocator mtl to memory event allowlist. (flutter/flutter#151153) 2024-07-02 [email protected] Roll Flutter Engine from 40c087b31515 to 433d360eee11 (7 revisions) (flutter/flutter#151165) 2024-07-02 [email protected] Refactor BuildInfo to always require packageConfigPath (flutter/flutter#150559) 2024-07-02 [email protected] Roll Flutter Engine from d3c5bd66a78f to 40c087b31515 (1 revision) (flutter/flutter#151156) 2024-07-02 [email protected] Roll Flutter Engine from fc5bc14e6091 to d3c5bd66a78f (1 revision) (flutter/flutter#151155) 2024-07-02 [email protected] Fix: `CupertinoActionSheet` should take up max height when actions section is short (flutter/flutter#150708) 2024-07-02 [email protected] Roll Flutter Engine from 3456fee1a6b9 to fc5bc14e6091 (8 revisions) (flutter/flutter#151150) 2024-07-02 [email protected] [tool] remove some temporary printTrace calls (flutter/flutter#151074) 2024-07-01 [email protected] 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 [email protected] Upgrade template Gradle, App AGP, Module AGP, and Kotlin versions, and tests (flutter/flutter#150969) 2024-07-01 [email protected] Roll Flutter Engine from b57a044ed10f to 3456fee1a6b9 (5 revisions) (flutter/flutter#151127) 2024-07-01 [email protected] Read AndroidManifest.xml and emit manifest-aar-impeller-(enabled|disabled) analytics (flutter/flutter#150970) 2024-07-01 [email protected] More docimports for animation library (flutter/flutter#151011) 2024-07-01 [email protected] Bump dartdoc to 8.0.10 (flutter/flutter#151107) 2024-07-01 [email protected] Fix missing `[` in docs (flutter/flutter#151091) 2024-07-01 [email protected] Roll pub packages (flutter/flutter#151028) 2024-07-01 [email protected] 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 [email protected],[email protected],[email protected] 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
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
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
///).