Skip to content
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

Fix rename error that occurs if the target is a naked binary #13

Merged
merged 1 commit into from
Jun 13, 2018

Conversation

yuuki
Copy link
Contributor

@yuuki yuuki commented Jun 13, 2018

Problem

ghg get gets the following error if ghg bin directory doesn't exist and the installation target is a naked binary.

% rm -fr /Users/y_uuki/.ghg/bin
% ghg get yuuki/grabeni
fetch the GitHub release for yuuki/grabeni
install yuuki/grabeni version: v0.4.2
download https://github.com/yuuki/grabeni/releases/download/v0.4.2/grabeni_darwin_amd64
[======================================] 17.6 MB/17.6 MB
install grabeni
rename /Users/y_uuki/.ghg/tmp/337788732/grabeni_darwin_amd64 /Users/y_uuki/.ghg/bin/grabeni: no such file or directory

Cause & Fix

The code path for naked binary don't create bin directory, so that moving the download file to bin directory fails.

% rm -fr /Users/y_uuki/.ghg/bin
% ./ghg get -u yuuki/grabeni
fetch the GitHub release for yuuki/grabeni
install yuuki/grabeni version: v0.4.2
download https://github.com/yuuki/grabeni/releases/download/v0.4.2/grabeni_darwin_amd64
[======================================] 17.6 MB/17.6 MB
/Users/y_uuki/.ghg/bin/grabeni exists. overwrite it
install grabeni
download https://github.com/yuuki/grabeni/releases/download/v0.4.2/grabeni_darwin_amd64.zip
[======================================] 4.44 MB/4.44 MB
extract grabeni_darwin_amd64.zip
/Users/y_uuki/.ghg/bin/grabeni exists. overwrite it
install grabeni
done!

Question

How about testing for that problem?
It seems good to delete bin directory before testing naked binary, but cleaning process is implemented by TestMain.
Which should we take the following choices?

  • Clean bin directory each TableDrivenTests iteration
  • or separate TableDrivenTests iterations to two test functions.
  • or simply add another test function for that problem.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.7%) to 68.106% when pulling 6e7ceb6 on yuuki:fix_linux_rename into 70be685 on Songmu:master.

@Songmu Songmu merged commit 190bc21 into Songmu:master Jun 13, 2018
@Songmu
Copy link
Owner

Songmu commented Jun 13, 2018

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants