Re: [PATCH 3/2] test: factor out helper function test_must_contain
- From
René Scharfe <l.s.r@web.de>
- Date
- Nov 21, 2015, 09:35 UTC
- Message-ID
- <56503ADC.50908@web.de>
- In-Reply-To
- <56502745.2000307@kdbg.org>
Am 21.11.2015 um 09:11 schrieb Johannes Sixt:
Show 18 quoted lines
> Am 20.11.2015 um 21:50 schrieb René Scharfe:
>> Extract a helper function for searching for a pattern in a file and
>> printing the whole file if the pattern is not found. It is useful
>> when starting tests with --verbose for debugging purposes.
>
>> +# Check if a file contains an expected pattern.
>> +test_must_contain () {
>> + if grep "$1" "$2"
>> + then
>> + return 0
>> + else
>> + echo "didn't find /$1/ in '$2', it contains:"
>> + cat "$2"
>> + return 1
>> + fi
>> +}
>
> There is already test_i18n_grep. Should it be folded into this function? That's a good point. But how? test_i18ngrep can also work as a filter and pass on options, so we'd need to parse all parameters and redirect stdin to a temporary file unless a file was specified. Or we could be sloppy and just check if the last parameter is a file and if yes then spew it out.
> Wouldn't we also want to have a function test_must_not_contain?
I doubt it. In such a function grep would display the lines that match unexpectedly already, so showing the whole file after that won't add much more of interest.
Show 6 quoted lines
> IMHO, we should not increase the number of functions that give a bonus > only when there is a test case failure. They do not scale well: There is > a permanent mental burden on every reviewer to watch out that they are > used in new tests. But without those functions, the burden is on the one > person investigating a test case failure, who has to live without the > debugging support.
test_must_contain doesn't have to be used everywhere, only in cases where a file is shown and grepped. I agree that letting an existing function do that job (or deciding that the job is not worth doing) is preferable.
Here's how I imagine the sloppy add-on to test_i18ncmp to look:
diff --git a/t/test-lib.sh b/t/test-lib.sh index 16c4d7b..db64600 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -985,13 +985,28 @@ test_i18ncmp () { test_i18ngrep () { if test -n "$GETTEXT_POISON" then : # pretend success elif test "x!" = "x$1" then shift ! grep "$@" else grep "$@" + + rc=$? + if test $rc != 0 + then + while test $# -gt 1 + do + shift + done + if test -f "$1" + then + echo "Expected pattern not found, content is:" + cat "$1" + fi + return $rc + fi fi }