diff options
author | Jonathan Nieder <jrnieder@gmail.com> | 2011-12-14 02:22:03 -0600 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2011-12-14 09:46:33 -0800 |
commit | 0d16451943ec4f17ea1503a12549531bb278788a (patch) | |
tree | caf5f813ada992736b961382fe8978ab262b8a4b /contrib/ciabot | |
parent | support pager.* for external commands (diff) | |
download | tgif-0d16451943ec4f17ea1503a12549531bb278788a.tar.xz |
test: errors preparing for a test are not special
This script uses the following idiom to start each test in a known
good state:
test_expect_success 'some commands use a pager' '
rm -f paginated.out || cleanup_fail &&
test_terminal git log &&
test -e paginated.out
'
where "cleanup_fail" is a function that prints an error message and
errors out.
That is bogus on three levels:
- Cleanup commands like "rm -f" and "test_unconfig" are designed not
to fail, so this logic would never trip.
- If they were to malfunction anyway, it is not useful to set apart
cleanup commands as a special kind of failure with a special error
message. Whichever command fails, the next step is to investigate
which command that was, for example by running tests with
"prove -e 'sh -x'", and fix it.
- Relying on left-associativity of mixed &&/|| lists makes the code
somewhat cryptic.
The fix is simple: drop the "|| cleanup_fail" in each test and the
definition of the "cleanup_fail" function so no new callers can arise.
Reported-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 'contrib/ciabot')
0 files changed, 0 insertions, 0 deletions