git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH] Improve tests for detached worktree in git-submodule
@ 2012-07-30 16:10 Daniel Graña
  2012-07-30 16:39 ` Jeff King
  2012-07-30 17:02 ` Junio C Hamano
  0 siblings, 2 replies; 12+ messages in thread
From: Daniel Graña @ 2012-07-30 16:10 UTC (permalink / raw)
  To: Jeff King, Junio C Hamano; +Cc: git, Daniel Graña


Signed-off-by: Daniel Graña <dangra@gmail.com>
---
 t/t7409-submodule-detached-worktree.sh |   31 ++++++++++++++++++++++++-------
 1 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/t/t7409-submodule-detached-worktree.sh b/t/t7409-submodule-detached-worktree.sh
index db75642..d88f400 100755
--- a/t/t7409-submodule-detached-worktree.sh
+++ b/t/t7409-submodule-detached-worktree.sh
@@ -15,7 +15,11 @@ TEST_NO_CREATE_REPO=1
 test_expect_success 'submodule on detached working tree' '
 	git init --bare remote &&
 	test_create_repo bundle1 &&
-	(cd bundle1 && test_commit "shoot") &&
+	(
+		cd bundle1 &&
+		test_commit "shoot" &&
+		git rev-list --max-count=1 HEAD > "$TRASH_DIRECTORY/expect"
+	) &&
 	mkdir home &&
 	(
 		cd home &&
@@ -23,14 +27,27 @@ test_expect_success 'submodule on detached working tree' '
 		git clone --bare ../remote .dotfiles &&
 		git submodule add ../bundle1 .vim/bundle/sogood &&
 		test_commit "sogood" &&
+		(
+			unset GIT_WORK_TREE GIT_DIR &&
+			cd .vim/bundle/sogood &&
+			git rev-list --max-count=1 HEAD > actual &&
+			test_cmp actual "$TRASH_DIRECTORY/expect"
+		) &&
 		git push origin master
-	) &&
+	)
 	mkdir home2 &&
 	(
 		cd home2 &&
-		export GIT_WORK_TREE="$(pwd)" GIT_DIR="$(pwd)/.dotfiles" &&
 		git clone --bare ../remote .dotfiles &&
-		git submodule update --init
+		export GIT_WORK_TREE="$(pwd)" GIT_DIR="$(pwd)/.dotfiles" &&
+		git checkout master &&
+		git submodule update --init &&
+		(
+			unset GIT_WORK_TREE GIT_DIR &&
+			cd .vim/bundle/sogood &&
+			git rev-list --max-count=1 HEAD > actual &&
+			test_cmp actual "$TRASH_DIRECTORY/expect"
+		)
 	)
 '
 
@@ -42,6 +59,7 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '
 		git clone --bare ../remote "$GIT_DIR" &&
 		git config core.bare false &&
 		git config core.worktree .. &&
+		git checkout master &&
 		git submodule add ../bundle1 .vim/bundle/dupe &&
 		test_commit "dupe" &&
 		git push origin master
@@ -52,9 +70,8 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '
 		git config core.bare false &&
 		git config core.worktree .. &&
 		git pull &&
-		git submodule update &&
-		git submodule status &&
-		test -d .vim/bundle/dupe
+		git submodule update --init &&
+		test -e .vim/bundle/dupe/shoot.t
 	)
 '
 
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] Improve tests for detached worktree in git-submodule
  2012-07-30 16:10 [PATCH] Improve tests for detached worktree in git-submodule Daniel Graña
@ 2012-07-30 16:39 ` Jeff King
       [not found]   ` <CAHCkQtNyNGBm8Z8FP7BybVOW0zQNgpxjwW_akLepYfLc-U+0cg@mail.gmail.com>
  2012-07-30 17:02 ` Junio C Hamano
  1 sibling, 1 reply; 12+ messages in thread
From: Jeff King @ 2012-07-30 16:39 UTC (permalink / raw)
  To: Daniel Graña; +Cc: Junio C Hamano, git

On Mon, Jul 30, 2012 at 01:10:10PM -0300, Daniel Graña wrote:

> Subject: Re: [PATCH] Improve tests for detached worktree in git-submodule
> 
> Signed-off-by: Daniel Graña <dangra@gmail.com>

The space between the subject and your S-o-b is an excellent place to
explain the rationale for your commit.

How are we improving them? What cases or classes of failure does this
catch that the original did not?  It may be because I have not been
following this topic closely, but reading the patch, I am not sure what
the purpose is. Please make life easier for reviewers by telling us what
to expect and why before we even get to the patch.

-Peff

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Improve tests for detached worktree in git-submodule
  2012-07-30 16:10 [PATCH] Improve tests for detached worktree in git-submodule Daniel Graña
  2012-07-30 16:39 ` Jeff King
