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 Wed, Mar 30, 2016 at 3:00 PM, Mehul Jain <mehul.jain2029@xxxxxxxxx> wrote:
> On Wed, Mar 30, 2016 at 2:46 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
>> With the exception of the missing --rebase argument, this is exactly
>> the same code as in test_rebase_autostash(), right? Rather than
>> repeating this code yet again, it might be nice to augment that
>> function to accept a (possibly) optional argument controlling whether
>> --rebase is used.
>
> Thanks for the idea. I have come up with something like this:
>
> test_pull () {
>         git reset --hard before-rebase &&
>         echo dirty >new_file &&
>         git add new_file &&
>         git pull $@ . copy &&
>         test_cmp_rev HEAD^ copy &&
>         test "$(cat new_file)" = dirty &&
>         test "$(cat file)" = "modified again"
> }
>
> test_pull_fail () {
>         git reset --hard before-rebase &&
>         echo dirty >new_file &&
>         git add new_file &&
>         test_must_fail git pull $@ . copy 2>err &&
>         test_i18ngrep "uncommitted changes." err
> }

Considering that these are specifically testing behavior related to
autostashing, it might make sense to have "autostash" in the function
names, but that's a very minor point.

> test_expect_success 'pull --rebase succeeds with dirty working
> directory and rebase.autostash set' '
>         test_config rebase.autostash true &&
>         test_pull --rebase
> '
> [...]
> test_expect_success 'pull --no-autostash & pull.rebase=true' '
>         test_config pull.rebase true &&
>         test_pull_fail --no-autostash
> '
>
> I'm sorry if this is bit difficult to digest without diff output. I
> just wanted to
> know if the above mention functions looks suitable to you.

This is exactly what I had in mind for simplifying the tests, and it's
perfectly easy to read in this form (a diff would be worse for this
illustration).

One other possibility would be to make this all table-driven by
collecting all of the above state information into a table and then
feeding that into a function (either as its argument list or via
stdin). For instance:

    test_autostash <<\-EOF
    ok,--rebase,rebase.autostash=true
    ok,--rebase --autostash,rebase.autostash=true
    ok,--rebase --autostash,rebase.autostash=false
    ok,--rebase --autostash,rebase.autostash=
    err,--rebase --no-autostash,rebase.autostash=true
    err,--rebase --no-autostash,rebase.autostash=false
    err,--rebase --no-autostash,rebase.autostash=
    ok,--autostash,pull.rebase=true
    err,--no-autostash,pull.rebase=true
   EOF

The function would loop over the input, split each line apart by
setting IFS=, and then run the test based upon the state information.
"ok" means autostash is expected to succeed, and err means it is
expected to fail. The function would want to specially recognize the
"foo.bar=" in the last argument in order to invoke test_unconfig()
rather than test_config().

However, this may be a case of diminishing returns. The tests as you
illustrated them are sufficiently simple and easy to grok that the
table-driven approach may not add much value (aside from making it
easier to see at a glance if any cases were omitted).
--
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]