Signed-Off-By: Steven Walter <stevenrwalter@gmail.com>
On Wed, Feb 22, 2012 at 12:08 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 104 quoted lines
> Steven Walter <stevenrwalter@gmail.com> writes:
>
>>>> + test -e "$SVN_TREE"/bar/zzz/yyy ' || true
>>>
>>> Care to explain what this " || true" is doing here, please?
>>
>> Ahh, good catch. I think the answer is that it shouldn't be there.
>> It was originally there because of the "test_must_fail" line, I think
>> (at least the other tests that use test_must_fail also have "||
>> true").
>
> Ok, that may explain the copy&paste error.
>
> But I do not think test_must_fail followed by || true makes much sense,
> either. The purpose of "test_must_fail" is to make sure the tested git
> command exits with non-zero status in a controlled way (i.e. not crash)
> so if the tested command that is expected to exit with non-zero status
> exited with zero status, the test has detected an *error*. E.g. if you
> know that the index and the working tree are different at one point in the
> test sequence, you would say:
>
> ... other setup steps ... &&
> test_must_fail git diff --exit-code &&
> ... and other tests ...
>
> so that failure by "git diff --exit-code" to exit with non-zero status
> (i.e. it did not find any difference when it should have) breaks the &&
> cascade.
>
> I just took a quick look at t9100 but I think all " || true" can be safely
> removed. None of them is associated with test_must_fail in any way. For
> whatever reason, these test seem to do
>
> test_expect_success 'label of the test' '
> body of the test
> ' || true
>
> for no good reason.
>
>> Do you want to just fix that up, or a new version of the original patch,
>> or a fix on top of the original patches?
>
> Eric queued the patch and then had me pull it as part of his history
> already, so it is doubly too late to replace it.
>
> Can you apply this patch and re-test?
>
>
> t/t9100-git-svn-basic.sh | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh
> index 4029f84..749b75e 100755
> --- a/t/t9100-git-svn-basic.sh
> +++ b/t/t9100-git-svn-basic.sh
> @@ -65,7 +65,8 @@ test_expect_success "$name" "
> git update-index --add dir/file/file &&
> git commit -m '$name' &&
> test_must_fail git svn set-tree --find-copies-harder --rmdir \
> - ${remotes_git_svn}..mybranch" || true
> + ${remotes_git_svn}..mybranch
> +"
>
>
> name='detect node change from directory to file #1'
> @@ -79,7 +80,8 @@ test_expect_success "$name" '
> git update-index --add -- bar &&
> git commit -m "$name" &&
> test_must_fail git svn set-tree --find-copies-harder --rmdir \
> - ${remotes_git_svn}..mybranch2' || true
> + ${remotes_git_svn}..mybranch2
> +'
>
>
> name='detect node change from file to directory #2'
> @@ -96,7 +98,8 @@ test_expect_success "$name" '
> ${remotes_git_svn}..mybranch3 &&
> svn_cmd up "$SVN_TREE" &&
> test -d "$SVN_TREE"/bar/zzz &&
> - test -e "$SVN_TREE"/bar/zzz/yyy ' || true
> + test -e "$SVN_TREE"/bar/zzz/yyy
> +'
>
> name='detect node change from directory to file #2'
> test_expect_success "$name" '
> @@ -109,7 +112,8 @@ test_expect_success "$name" '
> git update-index --add -- dir &&
> git commit -m "$name" &&
> test_must_fail git svn set-tree --find-copies-harder --rmdir \
> - ${remotes_git_svn}..mybranch4' || true
> + ${remotes_git_svn}..mybranch4
> +'
>
>
> name='remove executable bit from a file'
> @@ -162,7 +166,7 @@ test_expect_success "$name" '
>
> name='modify a symlink to become a file'
> test_expect_success "$name" '
> - echo git help > help || true &&
> + echo git help >help &&
> rm exec-2.sh &&
> cp help exec-2.sh &&
> git update-index exec-2.sh &&Steven Walter <stevenrwalter@gmail.com>