Re: [PATCH 2/2] receive-pack: support push-to-checkout hook

> Hi Junio,
> 
> On Mon, 1 Dec 2014, Junio C Hamano wrote:
> 
>> When receive.denyCurrentBranch is set to updateInstead, this hook
>> can be used to override the built-in "push-to-deploy" logic, which
>> insists that the working tree and the index must be unchanged
>> relative to HEAD.  The hook receives the commit with which the
>> tip of the current is going to be updated, and is responsible to
>> make any necessary changes to the working tree and to the index to
>> bring them to the desired state when the tip of the current branch
>> is updated to the new commit.
>>
>> For example, the hook can simply run "git read-tree -u -m HEAD $1"
>> to the workflow to emulate "'git fetch' going in the reverse
>> direction with 'git push'" better than the push-to-deploy logic, as
>> the two-tree form of "read-tree -u -m" is essentially the same as
>> "git checkout" that switches branches while keeping the local
>> changes in the working tree that do not interfere with the
>> difference between the branches.
> 
> I like it.
> 
> The only sad part is that the already huge test suite is enlarged by yet
> another extensive set of test cases (and those tests might not really
> need to be that extensive because they essentially only need to make sure
> that the hook is run successfully *instead* of trying to update the
> working directory, i.e. a simple 'touch yep' hook would have been enough).
> It starts to be painful to run the complete test suite, not only on
> Windows (where this has been a multi-hour endeavor for me for ages
> already). BuildHive (CloudBees' very kind offer of Jenkins CI for Open
> Source, integrated conveniently with GitHub) already takes over an hour to
> run the Git test suite – and BuildHive runs on Linux, not Windows!
> 
> That means that everytime I push into a GitHub Pull Request, I have to
> wait for a full hour to know whether everything is groovy.
> 
> Worse: when Git for Windows contributors (yes! they exist!) push into
> their Pull Requests, I have to wait for a full hour before I can merge,
> unless I want to merge something that the test suite did not validate!
> 
> So maybe it is time to start thinking about conciser tests that verify the
> bare minimum, especially for rarely exercised functionality? I mean,
> testing is always a balance between too much and too little. And at this
> point, I am afraid that several well-intended, but overly extensive tests
> increase the overall runtime of "make test" to a point where developers
> *avoid* running it, costing more time in the long run than necessary.
> 
> In this particular case, I think that we really, really *just* need to
> verify that the presence of the hook switches off the default behavior of
> updateInstead. *Nothing* else is needed to verify that this particular
> functionality hasn't regressed. I.e. something like:
> 
> +test_expect_success 'updateInstead with push-to-checkout hook' '
> +	rm -fr testrepo &&
> +	git clone . testrepo &&
> +	(
> +		cd testrepo &&
> +		echo unclean > path1 &&
> +		git config receive.denyCurrentBranch updateInstead &&
> +		echo 'touch yep' | write_script .git/hooks/push-to-checkout
> +	) &&
> +	git push testrepo HEAD^:refs/heads/master &&
> +	test unclean = $(cat testrepo/path1) &&
> +	test -f testrepo/yep
> +'
> 
> would be more appropriate (although it probably has one or three bugs,
> given that I wrote this in the mailer).
> 
> Ciao,
> Johannes
>