Re: [PATCH] tests: fix autostash

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Jun 8, 2013, 10:24 UTC
Message-ID: <CAMP44s3rYXOmGWR3JM8xy8ON4pqDRhYWVMQqOAyODVr5myq+yg@mail.gmail.com>
In-Reply-To: <CALkWK0=OdcLDCd9OonUS6kBZhA=oz3HYJT8Eiqnh1A++s5yWww@mail.gmail.com>

On Fri, Jun 7, 2013 at 10:29 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:

Show 16 quoted lines

> Felipe Contreras wrote:
>> diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh
>> index a5e69f3..ff370a3 100755
>> --- a/t/t3420-rebase-autostash.sh
>> +++ b/t/t3420-rebase-autostash.sh
>> @@ -71,8 +71,7 @@ testrebase() {
>>                 test_must_fail git rebase$type related-onto-branch &&
>>                 test_path_is_file $dotest/autostash &&
>>                 ! grep dirty file3 &&
>> -               rm -rf $dotest &&
>> -               git reset --hard &&
>> +               git rebase --abort &&
>>                 git checkout feature-branch
>>         '
>
> Incorrect.  I don't assume that --abort works yet, in this test.

Yes you do. The rest of the tests expect that the previous rebase has been aborted.

In fact, all the tests depend on the previous test finishing correctly, which is not the way tests should be written.

--- a/t/t3420-rebase-autostash.sh
+++ b/t/t3420-rebase-autostash.sh
@@ -70,7 +70,7 @@ testrebase() {
                echo dirty >>file3 &&
                test_must_fail git rebase$type related-onto-branch &&
                test_path_is_file $dotest/autostash &&
-               ! grep dirty file3 &&
+               false ! grep dirty file3 &&
                rm -rf $dotest &&
                git reset --hard &&
                git checkout feature-branch

# failed 19 among 22 test(s)

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.

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.

-- 
Felipe Contreras