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

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.

quoted

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.

quoted

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