Re: [PATCH v0 2/3] t/t3400-rebase.sh: add more tests to help migrating git-rebase.sh to C

[Jakub Narebski <jnareb@gmail.com>]

Nguyễn Thái Ngọc Duy    <pclouds@gmail.com> writes:

> These new tests make sure I don't miss any check being performed before
> rebase is proceeded (which is well tested by other tests)
> 
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  t/t3400-rebase.sh |   28 ++++++++++++++++++++++++++++
>  1 files changed, 28 insertions(+), 0 deletions(-)
> 
> diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
> index 6e391a3..37f86ab 100755
> --- a/t/t3400-rebase.sh
> +++ b/t/t3400-rebase.sh
> @@ -41,9 +41,37 @@ test_expect_success \
>       git tag topic
>  '
>  
> +test_expect_success 'rebase on dirty worktree' '
> +     echo dirty >> A &&
> +     ! git rebase master'

Shouldn't you use test_must_fail instead? From t/test-lib.sh
(paraphrasing):

  Writing this as "! git rebase master" is wrong, because
  the failure could be due to a segv.  We want a controlled failure.

> +
> +test_expect_success 'rebase on dirty cache' '
> +     git add  A &&
> +     ! git rebase master'
> +
>  test_expect_success 'rebase against master' '
> +     git reset HEAD &&
> +     git checkout -f &&
>       git rebase master'
>  
> +test_expect_success 'rebase against master twice' '
> +	git rebase master 2>&1|grep "Current branch my-topic-branch is up to date\\."
> +'

It would be more readable to split this line on '|', i.e.:

+test_expect_success 'rebase against master twice' '
+	git rebase master 2>&1 |
+	grep "Current branch my-topic-branch is up to date\\."
+'

I think we prefer to save output to a file, and compare this file with
expected output (using test_cmp).  Additionally you can check if the
message you want is in correct stream (is in STDERR, and STDOUT is
empty).

> +
> +test_expect_success 'rebase against master twice with --force' '
> +	git rebase --force-rebase master 2>&1|grep "Current branch my-topic-branch is up to date, rebase forced"
> +'
> +
> +test_expect_success 'rebase against master twice from another branch' '
> +	git checkout my-topic-branch^ &&
> +	git rebase master my-topic-branch 2>&1|grep "Current branch my-topic-branch is up to date\\."
> +'
> +
> +test_expect_success 'rebase fast-forward to master' '
> +	git checkout my-topic-branch^ &&
> +	git rebase my-topic-branch 2>&1|grep "Fast-forwarded HEAD to my-topic-branch"
> +'

Same as above.

> +
>  test_expect_success \
>      'the rebase operation should not have destroyed author information' \
>      '! (git log | grep "Author:" | grep "<>")'

Errrr... what?  Why git-log and not git-cat-file?  Why grep twice?
Additionally you do not check here that author is unchanged, only that
is not destroyed.

-- 
Jakub Narebski
Poland
ShadeHawk on #git