-
Notifications
You must be signed in to change notification settings - Fork 518
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
Prevent crashing when mv
warns and report warnings to the user instead.
#1326
Conversation
6bfbba8
to
b8d80fd
Compare
Should there be a test for this as well? Not sure how I can mock |
{ok, []} -> | ||
ok; | ||
{ok, Warning} -> | ||
?WARN("mv warning ~p", [Warning]), |
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.
I'd change the string to "mv: ~p"
there.
A test would be good. We may not need to mock anything -- if we can make a file inside CT's To mock meck:new(rebar_utils, [passthrough]),
meck:expect(rebar_utils, sh, fun(<STRING_FOR_MV>, _) -> {ok, "Warning"} end),
ok = rebar_utils:mv(Src, Dest),
meck:unload(rebar_utils). We possibly do not need to test that the warning is being relayed, though that would be nicer and would require more mocking (and conditional test skips for windows runs). |
905a5a1
to
8457428
Compare
Added a test. Mocking, because I wasn’t able to reproduce |
8457428
to
e2c57e4
Compare
In some cases, mv will throw a warning, while still moving the files correctly and returning a 0 return code: "mv: can't preserve ownership of ... Permission denied".
e2c57e4
to
2b6fa7a
Compare
And now green. 😉 |
mv
warns and report warnings to the user instead.
Merging, thanks! |
Thank you for the help, Fred! |
* Switch to recent rebar3 build for erlang/rebar3#1326
* Switch to recent rebar3 build for erlang/rebar3#1326
* Switch to recent rebar3 build for erlang/rebar3#1326
* Switch to recent rebar3 build for erlang/rebar3#1326
* Switch to recent rebar3 build for erlang/rebar3#1326
* Switch to recent rebar3 build for erlang/rebar3#1326 * Add docker-compose.yml * Update docs
* Switch to recent rebar3 build for erlang/rebar3#1326 * Add docker-compose.yml * Update docs
* Switch to recent rebar3 build for erlang/rebar3#1326 * Add docker-compose.yml * Update docs
* Switch to recent rebar3 build for erlang/rebar3#1326 * Add docker-compose.yml * Update docs
* Switch to recent rebar3 build for erlang/rebar3#1326 * Add docker-compose.yml * Update docs
* Switch to recent rebar3 build for erlang/rebar3#1326 * Add docker-compose.yml * Update docs
* Switch to recent rebar3 build for erlang/rebar3#1326 * Add docker-compose.yml * Update docs
* Switch to recent rebar3 build for erlang/rebar3#1326 * Add docker-compose.yml * Update docs
* Switch to recent rebar3 build for erlang/rebar3#1326 * Add docker-compose.yml * Update docs
* Switch to recent rebar3 build for erlang/rebar3#1326 * Add docker-compose.yml * Update docs
* Switch to recent rebar3 build for erlang/rebar3#1326 * Add docker-compose.yml * Update docs
* Switch to recent rebar3 build for erlang/rebar3#1326 * Add docker-compose.yml * Update docs
* Switch to recent rebar3 build for erlang/rebar3#1326 * Add docker-compose.yml * Update docs
* Switch to recent rebar3 build for erlang/rebar3#1326 * Add docker-compose.yml * Update docs
Fix #1324.
In some cases, mv will throw a warning, while still moving the files correctly and returning a 0 return code:
"mv: can't preserve ownership of ... Permission denied".