new: Fix test failures for Bash < 4.2.32

The previous arguments to `@go.test_compgen` should have ended with a
trailing slash; as it stood, it was returning the directory itself, with
a trailing slash appended. After some experimentation, it emerged that
removing the directory prefix (with trailing slash) from the expected
results (e.g.  `$TESTS_GO_SCRIPTS_DIR/lib/`) resulted in passing the
empty string as the sole argument to `assert_success` through Bash
4.2.31, and in passing no arguments at all to `assert_success` from Bash
4.2.32 on.

Again using the methodology described in the log message for commit
99ab780, I built different Bash
versions and performed a binary search using the following command until
I pinpointed 4.2.32 as the inflection point, replacing `BASH_VERSION` in
`PATH` at each iteration:

  $ PATH=/usr/local/bash-builds/BASH_VERSION/bin:$PATH \
      TEST_FILTER='tab complete --internal' gos test new

I then added the following line to `assert_success` from
`lib/bats/assertions`:

  printf 'ARGC: %d\n' "$#" >&2

and added `return 1` after the `assert_success` call in the `tab
complete --internal` test case to force an error under 4.2.32, which
produced the following results (edited for clarity):

  $ PATH=/usr/local/bash-builds/4.2.31/bin:$PATH \
      TEST_FILTER='tab complete --internal' gos test new

   ✗ new: tab complete --internal
     (in test file tests/new.bats, line 96)
       `assert_success "${expected[@]#$TEST_GO_SCRIPTS_DIR/lib/}"' failed
     ARGC: 1
     ARGC: 1
     output not equal to expected value:
       expected: ''
       actual:   'bar
     baz
     foo'

  $ PATH=/usr/local/bash-builds/4.2.32/bin:$PATH \
      TEST_FILTER='tab complete --internal' gos test new

   ✗ new: tab complete --internal
     (in test file tests/new.bats, line 96)
       `assert_success "${expected[@]#$TEST_GO_SCRIPTS_DIR/lib/}"' failed
     ARGC: 1
     ARGC: 0

Later I reproduced this behavior using a standalone script,
`expand-empty-list-test.bash`:

  #! /usr/bin/env bash

  print_argc() {
    printf 'ARGC: %d\n' "$#"
    for arg in "$@"; do
      printf 'ARG:  "%s"\n' "$arg"
    done
  }

  ARGV=('foo')
  print_argc "${ARGV[@]#foo}"

The last line can also be replaced with the following to achieve the
same effect:

  set - "${ARGV[@]#foo}"
  print_argc "$@"

Running either version of this script produces the output:

  $ /usr/local/bash-builds/4.2.31/bin/bash expand-empty-list-test.bash
  ARGC: 1
  ARG:  ""

  $ /usr/local/bash-builds/4.2.32/bin/bash expand-empty-list-test.bash
  ARGC: 0

Also, this behavior _only_ manifests when an array containing a single
value is expanded such that all of its characters are removed. Various
other values for `ARGV` (including the empty list) that result in more
than one value (even more than one empty string) do not exhibit this
behavior. Any form of expansion that removes all the characters from
every list item (including `##*`, `%%*`, and `//*/`) only exhibit this
behavior when `ARGV` contains a single string.

This is the patch and bug report for Bash 4.2.32:

  https://mirrors.ocf.berkeley.edu/gnu/bash/bash-4.2-patches/bash42-032
  http://lists.gnu.org/archive/html/bug-bash/2012-05/msg00010.html

Which appears to correspond to the following from
https://tiswww.case.edu/php/chet/bash/CHANGES:

  This document details the changes between this version,
  bash-4.3-alpha, and the previous version, bash-4.2-release.

  ggg. Fixed a bug that causes the shell to delete DEL characters in the
       expanded value of variables used in the same quoted string as
       variables that expand to nothing.