@ 2012-07-30 17:02 ` Junio C Hamano
  2012-07-30 17:18   ` Daniel Graña
  2012-07-30 17:43   ` Daniel Graña
  1 sibling, 2 replies; 12+ messages in thread
From: Junio C Hamano @ 2012-07-30 17:02 UTC (permalink / raw)
  To: Daniel Graña; +Cc: Jeff King, git

Daniel Graña <dangra@gmail.com> writes:

> Signed-off-by: Daniel Graña <dangra@gmail.com>
> ---
>  t/t7409-submodule-detached-worktree.sh |   31 ++++++++++++++++++++++++-------
>  1 files changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/t/t7409-submodule-detached-worktree.sh b/t/t7409-submodule-detached-worktree.sh
> index db75642..d88f400 100755
> --- a/t/t7409-submodule-detached-worktree.sh
> +++ b/t/t7409-submodule-detached-worktree.sh
> @@ -15,7 +15,11 @@ TEST_NO_CREATE_REPO=1
>  test_expect_success 'submodule on detached working tree' '
>  	git init --bare remote &&
>  	test_create_repo bundle1 &&
> -	(cd bundle1 && test_commit "shoot") &&
> +	(
> +		cd bundle1 &&
> +		test_commit "shoot" &&
> +		git rev-list --max-count=1 HEAD > "$TRASH_DIRECTORY/expect"

Better written as

	git rev-parse --verify HEAD >../expect

methinks.

> +	) &&
>  	mkdir home &&
>  	(
>  		cd home &&
> @@ -23,14 +27,27 @@ test_expect_success 'submodule on detached working tree' '
>  		git clone --bare ../remote .dotfiles &&
>  		git submodule add ../bundle1 .vim/bundle/sogood &&
>  		test_commit "sogood" &&
> +		(
> +			unset GIT_WORK_TREE GIT_DIR &&
> +			cd .vim/bundle/sogood &&
> +			git rev-list --max-count=1 HEAD > actual &&
> +			test_cmp actual "$TRASH_DIRECTORY/expect"

Likewise.

	git rev-parse --verify HEAD >actual &&
        test_cmp ../expect actual

As test_cmp turns into "diff -u", comparing expect against actual
(instead of the other way around) shows the deviation in a more
readable way when your test fails.

> +		) &&
>  		git push origin master
> -	) &&
> +	)
>  	mkdir home2 &&
>  	(
>  		cd home2 &&
> -		export GIT_WORK_TREE="$(pwd)" GIT_DIR="$(pwd)/.dotfiles" &&
>  		git clone --bare ../remote .dotfiles &&
> -		git submodule update --init
> +		export GIT_WORK_TREE="$(pwd)" GIT_DIR="$(pwd)/.dotfiles" &&

So you used to clone with the two environment variables in effect to
create the .otfiles, but now you don't use them while cloning.  That
makes more sense to me, especially the .otfiles is created "bare".

> +		git checkout master &&

So you populate the newly created home2 working tree out of the .otfiles
repository in it.

> +		git submodule update --init &&
> +		(
> +			unset GIT_WORK_TREE GIT_DIR &&
> +			cd .vim/bundle/sogood &&
> +			git rev-list --max-count=1 HEAD > actual &&
> +			test_cmp actual "$TRASH_DIRECTORY/expect"

Likewise.

> +		)
>  	)
>  '
>  
> @@ -42,6 +59,7 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '
>  		git clone --bare ../remote "$GIT_DIR" &&
>  		git config core.bare false &&
>  		git config core.worktree .. &&
> +		git checkout master &&
>  		git submodule add ../bundle1 .vim/bundle/dupe &&
>  		test_commit "dupe" &&
>  		git push origin master
> @@ -52,9 +70,8 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '
>  		git config core.bare false &&
>  		git config core.worktree .. &&
>  		git pull &&
> -		git submodule update &&
> -		git submodule status &&
> -		test -d .vim/bundle/dupe
> +		git submodule update --init &&
> +		test -e .vim/bundle/dupe/shoot.t

Is the "existence" the only thing you care about?  That's not all
that different from the old test that only checked the existence of
the directory dupe, no?

>  	)
>  '

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Fwd: [PATCH] Improve tests for detached worktree in git-submodule
       [not found]   ` <CAHCkQtNyNGBm8Z8FP7BybVOW0zQNgpxjwW_akLepYfLc-U+0cg@mail.gmail.com>
@ 2012-07-30 17:06     ` Daniel Graña
  0 siblings, 0 replies; 12+ messages in thread
From: Daniel Graña @ 2012-07-30 17:06 UTC (permalink / raw)
  To: git

Forwardning for the record as vger rejected my previous HTML email.

---------- Forwarded message ----------
From: Daniel Graña <dangra@gmail.com>
Date: Mon, Jul 30, 2012 at 2:04 PM
Subject: Re: [PATCH] Improve tests for detached worktree in git-submodule
To: Jeff King <peff@peff.net>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org


Hi Jeff,

I understand your complain and I must apologize for confusing you with Jens
from this thread
http://thread.gmane.org/gmane.comp.version-control.git/201851

This patch is a follow up to that thread (that I fail to include as
reply-to) and tries to address the issues noted by Junio and Jens there.



On Mon, Jul 30, 2012 at 1:39 PM, Jeff King <peff@peff.net> wrote:
>
> On Mon, Jul 30, 2012 at 01:10:10PM -0300, Daniel Graña wrote:
>
> > Subject: Re: [PATCH] Improve tests for detached worktree in
> > git-submodule
> >
> > Signed-off-by: Daniel Graña <dangra@gmail.com>
>
> The space between the subject and your S-o-b is an excellent place to
> explain the rationale for your commit.
>
> How are we improving them? What cases or classes of failure does this
> catch that the original did not?  It may be because I have not been
> following this topic closely, but reading the patch, I am not sure what
> the purpose is. Please make life easier for reviewers by telling us what
> to expect and why before we even get to the patch.
>
> -Peff

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Improve tests for detached worktree in git-submodule
  2012-07-30 17:02 ` Junio C Hamano
