Re: [PATCH] tests: fix autostash

From

Felipe Contreras <felipe.contreras@gmail.com>

Date

Jun 8, 2013, 14:01 UTC

Message-ID

<CAMP44s1p-VDMLNhoym5fH3XAn8CWqu2yqGq0mb+55qHOq-r_ww@mail.gmail.com>

In-Reply-To

<CALkWK0=qVXL=DtJzYRGFLQQh3_-cGC_t_zpaiTL9KVKN4--mVg@mail.gmail.com>

On Sat, Jun 8, 2013 at 8:19 AM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:

> Felipe Contreras wrote:

>> Doing 'rm -rf $dotest' is even worst than 'git rebase --abort',
>> because it relies on the implementation of 'git rebase', which might
>> need to remove more files than $dotest.
>
> Huh?  Tests aren't allowed to rely on how a command is implemented?
>
>   $ git grep test_path t
>
> Ofcourse they're implementation details.  Even in this very test, I
> check $dotest/autostash plenty of times.

The more the test relies on implementation details, the worst.

> Have you read rr/rebase-autostash?  The whole idea is to inject
> $dotest/autostash and teach various scripts about how their
> assumptions about $dotest have changed.
>
>> This wouldn't be a problem if the tests were implemented correctly,
>> but they are not, so 'git rebase --abort' is the only sane option.
>
> Then show me how to do it correctly.

Something like this.

diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh
index 6ba449b..b96a56a 100755
--- a/t/t3420-rebase-autostash.sh
+++ b/t/t3420-rebase-autostash.sh
@@ -33,54 +33,56 @@ test_expect_success setup '
 	git commit -m "related commit"
 '

+setup_tmp () {
+	git clone . tmp &&
+	cd tmp &&
+	git fetch -u origin "refs/heads/*:refs/heads/*" &&
+	test_config rebase.autostash true &&
+	git checkout -b rebased-feature-branch feature-branch
+}
+
 testrebase() {
 	type=$1
 	dotest=$2

 	test_expect_success "rebase$type: dirty worktree, non-conflicting rebase" '
-		test_config rebase.autostash true &&
-		git reset --hard &&
-		git checkout -b rebased-feature-branch feature-branch &&
-		test_when_finished git branch -D rebased-feature-branch &&
+		test_when_finished "rm -rf tmp" &&
+		(
+		setup_tmp &&
 		echo dirty >>file3 &&
 		git rebase$type unrelated-onto-branch &&
 		grep unrelated file4 &&
-		grep dirty file3 &&
-		git checkout feature-branch
+		grep dirty file3
+		)
 	'

 	test_expect_success "rebase$type: dirty index, non-conflicting rebase" '
-		test_config rebase.autostash true &&
-		git reset --hard &&
-		git checkout -b rebased-feature-branch feature-branch &&
-		test_when_finished git branch -D rebased-feature-branch &&
+		test_when_finished "rm -rf tmp" &&
+		(
+		setup_tmp &&
 		echo dirty >>file3 &&
 		git add file3 &&
 		git rebase$type unrelated-onto-branch &&
 		grep unrelated file4 &&
-		grep dirty file3 &&
-		git checkout feature-branch
+		grep dirty file3
+		)
 	'

 	test_expect_success "rebase$type: conflicting rebase" '
-		test_config rebase.autostash true &&
-		git reset --hard &&
-		git checkout -b rebased-feature-branch feature-branch &&
-		test_when_finished git branch -D rebased-feature-branch &&
+		test_when_finished "rm -rf tmp" &&
+		(
+		setup_tmp &&
 		echo dirty >>file3 &&
 		test_must_fail git rebase$type related-onto-branch &&
 		test_path_is_file $dotest/autostash &&
-		false ! grep dirty file3 &&
-		rm -rf $dotest &&
-		git reset --hard &&
-		git checkout feature-branch
+		! grep dirty file3
+		)
 	'

 	test_expect_success "rebase$type: --continue" '
-		test_config rebase.autostash true &&
-		git reset --hard &&
-		git checkout -b rebased-feature-branch feature-branch &&
-		test_when_finished git branch -D rebased-feature-branch &&
+		test_when_finished "rm -rf tmp" &&
+		(
+		setup_tmp &&
 		echo dirty >>file3 &&
 		test_must_fail git rebase$type related-onto-branch &&
 		test_path_is_file $dotest/autostash &&
@@ -89,45 +91,43 @@ testrebase() {
 		git add file2 &&
 		git rebase --continue &&
 		test_path_is_missing $dotest/autostash &&
-		grep dirty file3 &&
-		git checkout feature-branch
+		grep dirty file3
+		)
 	'

 	test_expect_success "rebase$type: --skip" '
-		test_config rebase.autostash true &&
+		test_when_finished "rm -rf tmp" &&
+		(
+		setup_tmp &&
 		git reset --hard &&
-		git checkout -b rebased-feature-branch feature-branch &&
-		test_when_finished git branch -D rebased-feature-branch &&
 		echo dirty >>file3 &&
 		test_must_fail git rebase$type related-onto-branch &&
 		test_path_is_file $dotest/autostash &&
 		! grep dirty file3 &&
 		git rebase --skip &&
 		test_path_is_missing $dotest/autostash &&
-		grep dirty file3 &&
-		git checkout feature-branch
+		grep dirty file3
+		)
 	'

 	test_expect_success "rebase$type: --abort" '
-		test_config rebase.autostash true &&
-		git reset --hard &&
-		git checkout -b rebased-feature-branch feature-branch &&
-		test_when_finished git branch -D rebased-feature-branch &&
+		test_when_finished "rm -rf tmp" &&
+		(
+		setup_tmp &&
 		echo dirty >>file3 &&
 		test_must_fail git rebase$type related-onto-branch &&
 		test_path_is_file $dotest/autostash &&
 		! grep dirty file3 &&
 		git rebase --abort &&
 		test_path_is_missing $dotest/autostash &&
-		grep dirty file3 &&
-		git checkout feature-branch
+		grep dirty file3
+		)
 	'

 	test_expect_success "rebase$type: non-conflicting rebase, conflicting stash" '
-		test_config rebase.autostash true &&
-		git reset --hard &&
-		git checkout -b rebased-feature-branch feature-branch &&
-		test_when_finished git branch -D rebased-feature-branch &&
+		test_when_finished "rm -rf tmp" &&
+		(
+		setup_tmp &&
 		echo dirty >file4 &&
 		git add file4 &&
 		git rebase$type unrelated-onto-branch &&
@@ -138,6 +138,7 @@ testrebase() {
 		git checkout feature-branch &&
 		git stash pop &&
 		grep dirty file4
+		)
 	'
 }

-- 
Felipe Contreras