search

scmbreeze / scm_breeze

Adds numbered shortcuts to the output git status, and much more
https://madebynathan.com/2011/10/19/git-shortcuts-like-youve-never-seen-before/
MIT License
2.82k stars 192 forks source link

make tests independent of bin locations #229

Closed vise890 closed 7 years ago

vise890 commented 7 years ago

On my system (Arch Linux), the commands {mv,rm,sed} live under /usr/bin, not under /bin. So the tests were failing.

This commit hard-codes the initial paths to make the tests independent on their real paths.

vise890 commented 7 years ago

it's probably worth solving #226 asap as well. @ghthor please make sure that i haven't broken anything before merging this.

ghthor commented 7 years ago

I don't understand.... wouldn't this make the tests fail if those commands were in /usr/bin?

jeffbyrnes commented 7 years ago

Yeah, this would break any system where these commands live in /usr/bin. @vise890 this would break on any *nix I use in my day-to-day.

Is there some reason not to just let your $PATH handle these, and avoid absolute paths? That appears to be the existing implementation.

vise890 commented 7 years ago

Does it really break tho? My understanding/hope was that alias wouldn't care if what you were aliasing didn't actually exist.

jeffbyrnes commented 7 years ago

Well, it would mean that these aliases don’t work, and since they’re aliasing commands that are needed by the tests, it would stand to follow that the tests would break.

Setting the aliases conditionally, based on OS, would be a more durable move, I think.

ghthor commented 7 years ago

I don't think the values from within these aliases make any difference. I think this is a test to assert that the code that adds argument expansion wrapping to these commands will wrap any existing aliases the user had setup. I believe we can just dump these changes if I am correct about this.

jeffbyrnes commented 7 years ago

That seems reasonable.

vise890 commented 7 years ago

right so is this ready to merge?

ghthor commented 7 years ago

Yeah, I think for now we should just keep powering forward. I need to setup the travis shit ASAP