@ 2012-07-30 17:18   ` Daniel Graña
  2012-07-30 17:44     ` Junio C Hamano
  2012-07-30 17:43   ` Daniel Graña
  1 sibling, 1 reply; 12+ messages in thread
From: Daniel Graña @ 2012-07-30 17:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git

On Mon, Jul 30, 2012 at 2:02 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Daniel Graña <dangra@gmail.com> writes:
>
>> Signed-off-by: Daniel Graña <dangra@gmail.com>
>> ---
>>  t/t7409-submodule-detached-worktree.sh |   31 ++++++++++++++++++++++++-------
>>  1 files changed, 24 insertions(+), 7 deletions(-)
>>
>> diff --git a/t/t7409-submodule-detached-worktree.sh b/t/t7409-submodule-detached-worktree.sh
>> index db75642..d88f400 100755
>> --- a/t/t7409-submodule-detached-worktree.sh
>> +++ b/t/t7409-submodule-detached-worktree.sh
>> @@ -15,7 +15,11 @@ TEST_NO_CREATE_REPO=1
>>  test_expect_success 'submodule on detached working tree' '
>>       git init --bare remote &&
>>       test_create_repo bundle1 &&
>> -     (cd bundle1 && test_commit "shoot") &&
>> +     (
>> +             cd bundle1 &&
>> +             test_commit "shoot" &&
>> +             git rev-list --max-count=1 HEAD > "$TRASH_DIRECTORY/expect"
>
> Better written as
>
>         git rev-parse --verify HEAD >../expect
>
> methinks.

You rule here,

is it still better than "git rev-parse --max-count=1 HEAD" seen in
t7406-submodule.update.sh?

>
>> +     ) &&
>>       mkdir home &&
>>       (
>>               cd home &&
>> @@ -23,14 +27,27 @@ test_expect_success 'submodule on detached working tree' '
>>               git clone --bare ../remote .dotfiles &&
>>               git submodule add ../bundle1 .vim/bundle/sogood &&
>>               test_commit "sogood" &&
>> +             (
>> +                     unset GIT_WORK_TREE GIT_DIR &&
>> +                     cd .vim/bundle/sogood &&
>> +                     git rev-list --max-count=1 HEAD > actual &&
>> +                     test_cmp actual "$TRASH_DIRECTORY/expect"
>
> Likewise.
>
>         git rev-parse --verify HEAD >actual &&
>         test_cmp ../expect actual

I tried to avoid the too many ".." usage, in that case it'd be:

    test_cmp ../../../../expect actual

>
> As test_cmp turns into "diff -u", comparing expect against actual
> (instead of the other way around) shows the deviation in a more
> readable way when your test fails.

must agree, thanks.


>
>> +             ) &&
>>               git push origin master
>> -     ) &&
>> +     )
>>       mkdir home2 &&
>>       (
>>               cd home2 &&
>> -             export GIT_WORK_TREE="$(pwd)" GIT_DIR="$(pwd)/.dotfiles" &&
>>               git clone --bare ../remote .dotfiles &&
>> -             git submodule update --init
>> +             export GIT_WORK_TREE="$(pwd)" GIT_DIR="$(pwd)/.dotfiles" &&
>
> So you used to clone with the two environment variables in effect to
> create the .otfiles, but now you don't use them while cloning.  That
> makes more sense to me, especially the .otfiles is created "bare".
>
>> +             git checkout master &&
>
> So you populate the newly created home2 working tree out of the .otfiles
> repository in it.

right, before it wasn't creating ~/.gitmodules and "git subodule
update --init" wasn't taking effect.

>
>> +             git submodule update --init &&
>> +             (
>> +                     unset GIT_WORK_TREE GIT_DIR &&
>> +                     cd .vim/bundle/sogood &&
>> +                     git rev-list --max-count=1 HEAD > actual &&
>> +                     test_cmp actual "$TRASH_DIRECTORY/expect"
>
> Likewise.
>
>> +             )
>>       )
>>  '
>>
>> @@ -42,6 +59,7 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '
>>               git clone --bare ../remote "$GIT_DIR" &&
>>               git config core.bare false &&
>>               git config core.worktree .. &&
>> +             git checkout master &&
>>               git submodule add ../bundle1 .vim/bundle/dupe &&
>>               test_commit "dupe" &&
>>               git push origin master
>> @@ -52,9 +70,8 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '
>>               git config core.bare false &&
>>               git config core.worktree .. &&
>>               git pull &&
>> -             git submodule update &&
>> -             git submodule status &&
>> -             test -d .vim/bundle/dupe
>> +             git submodule update --init &&
>> +             test -e .vim/bundle/dupe/shoot.t
>
> Is the "existence" the only thing you care about?  That's not all
> that different from the old test that only checked the existence of
> the directory dupe, no?

Except the submodule wasn't updating but the directory still existed
so test passed, now it check for a file that exists only if the
submodule update works.
I check for file existence as a shortcut for checking for submodule
revision and because I am not sure what else too check honestly.

the real purpose of this test is the use or "core.worktree" instead of
GIT_WORK_TREE.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] Improve tests for detached worktree in git-submodule
  2012-07-30 17:02 ` Junio C Hamano
  2012-07-30 17:18   ` Daniel Graña
@ 2012-07-30 17:43   ` Daniel Graña
  2012-07-30 17:50     ` Daniel Graña
  1 sibling, 1 reply; 12+ messages in thread
From: Daniel Graña @ 2012-07-30 17:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Daniel Graña

* Check submodule is correctly initialized and updated after cloning .dotfiles

Signed-off-by: Daniel Graña <dangra@gmail.com>
---

Remove $TRASH_DIRECTORY and "git rev-parse --verify HEAD" as suggested by Junio

 t/t7409-submodule-detached-worktree.sh |   31 ++++++++++++++++++++++++-------
 1 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/t/t7409-submodule-detached-worktree.sh b/t/t7409-submodule-detached-worktree.sh
index db75642..1d5a4c5 100755
--- a/t/t7409-submodule-detached-worktree.sh
+++ b/t/t7409-submodule-detached-worktree.sh
@@ -15,7 +15,11 @@ TEST_NO_CREATE_REPO=1
 test_expect_success 'submodule on detached working tree' '
 	git init --bare remote &&
 	test_create_repo bundle1 &&
-	(cd bundle1 && test_commit "shoot") &&
+	(
+		cd bundle1 &&
+		test_commit "shoot" &&
+		git rev-parse --verify HEAD > ../expect
+	) &&
 	mkdir home &&
 	(
 		cd home &&
@@ -23,14 +27,27 @@ test_expect_success 'submodule on detached working tree' '
 		git clone --bare ../remote .dotfiles &&
 		git submodule add ../bundle1 .vim/bundle/sogood &&
 		test_commit "sogood" &&
+		(
+			unset GIT_WORK_TREE GIT_DIR &&
+			cd .vim/bundle/sogood &&
+			git rev-parse --verify HEAD > actual &&
+			test_cmp ../../../../expect actual
+		) &&
 		git push origin master
-	) &&
+	)
 	mkdir home2 &&
 	(
 		cd home2 &&
-		export GIT_WORK_TREE="$(pwd)" GIT_DIR="$(pwd)/.dotfiles" &&
 		git clone --bare ../remote .dotfiles &&
-		git submodule update --init
+		export GIT_WORK_TREE="$(pwd)" GIT_DIR="$(pwd)/.dotfiles" &&
+		git checkout master &&
+		git submodule update --init &&
+		(
+			unset GIT_WORK_TREE GIT_DIR &&
+			cd .vim/bundle/sogood &&
+			git rev-parse --verify HEAD > actual &&
+			test_cmp ../../../../expect actual
+		)
 	)
 '
 
@@ -42,6 +59,7 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '
 		git clone --bare ../remote "$GIT_DIR" &&
 		git config core.bare false &&
 		git config core.worktree .. &&
+		git checkout master &&
 		git submodule add ../bundle1 .vim/bundle/dupe &&
 		test_commit "dupe" &&
 		git push origin master
@@ -52,9 +70,8 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '
 		git config core.bare false &&
 		git config core.worktree .. &&
 		git pull &&
-		git submodule update &&
-		git submodule status &&
-		test -d .vim/bundle/dupe
+		git submodule update --init &&
+		test -e .vim/bundle/dupe/shoot.t
 	)
 '
 
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] Improve tests for detached worktree in git-submodule
  2012-07-30 17:18   ` Daniel Graña
