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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

> > > 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
--
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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]