rylnd / shpec

Test your shell scripts!
MIT License
377 stars 25 forks source link

Launch shpec as zsh plugin is limiting #102

Open ezh opened 7 years ago

ezh commented 7 years ago

Right now shpec is alias

which shpec
shpec: aliased to zsh -c 'disable -r end; . /full/path/to/shpec'
  1. this prevents to pass arguments to shpec shpec -v, shpec file is not working
  2. this drop the current zsh environment with all defined functions

I offer to use function instead of alias. We may guard execution with ()

(disable -r time;which time); which time
/usr/bin/time
time: shell reserved word

It maybe something like

__shpec_location="$(dirname $0:A)/bin/shpec"
shpec() { (disable -r end; . $__shpec_location $@) }

There is a problem with multiple arguments, but this is different issue.

ezh commented 7 years ago

Enlighten shpec mailfunction: https://travis-ci.org/ezh/shpec/builds/217703619

Unable to process files with space in name: bash https://travis-ci.org/ezh/shpec/jobs/217703621 dash https://travis-ci.org/ezh/shpec/jobs/217703622 ksh https://travis-ci.org/ezh/shpec/jobs/217703623

Please note that errors within zsh are different One test in multiple_args block are passed and this is second argument. zsh alias is broken. zsh https://travis-ci.org/ezh/shpec/jobs/217703624

ezh commented 7 years ago

I'm happy to fix it up in #107 Rebase on master branch with OSX in test matrix was a bit tough. 😄

rylnd commented 7 years ago

@AdrieanKhisbe @ezh I could use your help here. I'm trying to reproduce this issue on CI for a regression test and I'm having some trouble.

As per my comment, the way that we run unit tests and the way that the shpec executable is interpreted means that we can't write a unit test for the antigen plugin (at least as it currently exists). Couple that with the inability to execute aliases on Travis, and I'm at a loss.

I would love it if we could somehow write a regression test for this (and for argument handling of the executable in general), but since that seems like a lot of effort for not a lot of gain, I would be happy with just fixing the damn plugin :wink:.

@ezh, I would love to give you credit for your time and effort on this issue; if you could submit a PR with just your changes to the antigen plugin, @AdrieanKhisbe and I can test it independently and then merge.

ezh commented 7 years ago

@rylnd Your logic is distracted for me. I don't understand what you do. I rewrite it a bit and now tests are successful.

Anyway, few notes for .travis.yml

  1. why antigen? there are also zgen, antybody, zpm, zplug and so on... IMHO this is common zsh plugin. There is nothing antigen specific. ANTIGEN variable is confused users
  2. why alias? why not function? I bet you always play only on 'nightmare level' of difficulty 😁
  3. FYI I look at the latest .travis.yml and see no shpec -v verification. As I remember that I catched error in master with one shell configuration, but not zsh one. So sad that you ignore the simplest tests.
rylnd commented 7 years ago

@ezh

  1. Good point; I am not a zsh user and I have not heard of plugins other than antigen. I'm all for generalizing it to something like ZSH_PLUGIN.
  2. My point was that I was unable to test the current implementation of the plugin due to the aforementioned issues. Without regression test, confirming the solution must be a manual process, and we have no confidence that the test suite would catch a regression.
  3. There are already tests for command flags in the standard test suite. If you feel that there are gaps in test coverage, please call them out specifically (e.g. issues and/or pull requests) and we can address them together.

A final note: please refrain from the negative comments; they are unnecessary and unwelcome here. Describing my decisions as "sad" or the plugin as "crappy" is not productive and only reflects poorly on you.