@ 2012-07-30 17:44     ` Junio C Hamano
  2012-07-30 17:51       ` Junio C Hamano
  2012-07-30 17:51       ` Daniel Graña
  0 siblings, 2 replies; 12+ messages in thread
From: Junio C Hamano @ 2012-07-30 17:44 UTC (permalink / raw)
  To: Daniel Graña; +Cc: Jeff King, git

Daniel Graña <dangra@gmail.com> writes:

> On Mon, Jul 30, 2012 at 2:02 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Daniel Graña <dangra@gmail.com> writes:
>>
>>> Signed-off-by: Daniel Graña <dangra@gmail.com>
>>> ---
>>>  t/t7409-submodule-detached-worktree.sh |   31 ++++++++++++++++++++++++-------
>>>  1 files changed, 24 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/t/t7409-submodule-detached-worktree.sh b/t/t7409-submodule-detached-worktree.sh
>>> index db75642..d88f400 100755
>>> --- a/t/t7409-submodule-detached-worktree.sh
>>> +++ b/t/t7409-submodule-detached-worktree.sh
>>> @@ -15,7 +15,11 @@ TEST_NO_CREATE_REPO=1
>>>  test_expect_success 'submodule on detached working tree' '
>>>       git init --bare remote &&
>>>       test_create_repo bundle1 &&
>>> -     (cd bundle1 && test_commit "shoot") &&
>>> +     (
>>> +             cd bundle1 &&
>>> +             test_commit "shoot" &&
>>> +             git rev-list --max-count=1 HEAD > "$TRASH_DIRECTORY/expect"
>>
>> Better written as
>>
>>         git rev-parse --verify HEAD >../expect
>>
>> methinks.
>
> You rule here,
>
> is it still better than "git rev-parse --max-count=1 HEAD" seen in
> t7406-submodule.update.sh?

"git rev-parse --max-count=1 HEAD" will show "--max-count=1\n"
followed by the value of HEAD, so if the expected result and the
actual result were both prepared by that command, the comparison
would succeed (as the irrelevant --max-count=1 line will appear in
both output), but honestly, I do not think it makes any sense.

Who wrote that crap?

>
>>
>>> +     ) &&
>>>       mkdir home &&
>>>       (
>>>               cd home &&
>>> @@ -23,14 +27,27 @@ test_expect_success 'submodule on detached working tree' '
>>>               git clone --bare ../remote .dotfiles &&
>>>               git submodule add ../bundle1 .vim/bundle/sogood &&
>>>               test_commit "sogood" &&
>>> +             (
>>> +                     unset GIT_WORK_TREE GIT_DIR &&
>>> +                     cd .vim/bundle/sogood &&
>>> +                     git rev-list --max-count=1 HEAD > actual &&
>>> +                     test_cmp actual "$TRASH_DIRECTORY/expect"
>>
>> Likewise.
>>
>>         git rev-parse --verify HEAD >actual &&
>>         test_cmp ../expect actual
>
> I tried to avoid the too many ".." usage, in that case it'd be:
>
>     test_cmp ../../../../expect actual

"$TRASH_DIRECTORY/expect" is fine as well.  Just drop the extra SP
between the redirection '>' and the filename, and make sure the
filename is inside double quotes for some versions of bash that
issue an unnecessary warning.

>>> +             git checkout master &&
>>
>> So you populate the newly created home2 working tree out of the .otfiles
>> repository in it.
>
> right, before it wasn't creating ~/.gitmodules and "git subodule
> update --init" wasn't taking effect.

Good.

>> Is the "existence" the only thing you care about?  That's not all
>> that different from the old test that only checked the existence of
>> the directory dupe, no?
>
> Except the submodule wasn't updating but the directory still existed
> so test passed, now it check for a file that exists only if the
> submodule update works.

OK.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Improve tests for detached worktree in git-submodule
  2012-07-30 17:43   ` Daniel Graña
