On Sun, Aug 8, 2010 at 01:57, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote:
> Ævar Arnfjörð Bjarmason wrote:
>
>> Change the test_have_prereq function in test-lib.sh to support a
>> comma-separated list of prerequisites. This is useful for tests that
>> need e.g. both POSIXPERM and SANITY.
>>
>> The implementation was stolen from Junio C Hamano and Johannes Sixt,
>> the tests and documentation were not.
>
> I think you can sell it better. :)
As Johannes points out the patch is different enough that I changed
the authorship. Actually I wouldn't modify any patch under someone
else's name without getting them to sign off on it.
>> +++ b/t/t0000-basic.sh
>> @@ -73,6 +73,23 @@ then
>> exit 1
>> fi
>>
>> +test_set_prereq HAVETHIS
>> +haveit=no
>> +test_expect_success HAVETHIS,HAVEIT 'test runs if prerequisites are satisfied' '
>> + test_have_prereq HAVEIT &&
>> + test_have_prereq HAVETHIS &&
>
> I think the similar code above was just a way to sneak in a sanity
> check for test_have_prereq(). I’d leave it out.
It's a sanity test. If that wasn't there the test might succeed
e.g. if the implementation was broken and only did a OR on the comma
delimited list, not a AND.
>> + haveit=yes
>> +'
>> +donthaveit=yes
>> +test_expect_success HAVEIT,DONTHAVEIT 'unmet prerequisites causes test to be skipped' '
>> + donthaveit=no
>> +'
>> +if test $haveit$donthaveit != yesyes
>> +then
>> + say "bug in test framework: multiple prerequisite tags do not work reliably"
>> + exit 1
>> +fi
>
> Maybe it would be simpler to squash this in with the other similar checks.
I think that's too much complexity for one test, especially in the
sanity file. I didn't squash it with the existing "yesyes" test
because we're testing basic functionality first (one prereq) then the
fancy stuff (multiple).
>> +++ b/t/test-lib.sh
>> @@ -327,12 +327,20 @@ test_set_prereq () {
>> satisfied=" "
>>
>> test_have_prereq () {
>> - case $satisfied in
>> - *" $1 "*)
>> - : yes, have it ;;
>> - *)
>> - ! : nope ;;
>> - esac
>> + # prerequisites can be concatenated with ','
>> + save_IFS=$IFS
>> + IFS=,
>> + set -- $*
>> + IFS=$save_IFS
>> + for prerequisite
>> + do
>> + case $satisfied in
>> + *" $prerequisite "*)
>> + : yes, have it ;;
>> + *)
>> + ! : nope ;;
>> + esac
>> + done
>
> Does that work?
It passes all the tests :)
> Except as noted above,
> Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
>
> Thanks.
Likewise.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html