From: Ivan Todoroski Date: Tue, 27 Mar 2012 23:36:45 GMT Subject: Re: [PATCH/RFC v2 3/4] fetch-pack: test cases for the new --stdin option Message-ID: <4F724F0D.7000602@gmx.net> In-Reply-To: <7v4ntaj61t.fsf@alter.siamese.dyndns.org> 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.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.