@ 2012-07-30 17:50     ` Daniel Graña
  0 siblings, 0 replies; 12+ messages in thread
From: Daniel Graña @ 2012-07-30 17:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Daniel Graña

On Mon, Jul 30, 2012 at 2:43 PM, Daniel Graña <dangra@gmail.com> wrote:
> * Check submodule is correctly initialized and updated after cloning .dotfiles
>
> Signed-off-by: Daniel Graña <dangra@gmail.com>
> ---

Ignore this patch, I am sending another without extra space after
standard output redirection.



>
> Remove $TRASH_DIRECTORY and "git rev-parse --verify HEAD" as suggested by Junio
>
>  t/t7409-submodule-detached-worktree.sh |   31 ++++++++++++++++++++++++-------
>  1 files changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/t/t7409-submodule-detached-worktree.sh b/t/t7409-submodule-detached-worktree.sh
> index db75642..1d5a4c5 100755
> --- a/t/t7409-submodule-detached-worktree.sh
> +++ b/t/t7409-submodule-detached-worktree.sh
> @@ -15,7 +15,11 @@ TEST_NO_CREATE_REPO=1
>  test_expect_success 'submodule on detached working tree' '
>         git init --bare remote &&
>         test_create_repo bundle1 &&
> -       (cd bundle1 && test_commit "shoot") &&
> +       (
> +               cd bundle1 &&
> +               test_commit "shoot" &&
> +               git rev-parse --verify HEAD > ../expect
> +       ) &&
>         mkdir home &&
>         (
>                 cd home &&
> @@ -23,14 +27,27 @@ test_expect_success 'submodule on detached working tree' '
>                 git clone --bare ../remote .dotfiles &&
>                 git submodule add ../bundle1 .vim/bundle/sogood &&
>                 test_commit "sogood" &&
> +               (
> +                       unset GIT_WORK_TREE GIT_DIR &&
> +                       cd .vim/bundle/sogood &&
> +                       git rev-parse --verify HEAD > actual &&
> +                       test_cmp ../../../../expect actual
> +               ) &&
>                 git push origin master
> -       ) &&
> +       )
>         mkdir home2 &&
>         (
>                 cd home2 &&
> -               export GIT_WORK_TREE="$(pwd)" GIT_DIR="$(pwd)/.dotfiles" &&
>                 git clone --bare ../remote .dotfiles &&
> -               git submodule update --init
> +               export GIT_WORK_TREE="$(pwd)" GIT_DIR="$(pwd)/.dotfiles" &&
> +               git checkout master &&
> +               git submodule update --init &&
> +               (
> +                       unset GIT_WORK_TREE GIT_DIR &&
> +                       cd .vim/bundle/sogood &&
> +                       git rev-parse --verify HEAD > actual &&
> +                       test_cmp ../../../../expect actual
> +               )
>         )
>  '
>
> @@ -42,6 +59,7 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '
>                 git clone --bare ../remote "$GIT_DIR" &&
>                 git config core.bare false &&
>                 git config core.worktree .. &&
> +               git checkout master &&
>                 git submodule add ../bundle1 .vim/bundle/dupe &&
>                 test_commit "dupe" &&
>                 git push origin master
> @@ -52,9 +70,8 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '
>                 git config core.bare false &&
>                 git config core.worktree .. &&
>                 git pull &&
> -               git submodule update &&
> -               git submodule status &&
> -               test -d .vim/bundle/dupe
> +               git submodule update --init &&
> +               test -e .vim/bundle/dupe/shoot.t
>         )
>  '
>
> --
> 1.7.5.4
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Improve tests for detached worktree in git-submodule
  2012-07-30 17:44     ` Junio C Hamano
