Re: [PATCH v3 1/4] t3431: add rebase --fork-point tests

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

 



Hi Hannes & Denton,


On Fri, 5 Apr 2019, Johannes Sixt wrote:

> Am 05.04.19 um 19:25 schrieb Denton Liu:
> > On Fri, Apr 05, 2019 at 04:55:37PM +0200, Johannes Schindelin wrote:
> >> On Mon, 1 Apr 2019, Denton Liu wrote:
> >>> +test_rebase() {
> >>> +	expected="$1" &&
> >>> +	shift &&
> >>> +	test_expect_success "git rebase $@" "
> >>> +		git checkout master &&
> >>> +		git reset --hard E &&
> >>> +		git checkout side &&
> >>> +		git reset --hard G &&
> >>> +		git rebase $@ &&
> >>
> >> I think we need this patch, to make the macOS build happy:

Actually, my patch did not even fix the build, I looked at the wrong
(succeeding) build, sorry for the noise.

> > Thanks for digging into this, both.
> >
> > Out of curiosity, t3432 is written similarly:
> >
> > 	test_rebase_same_head() {
> > 		status="$1" &&
> > 		shift &&
> > 		test_expect_$status "git rebase $@ with $changes is no-op" "
> > 			oldhead=\$(git rev-parse HEAD) &&
> > 			test_when_finished 'git reset --hard \$oldhead' &&
> > 			git rebase $@ &&
> > 			newhead=\$(git rev-parse HEAD) &&
> > 			test_cmp_rev \$oldhead \$newhead
> > 		"
> > 	}

That is curious, indeed!

> Using $@ in these expansions is wrong. You do not want to forward an
> argument list, but you want to construct a command line. $* is correct
> here. Then you can remove the single-quotes at the invocation, like so:
>
> 	test_rebase_same_head success
> 	test_rebase_same_head success --onto B B
>
> Function test_rebase() could be done in the same way, but the first
> argument, expected, still needs quotes at the call site, of course.

That's a good idea, let me run with it.

Ciao,
Dscho




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

  Powered by Linux