This has been driving nuts for years. The output of the diff emitted
when a test fails was always reversed, because the diff tool is called
with `${difftool} ${new} ${old}` so all the `-` and `+` contexts are
reversed, and the highlights are all screwed up.
The output of a `make test` run should show what has changed from the
baseline/expected, not how the expected differs from the actual. When
considered from both the perspective of intentional changes to the test
outputs and failed test outputs, it is desirable to see how the test
output has changed from the previously expected, and not the other way
around.
(If you were used to the previous behavior, I apologize. But it was
wrong.)
Using printf like
printf "The message"
is unsafe, because if the message contains any formatting characters,
they'll be interpreted.
In this case it's not all that important because the message contains
only filenames of our tests and static strings, but still.
There's just waaayy too many things that could go wrong with it, so it
annoys more than it helps, especially since we don't get any
indication what failed.
E.g. on FreeBSD, the test failed without a usable message just because
`tput` couldn't find an attribute (so colors were unset).
The one thing I was missing:
`echo -n` isn't POSIX. In practice, it appears the only shell to encounter this
is macOS' crusty old bash in sh-mode. Just replace it with `touch`.
This reverts commit fc5e8f9fec.
This makes the script worse, but it's good enough.
The required changes are:
- `shopt -s nullglob`, which we simply don't use (we have one glob, but that's
guaranteed to match because we ship the files)
- One array, which we replace with a direct use of the glob (plus it
used `echo` again?)
- The `function` word, which I'm still annoyed is even a thing!
- Variable indirection (`color=${!color_var}` - instead we pass the
value directly - which makes the script uglier!)
- One array, which we replace with a function
- A use of `type -t`, replaced with `command -v`
- A use of `${var:begin:end}` substring expansion, replaced with trickery.
- `set -o pipefail` is replaced with a function
Note that checkbashisms still complains about `command -v`, because
we're not using it with "-p". But we _want_ to check the current
$PATH, and `command -v` is POSIX.
This still uses `local`, which technically isn't in POSIX.
The tests now appear to pass in:
- bash
- dash
- zsh
- mksh
- busybox
Because the 'getopt' library differs between systems, it's likely
that there will be different output. This is the case between the
GNU-based Linux and the BSD-based Darwin, for the 'getopt' library,
it seems. It causes the tests to produce different results.
To allow us to test, and check for regressions, on the different
platforms, the invocation code has been updated to allow a
system-specific suffix to be used on the test files. If this suffix
is found, the test will also be flagged as being system-specific
which should ensure the change in behaviour is noted.
The Travis macOS test systems do not appear to have colordiff present, so any
failures would mean that no output would be shown. This may also be a
problem for the other test scripts as well, but the invocation tests are
the ones being affected here.
We change our behaviour to downgrade to the plain diff tool if colordiff is
not present.
The invocation tests were not especially clear on how they should be
used, without reading the code. And who really wants to do that? So,
a description of what the test does (and thus what each file is) is
now present in the file prologue comment.
The new '-C' initial command needs some tests, and as there are no
tests just yet for the command invocation, this change adds a harness
and calls it from the high-level tests in the Makefile.
The tests are similar in style to the other high level tests, in that
we capture the output and compare it to that which we expect. The
harness itself is written in bash - sorry - because we're testing the
fish shell's invocation, and trying to do that with the fish we've
just built wouldn't actually make for a very useful test when things
go wrong.
The 'tests/invocation.sh' script can be executed manually, or as part
of the 'make test' target, to make it easy to use both as part of the
development and as part of automation.
The harness has only been tested on linux with bash 4.3.11, and requires
grep and sed. Although not tested with OS X, I believe I have avoided
the syntax which is inconsistent.
The tests added here cover just the initial command's basic execution,
and when it is mixed with the regular '-c' command.