@ 2012-07-30 17:51       ` Junio C Hamano
  2012-07-30 17:51       ` Daniel Graña
  1 sibling, 0 replies; 12+ messages in thread
From: Junio C Hamano @ 2012-07-30 17:51 UTC (permalink / raw)
  To: Daniel Graña; +Cc: Jeff King, git

Junio C Hamano <gitster@pobox.com> writes:

> Daniel Graña <dangra@gmail.com> writes:
> ...
>> is it still better than "git rev-parse --max-count=1 HEAD" seen in
>> t7406-submodule.update.sh?
>
> "git rev-parse --max-count=1 HEAD" will show "--max-count=1\n"
> followed by the value of HEAD, so if the expected result and the
> actual result were both prepared by that command, the comparison
> would succeed (as the irrelevant --max-count=1 line will appear in
> both output), but honestly, I do not think it makes any sense.

Thanks for noticing; I'll queue this.

-- >8 --
Subject: [PATCH] t7406: fix misleading "rev-parse --max-count=1 HEAD"

The test happened to use "rev-parse --max-count=1 HEAD" consistently
to prepare the expected output and the actual output, so the
comparison between them gave us a correct success/failure because
both output had irrelevant "--max-count=1" in it.

But that is not an excuse to keep it broken.  Replace it a more
meaningful construct "rev-parse --verify HEAD".

Noticed by Daniel Graña while working on his submodule tests.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t7406-submodule-update.sh | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 807f761..f4e6602 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -305,7 +305,7 @@ test_expect_success 'submodule update continues after checkout error' '
 	 git submodule init &&
 	 git commit -am "new_submodule" &&
 	 (cd submodule2 &&
-	  git rev-parse --max-count=1 HEAD > ../expect
+	  git rev-parse --verify HEAD >../expect
 	 ) &&
 	 (cd submodule &&
 	  test_commit "update_submodule" file
@@ -322,7 +322,7 @@ test_expect_success 'submodule update continues after checkout error' '
 	 git checkout HEAD^ &&
 	 test_must_fail git submodule update &&
 	 (cd submodule2 &&
-	  git rev-parse --max-count=1 HEAD > ../actual
+	  git rev-parse --verify HEAD >../actual
 	 ) &&
 	 test_cmp expect actual
 	)
@@ -351,7 +351,7 @@ test_expect_success 'submodule update continues after recursive checkout error'
 	  test_commit "update_submodule_again_again" file
 	 ) &&
 	 (cd submodule2 &&
-	  git rev-parse --max-count=1 HEAD > ../expect &&
+	  git rev-parse --verify HEAD >../expect &&
 	  test_commit "update_submodule2_again" file
 	 ) &&
 	 git add submodule &&
@@ -366,7 +366,7 @@ test_expect_success 'submodule update continues after recursive checkout error'
 	 ) &&
 	 test_must_fail git submodule update --recursive &&
 	 (cd submodule2 &&
-	  git rev-parse --max-count=1 HEAD > ../actual
+	  git rev-parse --verify HEAD >../actual
 	 ) &&
 	 test_cmp expect actual
 	)
@@ -398,12 +398,12 @@ test_expect_success 'submodule update exit immediately in case of merge conflict
 	 ) &&
 	 git checkout HEAD^ &&
 	 (cd submodule2 &&
-	  git rev-parse --max-count=1 HEAD > ../expect
+	  git rev-parse --verify HEAD >../expect
 	 ) &&
 	 git config submodule.submodule.update merge &&
 	 test_must_fail git submodule update &&
 	 (cd submodule2 &&
-	  git rev-parse --max-count=1 HEAD > ../actual
+	  git rev-parse --verify HEAD >../actual
 	 ) &&
 	 test_cmp expect actual
 	)