This doesn't speak directly to the "single item array expansion to empty
string" behavior difference, but the patch contains the following change
to `expand_word_internal` from `subst.c`:

  *** ../bash-20120427/subst.c  2012-04-22 16:19:10.000000000 -0400
  --- subst.c 2012-05-07 16:06:35.000000000 -0400
  ***************
  *** 8152,8155 ****
  --- 8152,8163 ----
        dispose_word_desc (tword);

  +     /* Kill quoted nulls; we will add them back at the end of
  +        expand_word_internal if nothing else in the string */
  +     if (had_quoted_null && temp && QUOTED_NULL (temp))
  +       {
  +         FREE (temp);
  +         temp = (char *)NULL;
  +       }
  +
        goto add_string;
        break;

`QUOTED_NULL` is defined in `subst.h` as:

  /* Is the first character of STRING a quoted NULL character? */
  #define QUOTED_NULL(string) ((string)[0] == CTLNUL && (string)[1] == '\0')

and `CTLNUL` is defined in `shell.h` as:

  #define CTLNUL '\177'

Back to `expand_word_internal`, the `add_string:` label following the
`Kill quoted nulls` block is followed by:

  add_string:
    if (temp)
      {
        istring = sub_append_string (...);
        temp = (char *)0;
      }

    break;

Finally, the end of the function contains the following comment and
block of code:

```
finished_with_string:
  /* ... */

  /* If we expand to nothing and there were no single or double quotes
     in the word, we throw it away.  Otherwise, we return a NULL word.
     The single exception is for $@ surrounded by double quotes when
     there are no positional parameters.  In that case, we also throw
     the word away. */

  if (*istring == '\0')
    {
      if (quoted_dollar_at == 0 && ...)
        {
          /* ... */
        }
      /* According to sh, ksh, and Posix.2, if a word expands into nothing
         and a double-quoted "$@" appears anywhere in it, then the entire
         word is removed. */
      else  if (quoted_state == UNQUOTED || quoted_dollar_at)
        list = (WORD_LIST *)NULL;
```

As best I can tell, the 4.2.32 patch that kills `QUOTED_NULL` values
prevented the assignment to `istring` (under the `add_string:` label)
that was still performed as of 4.2.31. Somehow expanding a single-item
array such that it results in the empty string produces a `QUOTED_NULL`,
hence the difference in behavior betweeen the two versions.

With this change to the test, the bugs in the tests themselves are
fixed, and the tests all pass under both versions. That said, this is a
rather pernicious kind of bug that seems difficult to detect and defend
against, given that it only manifests when single-item arrays are
expanded as a single empty string. The interface to `assert_success` and
`assert_failure` isn't wrong; it's intended to make it easy to check
command output without requiring two different assertions when a
straight equality check will do—especially when there should be no
output at all. I'll have to think on whether there's an effective
mechanism or idiom that can guard against it; but for now, my guard is
up, at least.

Author: mbland

tests/new.bats

@@ -90,7 +90,7 @@ assert_command_script_is_executable() {
   touch "${internal_modules[@]}"
 
   local expected
-  @go.test_compgen 'expected' -f "$TEST_GO_SCRIPTS_DIR/lib"
+  @go.test_compgen 'expected' -f "$TEST_GO_SCRIPTS_DIR/lib/"
 
   run "$TEST_GO_SCRIPT" complete 2 new '--internal'
   assert_success "${expected[@]#$TEST_GO_SCRIPTS_DIR/lib/}"
@@ -112,7 +112,7 @@ assert_command_script_is_executable() {
   touch "${public_modules[@]}"
 
   local expected
-  @go.test_compgen 'expected' -f "$TEST_GO_ROOTDIR/lib"
+  @go.test_compgen 'expected' -f "$TEST_GO_ROOTDIR/lib/"
 
   run "$TEST_GO_SCRIPT" complete 2 new '--public'
   assert_success "${expected[@]#$TEST_GO_ROOTDIR/lib/}"
@@ -134,7 +134,7 @@ assert_command_script_is_executable() {
   touch "${test_files[@]}"
 
   local expected
-  @go.test_compgen 'expected' -f "$TEST_GO_ROOTDIR/$_GO_TEST_DIR"
+  @go.test_compgen 'expected' -f "$TEST_GO_ROOTDIR/$_GO_TEST_DIR/"
 
   run "$TEST_GO_SCRIPT" complete 2 new '--test'
   assert_success "${expected[@]#$TEST_GO_ROOTDIR/$_GO_TEST_DIR/}"