Re: [PATCH/RFC v2 3/4] fetch-pack: test cases for the new --stdin option

[Ivan Todoroski <grnch@gmx.net>]

On 27.03.2012 19:40, Junio C Hamano wrote:
> 
> It is sensible to expect that we see all the refs we told it to fetch, but
> I do not think it is sensible to require they come in the same order as we
> have given them to the command.

Oh, OK. I was not aware that fetch-pack is free to reorder commits (at 
least in principle). I will adjust the tests to be order-independent.

>> For the --stateless-rpc tests we cannot easily execute fetch-pack to the
>> end because that would require simulating the remote protocol. We settle
>> for only checking 2 cases:
>>
>> 1) Whether fetch-pack correctly fails to parse the refs if they are not
>> terminated by a flush packet
>>
>> 2) Whether fetch-pack finishes parsing the refs without error when they
>> are correctly terminated by a flush packet
>>
>> The fetch-pack invocation fails in both cases due to the missing remote
>> side of the protocol, but it fails in different ways which allows us to
>> determine how the refs parsing ended by inspecting the different error
>> messages.
> 
> Ick.

Yeah... I couldn't figure out a way to do an isolated test of the 
packetized version of --stdin when --stateless-rpc is also in effect. 
Any guidance here would be welcome.

On second thought, maybe we can just drop these two --stateless-rpc 
tests from this patch? The "git clone" test in the next patch also 
exercises the packetized refs in --stateless-rpc mode and if there was 
anything wrong with them it would fail.

>> +
>> +test_expect_success 'fetch refs from cmdline, make sure it still works OK' '
>> +	cd client &&
>> +	git fetch-pack --no-progress .. $(cat ../stdin.exp) |
>> +	cut -d " " -f 2 > ../stdin.act &&
>> +	cd .. &&
>> +	test_cmp stdin.exp stdin.act
>> +'
> 
>  - Do not chdir around without being in a subprocess ();

Sorry, I didn't realize the tests were eval-ed in the current 
environment. I will correct all such problems in the next version.

>  - Do not place the command you are testing that might crash on the
>    upstream of the pipe;
> 
>  - style;

Noted.

> 	(
> 		cd client &&
>                 git fetch-pack ... <../stdin.exp >stdin.raw
> 	) &&
> 	cut -d " " -f 2 <stdin.raw | sort >stdin.act &&
> 	test_cmp stdin.exp stdin.act
> 
> By the way, why are these not called "expect" and "actual" like most other
> tests?

The test files I worked with used the shorter exp/act convention so I 
followed that.

Or are you wondering about the "stdin." prefix I added? That was because 
I didn't want to overwrite any exp/act files that might be created by 
the previous tests, or is that not a concern?

Sorry, my inexperience with the Git test framework is showing...

> Do we want to have a separate test to see what happens when there are dups
> in the input?

Good idea. I will check what happens in the current fetch-pack if 
duplicate refs are passed on the command line and will write a test to 
ensure that the --stdin version does the same thing.


P.S.
It goes without saying that I implicitly accept all the corrections that 
I did not reply to and will include them in the next version.