@@ -432,12 +432,12 @@ test_expect_success 'submodule update exit immediately after recursive rebase er
 	 ) &&
 	 git checkout HEAD^ &&
 	 (cd submodule2 &&
-	  git rev-parse --max-count=1 HEAD > ../expect
+	  git rev-parse --verify HEAD >../expect
 	 ) &&
 	 git config submodule.submodule.update rebase &&
 	 test_must_fail git submodule update &&
 	 (cd submodule2 &&
-	  git rev-parse --max-count=1 HEAD > ../actual
+	  git rev-parse --verify HEAD >../actual
 	 ) &&
 	 test_cmp expect actual
 	)
-- 
1.7.12.rc0.80.gb93a609

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH] Improve tests for detached worktree in git-submodule
  2012-07-30 17:44     ` Junio C Hamano
  2012-07-30 17:51       ` Junio C Hamano
@ 2012-07-30 17:51       ` Daniel Graña
  2012-07-30 18:09         ` Junio C Hamano
  1 sibling, 1 reply; 12+ messages in thread
From: Daniel Graña @ 2012-07-30 17:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Daniel Graña

* Check submodule is correctly initialized and updated after cloning .dotfiles

Signed-off-by: Daniel Graña <dangra@gmail.com>
---
 t/t7409-submodule-detached-worktree.sh |   31 ++++++++++++++++++++++++-------
 1 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/t/t7409-submodule-detached-worktree.sh b/t/t7409-submodule-detached-worktree.sh
index db75642..f82a757 100755
--- a/t/t7409-submodule-detached-worktree.sh
+++ b/t/t7409-submodule-detached-worktree.sh
@@ -15,7 +15,11 @@ TEST_NO_CREATE_REPO=1
 test_expect_success 'submodule on detached working tree' '
 	git init --bare remote &&
 	test_create_repo bundle1 &&
-	(cd bundle1 && test_commit "shoot") &&
+	(
+		cd bundle1 &&
+		test_commit "shoot" &&
+		git rev-parse --verify HEAD >../expect
+	) &&
 	mkdir home &&
 	(
 		cd home &&
@@ -23,14 +27,27 @@ test_expect_success 'submodule on detached working tree' '
 		git clone --bare ../remote .dotfiles &&
 		git submodule add ../bundle1 .vim/bundle/sogood &&
 		test_commit "sogood" &&
+		(
+			unset GIT_WORK_TREE GIT_DIR &&
+			cd .vim/bundle/sogood &&
+			git rev-parse --verify HEAD >actual &&
+			test_cmp ../../../../expect actual
+		) &&
 		git push origin master
-	) &&
+	)
 	mkdir home2 &&
 	(
 		cd home2 &&
-		export GIT_WORK_TREE="$(pwd)" GIT_DIR="$(pwd)/.dotfiles" &&
 		git clone --bare ../remote .dotfiles &&
-		git submodule update --init
+		export GIT_WORK_TREE="$(pwd)" GIT_DIR="$(pwd)/.dotfiles" &&
+		git checkout master &&
+		git submodule update --init &&
+		(
+			unset GIT_WORK_TREE GIT_DIR &&
+			cd .vim/bundle/sogood &&
+			git rev-parse --verify HEAD >actual &&
+			test_cmp ../../../../expect actual
+		)
 	)
 '
 
@@ -42,6 +59,7 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '
 		git clone --bare ../remote "$GIT_DIR" &&
 		git config core.bare false &&
 		git config core.worktree .. &&
+		git checkout master &&
 		git submodule add ../bundle1 .vim/bundle/dupe &&
 		test_commit "dupe" &&
 		git push origin master
@@ -52,9 +70,8 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '
 		git config core.bare false &&
 		git config core.worktree .. &&
 		git pull &&
