sstephenson / bats

Bash Automated Testing System
MIT License
7.12k stars 519 forks source link

Remove fancy syntax for bash 3.00 compatibility #233

Closed aallrd closed 6 years ago

aallrd commented 6 years ago

Hello, I tried to use BATS to test a script on a SunOS 10u9 server running a BASH 3.00.16(1)-release and I had issues with the += syntax. This commit changes this syntax to a plain concatenation that works with our old release.

dimo414 commented 6 years ago

The += operator was added in Bash 3.1, which was released ~15 years ago. It seems much more productive for you to upgrade your system than for BATS to be stuck supporting 15 year old syntax indefinitely.

If BATS really wants to support 3.0 syntax it would also be necessary to improve the the CI tests to run against a Bash 3.0 installation, otherwise it's likely incompatible syntax will be re-added later.

aallrd commented 6 years ago

Believe me, I would like nothing more than working with a more recent version of bash :) Unfortunately, my clients are running old SunOS releases in production that ship with this equivalently old release of bash, there is no way for me to ask them to upgrade this component. I agree that the += operator makes for cleaner code, but considering that it is only used 4 times I think it's a small price to pay to extend the compatibility of this tool to even older releases of bash (at least 3.0 in my case). The only draw back I see (besides the ugly syntax) is the testing part, but if you already do test with bash 3.1 it should not be hard to run 3.0.

dimo414 commented 6 years ago

it should not be hard to run 3.0

I'd suggest including the necessary changes to the CI config in your pull request, then. It doesn't make sense to do a one-time cleanup of a project to make it compatible with an old version of a dependency without a plan to enforce that requirement going forward.

SunOS was superseded by Solaris in 1993, and hasn't been supported since 2003. Personally, supporting 3.0 simply isn't a reasonable requirement in 2017 - I'd just maintain your own fork.

Sylvain303 commented 6 years ago

@aallrd the bats internal preprocessor could be followed by a post-processor. We could imagine to "translate" some too new bash features to old tricks accomplishing the same job. It means that you would have to pre-process the test on a recent (decent) machine first (your laptop or any controlling machine), then upload the result to the destination machine.

That have the advantage of keeping bats code uptodate, And introducing remote testing step, which allow to concentrate on specific details. It could be accomplished by any Continuous Integration tools, on your side.

Suggested process details:

jasonkarns commented 6 years ago

@aallrd Is there a reason that you can't use a newer version of bash just for Bats? You wouldn't need to upgrade the system /bin/bash; just make it available such that your .bats tests can use it ie #! /opt/bin/bash or similar.

aallrd commented 6 years ago

Hello,

Sorry for the late reply.

I was working on running the Bats tests with the older bash, it's actually very easy and I just added a step in the Travis CI job to download, build, install and export the bash 3.0 in the PATH. The tests even run fine, everything is passing except a failing test with the skipped tests. The error has to do with some regex internals of the bash I think, but I failed to debug the issue until now.

The reason I cannot use a newer version of bash is because I have to make sure my scripts are working even on the old systems of my clients. I cannot impose this kind of change on my clients infrastructure. Therefore I cannot ship a script with features from a "recent" bash, so testing with the older bash allows me to make sure that I respect this condition (and still test that my script is working...).

Anyway, I understand that this is a very isolated issue so I will remove my PR. I will use this tweaked version internally (even if it's not fully working) to answer my specific need, but I won't pollute this project's code with this old compatibility issue.