Re: [PATCH 1/3] test-lib: Add support for multiple test prerequisites
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Aug 8, 2010, 13:29 UTC
- Message-ID
- <AANLkTikFUH38mzuPo8D-MwJQZdUPEaBUVKgrrfYX=5=p@mail.gmail.com>
- In-Reply-To
- <20100808015729.GA9284@burratino>
On Sun, Aug 8, 2010 at 01:57, Jonathan Nieder <jrnieder@gmail.com> wrote:
Show 10 quoted lines
> Æ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.
Show 13 quoted lines
>> +++ 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.
Show 13 quoted lines
>> + 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).
Show 27 quoted lines
>> +++ 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@gmail.com> > > Thanks.
Likewise.