-
Notifications
You must be signed in to change notification settings - Fork 426
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
Delete existing empty dest folder before rename #2568
Delete existing empty dest folder before rename #2568
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2568 +/- ##
==========================================
+ Coverage 78.20% 78.28% +0.08%
==========================================
Files 107 107
Lines 11802 11805 +3
==========================================
+ Hits 9230 9242 +12
+ Misses 2081 2071 -10
- Partials 491 492 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
minor comments, also could you please add integration test links in description Test section, the new test and an old test where rename fails if directory is not empty and rename passes when dest directory doesnt exist,
I've added it, and you can open it through the Kokoro button from the checks section. All the old tests are passing, and I've added a new test case where the destination directory exists and is empty. You can see the successful results from Kokoro in the passes results section. |
Oh I just noticed that this got merged in the middle of my review. |
Description
Link to the issue in case of a bug fix.
NA
Testing details
Without the fix, the additional test was failing and I could see RenameFolder error from the logs.