-		git submodule update &&
-		git submodule status &&
-		test -d .vim/bundle/dupe
+		git submodule update --init &&
+		test -e .vim/bundle/dupe/shoot.t
 	)
 '
 
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] Improve tests for detached worktree in git-submodule
  2012-07-30 17:51       ` Daniel Graña
@ 2012-07-30 18:09         ` Junio C Hamano
  2012-07-30 18:15           ` Daniel Graña
  0 siblings, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2012-07-30 18:09 UTC (permalink / raw)
  To: Daniel Graña; +Cc: git

Daniel Graña <dangra@gmail.com> writes:

> * Check submodule is correctly initialized and updated after cloning .dotfiles
>
> Signed-off-by: Daniel Graña <dangra@gmail.com>
> ---

Thanks.

> @@ -23,14 +27,27 @@ test_expect_success 'submodule on detached working tree' '
>  		git clone --bare ../remote .dotfiles &&
>  		git submodule add ../bundle1 .vim/bundle/sogood &&
>  		test_commit "sogood" &&
> +		(
> +			unset GIT_WORK_TREE GIT_DIR &&
> +			cd .vim/bundle/sogood &&
> +			git rev-parse --verify HEAD >actual &&
> +			test_cmp ../../../../expect actual
> +		) &&
>  		git push origin master
> -	) &&
> +	)

I do not think you meant to break the && chain here on purpose.
I'll queue with a minor fix-up here.

>  	mkdir home2 &&
>  	(
>  		cd home2 &&
> -		export GIT_WORK_TREE="$(pwd)" GIT_DIR="$(pwd)/.dotfiles" &&
>  		git clone --bare ../remote .dotfiles &&
> -		git submodule update --init
> +		export GIT_WORK_TREE="$(pwd)" GIT_DIR="$(pwd)/.dotfiles" &&
> +		git checkout master &&
> +		git submodule update --init &&
> +		(
> +			unset GIT_WORK_TREE GIT_DIR &&
> +			cd .vim/bundle/sogood &&
> +			git rev-parse --verify HEAD >actual &&
> +			test_cmp ../../../../expect actual
> +		)
>  	)
>  '
>  
> @@ -42,6 +59,7 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '
>  		git clone --bare ../remote "$GIT_DIR" &&
>  		git config core.bare false &&
>  		git config core.worktree .. &&
> +		git checkout master &&
>  		git submodule add ../bundle1 .vim/bundle/dupe &&
>  		test_commit "dupe" &&
>  		git push origin master
> @@ -52,9 +70,8 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '
>  		git config core.bare false &&
>  		git config core.worktree .. &&
>  		git pull &&
> -		git submodule update &&
> -		git submodule status &&
> -		test -d .vim/bundle/dupe
> +		git submodule update --init &&
> +		test -e .vim/bundle/dupe/shoot.t
>  	)
>  '

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Improve tests for detached worktree in git-submodule
  2012-07-30 18:09         ` Junio C Hamano
@ 2012-07-30 18:15           ` Daniel Graña
  0 siblings, 0 replies; 12+ messages in thread
From: Daniel Graña @ 2012-07-30 18:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Mon, Jul 30, 2012 at 3:09 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Daniel Graña <dangra@gmail.com> writes:
>
>> * Check submodule is correctly initialized and updated after cloning .dotfiles
>>
>> Signed-off-by: Daniel Graña <dangra@gmail.com>
>> ---
>
> Thanks.
>
>> @@ -23,14 +27,27 @@ test_expect_success 'submodule on detached working tree' '
>>               git clone --bare ../remote .dotfiles &&
>>               git submodule add ../bundle1 .vim/bundle/sogood &&
>>               test_commit "sogood" &&
>> +             (
>> +                     unset GIT_WORK_TREE GIT_DIR &&
>> +                     cd .vim/bundle/sogood &&
>> +                     git rev-parse --verify HEAD >actual &&
>> +                     test_cmp ../../../../expect actual
>> +             ) &&
>>               git push origin master
>> -     ) &&
>> +     )
>
> I do not think you meant to break the && chain here on purpose.
> I'll queue with a minor fix-up here.

great, thanks.

>
>>       mkdir home2 &&
>>       (
>>               cd home2 &&
>> -             export GIT_WORK_TREE="$(pwd)" GIT_DIR="$(pwd)/.dotfiles" &&
>>               git clone --bare ../remote .dotfiles &&
>> -             git submodule update --init
>> +             export GIT_WORK_TREE="$(pwd)" GIT_DIR="$(pwd)/.dotfiles" &&
>> +             git checkout master &&
>> +             git submodule update --init &&
>> +             (
>> +                     unset GIT_WORK_TREE GIT_DIR &&
>> +                     cd .vim/bundle/sogood &&
>> +                     git rev-parse --verify HEAD >actual &&
>> +                     test_cmp ../../../../expect actual
>> +             )
>>       )
>>  '
>>
>> @@ -42,6 +59,7 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '
>>               git clone --bare ../remote "$GIT_DIR" &&
>>               git config core.bare false &&
>>               git config core.worktree .. &&
>> +             git checkout master &&
>>               git submodule add ../bundle1 .vim/bundle/dupe &&
>>               test_commit "dupe" &&
>>               git push origin master
>> @@ -52,9 +70,8 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '
>>               git config core.bare false &&
>>               git config core.worktree .. &&
>>               git pull &&
>> -             git submodule update &&
>> -             git submodule status &&
>> -             test -d .vim/bundle/dupe
>> +             git submodule update --init &&
>> +             test -e .vim/bundle/dupe/shoot.t
>>       )
>>  '

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2012-07-30 18:15 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-30 16:10 [PATCH] Improve tests for detached worktree in git-submodule Daniel Graña
2012-07-30 16:39 ` Jeff King
     [not found]   ` <CAHCkQtNyNGBm8Z8FP7BybVOW0zQNgpxjwW_akLepYfLc-U+0cg@mail.gmail.com>
2012-07-30 17:06     ` Fwd: " Daniel Graña
2012-07-30 17:02 ` Junio C Hamano
2012-07-30 17:18   ` Daniel Graña
2012-07-30 17:44     ` Junio C Hamano
2012-07-30 17:51       ` Junio C Hamano
2012-07-30 17:51       ` Daniel Graña
2012-07-30 18:09         ` Junio C Hamano
2012-07-30 18:15           ` Daniel Graña
2012-07-30 17:43   ` Daniel Graña
2012-07-30 17:50     ` Daniel Graña

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).