Re: [PATCH 5/5] t/t5520: test --[no-]autostash with pull.rebase=true

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Apr 1, 2016 at 6:27 AM, Mehul Jain <mehul.jain2029@xxxxxxxxx> wrote:
> I tried out this method also. Below is the script that I wrote for this:
>
> test_autostash () {
>     OLDIFS=$IFS
>     IFS=',    ='
>     while read -r expect cmd config_variable value
>     do
>         test_expect_success "$cmd, $config_variable=$value" '
>             if [ "$value" = "" ]; then
>                 test_unconfig $config_variable
>             else
>                 test_config $config_variable $value
>             fi &&
>
>             git reset --hard before-rebase &&
>             echo dirty >new_file &&
>             git add new_file &&
>
>             if [ $expect = "ok" ]; then
>                 git pull '$cmd' . copy &&
>                 echo test_cmp_rev HEAD^ copy &&
>                 test "$(cat new_file)" = dirty &&
>                 test "$(cat file)" = "modified again"
>             else
>                 test_must_fail git pull '$cmd' . copy 2>err &&
>                 test_i18ngrep "uncommitted changes." err
>             fi
>         '
>     done
>     IFS=$OLDIFS
> }
>
> test_autostash <<-\EOF
>     ok,--rebase,rebase.autostash=true
>     [...]
>     err,--no-autostash,pull.rebase=true
>     EOF
>
> Things worked out perfectly.
>
> Unfortunately there was a strange behaviour that I noticed
> and frankly I don't understand why it happened.
>
> In test_autostash() there's a line
>
>     echo test_cmp_rev HEAD^ copy &&
>
> Originally it should have been
>
>     test_cmp_rev HEAD^ copy &&
>
> but this raise following error while testing
>
>     ./t5520-pull.sh: 684: eval: diff -u: not found

This is caused by the custom IFS=',\t=' which is still in effect when
test_cmp_rev() is invoked. You need to restore IFS within the loop
itself.

> I'm not able to understand why putting an "echo" before
> test_cmp didn't raise the above error. This looks quite
> strange. Any thoughts?

With 'echo', test_cmp_rev() is never even invoked (the command is just
printed by 'echo'), and 'echo' succeeds (returns 0), so the test
succeeds, but isn't actually doing an revision verification.

The test also behaves incorrectly on these lines:

    git pull '$cmd' . copy &&

and:

    test_must_fail git pull '$cmd' . copy 2>err &&

Those single quotes around $cmd are within the second argument to
test_expect_success(), which itself is a single quoted string. So,
'$cmd' is actually ending and re-starting the "outer" quoted string.
This isn't a problem when $cmd has a single token (such as
"--rebase"), but it causes test_expect_success() to complain about
incorrect number of arguments when $cmd is composed of multiple tokens
(such as "--rebase --autostash").

Moreover, $cmd shouldn't be quoted at all. When $cmd is "--rebase
--autostash", you want git-pull to see the --rebase and --autostash as
separate arguments, but the quoting causes them to be treated as a
single argument, which git-pull doesn't recognize. So, dropping the
quotes around $cmd is the correct thing to do.

> Though the above code works perfectly and can be used in
> place of previous tests. Only problem remains is tests titles.
> Currently with this script, test titles will be:
>
> ok 21 - --rebase, rebase.autostash=true
> ok 22 - --rebase --autostash, rebase.autostash=true
> ok 23 - --rebase --autostash, rebase.autostash=false
> ok 24 - --rebase --autostash, rebase.autostash=
> ok 25 - --rebase --no-autostash, rebase.autostash=true
> ok 26 - --rebase --no-autostash, rebase.autostash=false
> ok 27 - --rebase --no-autostash, rebase.autostash=
> ok 28 - --autostash, pull.rebase=true
> ok 29 - --no-autostash, pull.rebase=true
>
> Any thoughts/suggestions on them?

I don't see a particular problem with the titles. You could prefix
them with "pull " if that's what you mean.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]