On Sat, Jun 8, 2013 at 8:19 AM, Ramkumar Ramachandra <artag...@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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to