Changes as per code review.

Author: jessamynsmith

tests/test_allvirtualenv.sh

@@ -8,6 +8,8 @@ oneTimeSetUp() {
     mkdir -p "$WORKON_HOME"
     unset VIRTUAL_ENV
     source "$test_dir/../virtualenvwrapper.sh"
+    # These three env names must sort the same whether the OS considers leading whitespace or not.
+    # "test" is after " env" and after "env", so the asserts work on OSX and Linux.
     mkvirtualenv test1 >/dev/null 2>&1
     mkvirtualenv test2 >/dev/null 2>&1
     # Only test with leading and internal spaces. Directory names with trailing spaces are legal,

tests/test_cd.sh

@@ -9,9 +9,6 @@ oneTimeSetUp() {
     unset VIRTUAL_ENV
     source "$test_dir/../virtualenvwrapper.sh"
     mkvirtualenv cd-test >/dev/null 2>&1
-    # Only test with leading and internal spaces. Directory names with trailing spaces are legal,
-    # and work with virtualenv on OSX, but error out on Linux.
-    mkvirtualenv " env with space" >/dev/null 2>&1
     deactivate
 }
 
@@ -41,16 +38,6 @@ test_cdvirtual() {
     virtualenvwrapper_cd "$start_dir"
 }
 
-test_cdvirtual_space_in_name() {
-    workon " env with space"
-    start_dir="$(pwd)"
-    cdvirtualenv
-    assertSame "$VIRTUAL_ENV" "$(pwd)"
-    cdvirtualenv bin
-    assertSame "$VIRTUAL_ENV/bin" "$(pwd)"
-    virtualenvwrapper_cd "$start_dir"
-}
-
 test_cdsitepackages () {
     start_dir="$(pwd)"
     cdsitepackages
@@ -60,16 +47,6 @@ test_cdsitepackages () {
     virtualenvwrapper_cd "$start_dir"
 }
 
-test_cdsitepackages_space_in_name () {
-    workon " env with space"
-    start_dir="$(pwd)"
-    cdsitepackages
-    pyvers=$(python -V 2>&1 | cut -f2 -d' ' | cut -f1-2 -d.)
-    sitepackages="$VIRTUAL_ENV/lib/python${pyvers}/site-packages"
-    assertSame "$sitepackages" "$(pwd)"
-    virtualenvwrapper_cd "$start_dir"
-}
-
 test_cdsitepackages_with_arg () {
     start_dir="$(pwd)"
     pyvers=$(python -V 2>&1 | cut -f2 -d' ' | cut -f1-2 -d.)

tests/test_cd_space_in_name.sh

@@ -0,0 +1,55 @@
+# -*- mode: shell-script -*-
+
+test_dir=$(cd $(dirname $0) && pwd)
+source "$test_dir/setup.sh"
+
+oneTimeSetUp() {
+    export WORKON_HOME="$WORKON_HOME/ this has spaces"
+    rm -rf "$WORKON_HOME"
+    mkdir -p "$WORKON_HOME"
+    unset VIRTUAL_ENV
+    source "$test_dir/../virtualenvwrapper.sh"
+    # Only test with leading and internal spaces. Directory names with trailing spaces are legal,
+    # and work with virtualenv on OSX, but error out on Linux.
+    mkvirtualenv " env with space" >/dev/null 2>&1
+    deactivate
+}
+
+oneTimeTearDown() {
+    rm -rf "$WORKON_HOME"
+}
+
+setUp () {
+    echo
+}
+
+tearDown () {
+    deactivate >/dev/null 2>&1
+}
+
+cd () {
+    fail "Should not be using override cd function"
+}
+
+test_cdvirtual_space_in_workon_home_space_in_name() {
+    workon " env with space"
+    start_dir="$(pwd)"
+    cdvirtualenv
+    assertSame "$VIRTUAL_ENV" "$(pwd)"
+    cdvirtualenv bin
+    assertSame "$VIRTUAL_ENV/bin" "$(pwd)"
+    virtualenvwrapper_cd "$start_dir"
+}
+
+test_cdsitepackages_space_in_name () {
+    workon " env with space"
+    start_dir="$(pwd)"
+    cdsitepackages
+    pyvers=$(python -V 2>&1 | cut -f2 -d' ' | cut -f1-2 -d.)
+    sitepackages="$VIRTUAL_ENV/lib/python${pyvers}/site-packages"
+    assertSame "$sitepackages" "$(pwd)"
+    virtualenvwrapper_cd "$start_dir"
+}
+
+
+. "$test_dir/shunit2"

tests/test_cp.sh

@@ -33,7 +33,16 @@ test_new_env_activated () {
     assertTrue virtualenvwrapper_verify_active_environment
 }
 
+test_virtual_env_variable () {
+    mkvirtualenv "source" >/dev/null 2>&1
+    cpvirtualenv "source" "destination" >/dev/null 2>&1
+    assertSame "Wrong virtualenv name" "destination" $(basename "$VIRTUAL_ENV")
+    assertTrue "$WORKON_HOME not in $VIRTUAL_ENV" "echo $VIRTUAL_ENV | grep -q $WORKON_HOME"
+}
+
 test_virtual_env_variable_space_in_name () {
+    # Only test with leading and internal spaces. Directory names with trailing spaces are legal,
+    # and work with virtualenv on OSX, but error out on Linux.
     mkvirtualenv " space source" >/dev/null 2>&1
     cpvirtualenv " space source" " space destination" >/dev/null 2>&1
     assertSame "Wrong virtualenv name" " space destination" "$(basename "$VIRTUAL_ENV")"

tests/test_ls.sh

@@ -25,7 +25,16 @@ test_get_site_packages_dir () {
     deactivate
 }
 
+test_lssitepackages () {
+    mkvirtualenv "lssitepackagestest" >/dev/null 2>&1
+    contents="$(lssitepackages)"    
+    assertTrue "did not find easy_install in site-packages" "echo $contents | grep -q easy_install"
+    deactivate
+}
+
 test_lssitepackages_space_in_name () {
+    # Only test with leading and internal spaces. Directory names with trailing spaces are legal,
+    # and work with virtualenv on OSX, but error out on Linux.
     mkvirtualenv " space lssitepackagestest" >/dev/null 2>&1
     contents="$(lssitepackages)"    
     assertTrue "did not find easy_install in site-packages" "echo $contents | grep -q easy_install"

tests/test_workon.sh

@@ -108,8 +108,8 @@ test_virtualenvwrapper_show_workon_options_chpwd () {
 test_virtualenvwrapper_show_workon_options_no_envs () {
     old_home="$WORKON_HOME"
     export WORKON_HOME=${TMPDIR:-/tmp}/$$
-    envs=$(virtualenvwrapper_show_workon_options 2>/dev/null\
-        | tr '\n' '' 2>/dev/null | tr ' ' '' 2>/dev/null)
+    # On OSX there is a space and on Linux there is not, so strip all spaces
+    envs=$(virtualenvwrapper_show_workon_options 2>/dev/null | sed 's/\n //g')
     assertSame "" "$envs"
     export WORKON_HOME="$old_home"
 }

virtualenvwrapper.sh

@@ -521,6 +521,7 @@ function rmvirtualenv {
 
     # support to remove several environments
     typeset env_name
+    # Must quote the parameters, as environments could have spaces in their names
     for env_name in "$@"
     do
         echo "Removing $env_name..."
@@ -559,13 +560,18 @@ function virtualenvwrapper_show_workon_options {
     # NOTE: DO NOT use ls or cd here because colorized versions spew control 
     #       characters into the output list.
     # echo seems a little faster than find, even with -depth 3.
+    # Note that this is a little tricky, as there may be spaces in the path.
     #
     # 1. Look for environments by finding the activate scripts.
     #    Use a subshell so we can suppress the message printed
     #    by zsh if the glob pattern fails to match any files.
-    # 2. Strip the WORKON_HOME prefix from each name.
-    # 3. Strip the bindir/activate script suffix.
-    # 4. Format the output to show one name on a line.
+    #    This yields a single, space-separated line containing all matches.
+    # 2. Replace the trailing newline with a space, so every
+    #    possible env has a space following it.
+    # 3. Strip the bindir/activate script suffix, replacing it with
+    #    a slash, as that is an illegal character in a directory name.
+    #    This yields a slash-separated list of possible env names.
+    # 4. Replace each slash with a newline to show the output one name per line.
     # 5. Eliminate any lines with * on them because that means there 
     #    were no envs.
     (virtualenvwrapper_cd "$WORKON_HOME" && echo */$VIRTUALENVWRAPPER_ENV_BIN_DIR/activate) 2>/dev/null \
@@ -723,6 +729,8 @@ function workon {
         return 1
     elif [ "$env_name" = "." ]
     then
+        # The IFS default of breaking on whitespace causes issues if there
+        # are spaces in the env_name, so change it.
         IFS='%'
         env_name="$(basename $(pwd))"
         unset IFS
@@ -885,15 +893,15 @@ function cdsitepackages {
     virtualenvwrapper_verify_workon_home || return 1
     virtualenvwrapper_verify_active_environment || return 1
     typeset site_packages="`virtualenvwrapper_get_site_packages_dir`"
-    virtualenvwrapper_cd "$site_packages"/$1
+    virtualenvwrapper_cd "$site_packages/$1"
 }
 
 # Does a ``cd`` to the root of the currently-active virtualenv.
 #:help:cdvirtualenv: change to the $VIRTUAL_ENV directory
 function cdvirtualenv {
     virtualenvwrapper_verify_workon_home || return 1
     virtualenvwrapper_verify_active_environment || return 1
-    virtualenvwrapper_cd "$VIRTUAL_ENV"/$1
+    virtualenvwrapper_cd "$VIRTUAL_ENV/$1"
 }
 
 # Shows the content of the site-packages directory of the currently-active
@@ -1293,7 +1301,9 @@ function allvirtualenv {
     virtualenvwrapper_verify_workon_home || return 1
     typeset d
 
-    IFS=''
+    # The IFS default of breaking on whitespace causes issues if there
+    # are spaces in the env_name, so change it.
+    IFS='%'
     virtualenvwrapper_show_workon_options | while read d
     do
         [ ! -d "$WORKON_HOME/$d" ] && continue