Checking for rvm Ruby versions

Question

As it is going to be used as part of some automation and not just at the command line I was wondering if it could be improved at all.

Improve would mean any of:

• handle sad cases better
• handle error better
• report errors better
• better able to work on multiple linux OS including OSX

Improve would NOT mean:

• shorter code
• short variable names
• Linux shell version specific tricks or shortcuts
• allowing for rvm not being present (checked elsewhere)

function make_sure_rvm_rubies_installed() {

rvm list > $TMP/local_ruby_versions.txt
local_ruby_versions=(1.8.7 1.9.3)

for version in "${local_ruby_versions[@]}"
do
  cat $TMP/local_ruby_versions.txt | grep -q $version
  version_search_result=$?
  if [ $version_search_result = 0 ]
  then
    echo "Required RVM Ruby version $version confirmed as present on this machine"
  else
    echo "*** EXITING SMOKE TEST *** - not all required ruby versions are present in RVM"
    echo "Please install RVM ruby version: $version and then re-run this program"
    exit
  fi
done
if [ $? = 0 ]
  then echo "All required Ruby Versions confirmed as present locally"
fi
}

make_sure_rvm_rubies_installed

Answer 1

The name make_sure_rvm_rubies_installed describes very well what the function does, but it's a bit too much. Maybe check_installed_rubies would be good enough.

---

You didn't indent the body of the function. It's better to indent the inner part of large { ... } blocks for readability.

No need to use the function keyword when declaring functions, though you can if you like.

---

Instead of cat ... | grep it's better to omit the cat and use the file as a parameter of grep.

No need to save the exit code of the grep in the version_search_result variable, you can do this directly:

  if grep -q $version $TMP/local_ruby_versions.txt
  then
    echo "Required RVM Ruby version $version confirmed as present on this machine"
  else
    echo "*** EXITING SMOKE TEST *** - not all required ruby versions are present in RVM"
    echo "Please install RVM ruby version: $version and then re-run this program"
    exit
  fi

The exit there is a bit unexpected. The script could be reusable if you return 1 instead, to give a chance to the caller to recover gracefully if necessary. (In your example use case it doesn't matter, but this is a tip for the future.)

---

I'm not sure what you're expecting from the $? check in this code:

for version in "${local_ruby_versions[@]}"
do
  # ...
done
if [ $? = 0 ]
  then echo "All required Ruby Versions confirmed as present locally"
fi

When a version is not found the original loop exits the entire script (I recommended above to return from the function). So in case of failures this if is not reached. Otherwise the last command to evaluate is an echo, which should just succeed, so this if condition seems pointless, you can omit it.

---

Although arrays are cool, and it's great that you can use them, in simple situations like this I would use a normal variable:

local_ruby_versions='1.8.7 1.9.3'

for version in $local_ruby_versions; do ...; done

---

You create the file temporary file $TMP/local_ruby_versions.txt but you don't clean it up when finished.