Re: Re: [PATCH v3 5/5] do not die when error in config parsing of buf occurs

From: Jeff King <peff@peff.net>
Date: May 11, 2013, 09:59 UTC
Message-ID: <20130511095936.GB17326@sigill.intra.peff.net>
In-Reply-To: <20130511085531.GA3670@book-mint>

On Sat, May 11, 2013 at 10:55:31AM +0200, Heiko Voigt wrote:

Show 32 quoted lines

> > > diff --git a/t/t1307-config-blob.sh b/t/t1307-config-blob.sh
> > > index fdc257e..a4929eb 100755
> > > --- a/t/t1307-config-blob.sh
> > > +++ b/t/t1307-config-blob.sh
> > > @@ -55,12 +55,17 @@ test_expect_success 'editing a blob is an error' '
> > >  test_expect_success 'parse errors in blobs are properly attributed' '
> > >  	cat >config <<-\EOF &&
> > >  	[some]
> > > -		value = "
> > > +		value = 1
> > > +		error = "
> > >  	EOF
> > >  	git add config &&
> > >  	git commit -m broken &&
> > >  
> > > -	test_must_fail git config --blob=HEAD:config some.value 2>err &&
> > > +	test_must_fail git config --blob=HEAD:config some.value 1>out 2>err &&
> > > +
> > > +	# Make sure there is no output of values on stdout
> > > +	touch out.expect &&
> > > +	test_cmp out.expect out &&
> > 
> > I'm not clear on what is being tested here. Were we outputting to stdout
> > before this patch (I would have thought our die() meant we did not.
> 
> We where not outputting to stdout before so the test is correct there as
> well. I extended the test when implementing the non-dying version of
> git_config_with_options() so I could see the test fail. If just
> returning the error it would still output the values read until then. So
> if you think that it belongs into the initial version of this test
> (maybe including some comment why we need an extra parseable value) I
> am happy to move it there.

>From what you wrote above, it sounds like we would expect git-config to

write out some.value before failing on some.error. But that isn't what the test is checking, is it? It is checking that nothing is output.

I do not think the output matters much either way when git-config has failed; if it returns a non-zero exit code, then the results of stdout cannot be trusted (after all, it may have been killed by signal after it had written out half of the output).

Still slightly puzzled...

-Peff