Re: [PATCH] Show submodules as modified when they contain a dirty work tree

From: Nanako Shiraishi <nanako3@lavabit.com>
Date: Jan 15, 2010, 13:32 UTC
Message-ID: <20100115223221.6117@nanako3.lavabit.com>
In-Reply-To: <7vbpgyqy4a.fsf@alter.siamese.dyndns.org>

Quoting Jens Lehmann <Jens.Lehmann@web.de>

Show 17 quoted lines

> diff --git a/t/t7506-status-submodule.sh b/t/t7506-status-submodule.sh
> index 3ca17ab..47e205b 100755
> --- a/t/t7506-status-submodule.sh
> +++ b/t/t7506-status-submodule.sh
> @@ -10,8 +10,12 @@ test_expect_success 'setup' '
>  	: >bar &&
>  	git add bar &&
>  	git commit -m " Add bar" &&
> +	: >foo &&
> +	git add foo &&
> +	git commit -m " Add foo" &&
>  	cd .. &&
> -	git add sub &&
> +	echo output > .gitignore
> +	git add sub .gitignore &&
>  	git commit -m "Add submodule sub"
>  '

This is not a new problem you introduced, but if some commands before 'cd ..' fails, the next test will run in 'sub'. Other tests run operations inside () to avoid this problem.

Show 6 quoted lines

> @@ -23,6 +27,31 @@ test_expect_success 'commit --dry-run -a clean' '
>  	git commit --dry-run -a |
>  	grep "nothing to commit"
>  '
> +
> +echo "changed" > sub/foo

Have it inside the next test_expect_success.

> +test_expect_success 'status with modified file in submodule' '
> +	git status | grep "modified:   sub"
> +'

To catch failure from 'git status' this is better written like this.
    git status >output &&
    grep "modified:   sub" output

Show 6 quoted lines

> +test_expect_success 'status with modified file in submodule (porcelain)' '
> +	git status --porcelain >output &&
> +	diff output - <<-EOF
> + M sub
> +EOF
> +'

If you use -EOF you may want to align it with tab to make it easier to read. The one in t7005-editor.sh is a good example (t7401 is a bad example to imitate).

> +(cd sub && git checkout foo)
> +
> +echo "content" > sub/new-file

Move this part to the next test_expect_success to catch broken checkout.

> +test_expect_success 'status with untracked file in submodule' '
> +	git status | grep "modified:   sub"
> +'

Same comment as before.

Show 6 quoted lines

> +test_expect_success 'status with untracked file in submodule (porcelain)' '
> +	git status --porcelain >output &&
> +	diff output - <<-EOF
> + M sub
> +EOF
> +'

Same comment as before.

> +rm sub/new-file
> +

Do you need this? If so, move it inside the next test_expect_success.

Show 5 quoted lines

>  test_expect_success 'rm submodule contents' '
>  	rm -rf sub/* sub/.git
>  '
> -- 
> 1.6.6.203.g28a8ba.dirty

The following can be squashed to 4519d9cf092a173ac7b0a5570b0d5d602086ecf2

diff --git a/t/t7506-status-submodule.sh b/t/t7506-status-submodule.sh
index 47e205b..253c334 100755
--- a/t/t7506-status-submodule.sh
+++ b/t/t7506-status-submodule.sh
@@ -5,63 +5,87 @@ test_description='git status for submodule'
 . ./test-lib.sh
 
 test_expect_success 'setup' '
-	test_create_repo sub
-	cd sub &&
-	: >bar &&
-	git add bar &&
-	git commit -m " Add bar" &&
-	: >foo &&
-	git add foo &&
-	git commit -m " Add foo" &&
-	cd .. &&
-	echo output > .gitignore
+	test_create_repo sub &&
+	(
+		cd sub &&
+		: >bar &&
+		git add bar &&
+		git commit -m " Add bar" &&
+		: >foo &&
+		git add foo &&
+		git commit -m " Add foo"
+	) &&
+	echo output > .gitignore &&
 	git add sub .gitignore &&
 	git commit -m "Add submodule sub"
 '
 
 test_expect_success 'status clean' '
-	git status |
-	grep "nothing to commit"
+	git status >output &&
+	grep "nothing to commit" output
 '
+
 test_expect_success 'commit --dry-run -a clean' '
-	git commit --dry-run -a |
-	grep "nothing to commit"
+	test_must_fail git commit --dry-run -a >output &&
+	grep "nothing to commit" output
 '
 
-echo "changed" > sub/foo
 test_expect_success 'status with modified file in submodule' '
-	git status | grep "modified:   sub"
+	(cd sub && git reset --hard) &&
+	echo "changed" >sub/foo &&
+	git status >output &&
+	grep "modified:   sub" output
 '
+
 test_expect_success 'status with modified file in submodule (porcelain)' '
+	(cd sub && git reset --hard) &&
+	echo "changed" >sub/foo &&
+	git status --porcelain >output &&
+	diff output - <<-\EOF
+	 M sub
+	EOF
+'
+
+test_expect_success 'status with added file in submodule' '
+	(cd sub && git reset --hard && echo >foo && git add foo) &&
+	git status >output &&
+	grep "modified:   sub" output
+'
+
+test_expect_success 'status with added file in submodule (porcelain)' '
+	(cd sub && git reset --hard && echo >foo && git add foo) &&
 	git status --porcelain >output &&
-	diff output - <<-EOF
- M sub
-EOF
+	diff output - <<-\EOF
+	 M sub
+	EOF
 '
-(cd sub && git checkout foo)
 
-echo "content" > sub/new-file
 test_expect_success 'status with untracked file in submodule' '
-	git status | grep "modified:   sub"
+	(cd sub && git reset --hard) &&
+	echo "content" >sub/new-file &&
+	git status >output &&
+	grep "modified:   sub" output
 '
+
 test_expect_success 'status with untracked file in submodule (porcelain)' '
 	git status --porcelain >output &&
-	diff output - <<-EOF
- M sub
-EOF
+	diff output - <<-\EOF
+	 M sub
+	EOF
 '
-rm sub/new-file
 
 test_expect_success 'rm submodule contents' '
 	rm -rf sub/* sub/.git
 '
+
 test_expect_success 'status clean (empty submodule dir)' '
-	git status |
-	grep "nothing to commit"
+	git status >output &&
+	grep "nothing to commit" output
 '
+
 test_expect_success 'status -a clean (empty submodule dir)' '
-	git commit --dry-run -a |
-	grep "nothing to commit"
+	test_must_fail git commit --dry-run -a >output &&
+	grep "nothing to commit" output
 '
 
 test_done

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/