On Tue, Jul 28, 2015 at 2:20 PM, Matthieu Moy <Matthieu.Moy@xxxxxxxxxxxxxxx> wrote: > Karthik Nayak <karthik.188@xxxxxxxxx> writes: > >> --- a/t/t6302-for-each-ref-filter.sh >> +++ b/t/t6302-for-each-ref-filter.sh >> @@ -149,4 +149,25 @@ test_expect_success 'check `colornext` format option' ' >> test_cmp expect actual >> ' >> >> +test_expect_success 'check `ifexists` format option' ' >> + cat >expect <<-\EOF && >> + [foo1.10] >> + [foo1.3] >> + [foo1.6] >> + EOF >> + git for-each-ref --format="%(ifexists:[%s])%(refname:short)" | grep "foo" >actual && >> + test_cmp expect actual >> +' > > You're testing only the positive case of ifexists, right? You need a > test for the negative case (i.e. the attribute does not exist) too. > Ideally, check that the ifexist actually applies to the right attribute, > like > > --format '%(ifexist:<%s>)%(attribute1) %(ifexist:[%s])%(attribute2)' > > and manage to get an output like > > <foo> > [bar] > <foobar> [barfoo] > Yes! this seems like an important test, thanks :) > > You have a double || that is not useful. Not really harmful either, but > it may distract the reader. You may want to s/||/|/. > Will change! -- Regards, Karthik Nayak -- 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