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

From: Mehul Jain <mehul.jain2029@gmail.com>
Date: Mar 30, 2016, 19:00 UTC
Message-ID: <CA+DCAeQPr2vxvm6MKiOLpDtmpC2d=RcvYhuFeimSn+xX2TAvtQ@mail.gmail.com>
In-Reply-To: <CAPig+cQ93+dCqJMRcQYSRHLDuYtwkeK_aSrfv2=2=g7ZhO85TQ@mail.gmail.com>

Hi Eric,

Thanks for the reviews on this series.

On Wed, Mar 30, 2016 at 2:46 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:

Show 5 quoted lines

> 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:

        * Introduce two function test_pull() and test_pull_fail() in
the place of
          test_rebase_autostash() and test_rebase_no_autostash.()

          Using these functions we can easily re-write all the 6 tests which
          deals with combination of autostash and rebase.autostash. Plus
          these functions helped in writing two new tests which deals with
          combination of pull.rebase and autostash. Thus reducing the code
          base to simpler and fewer lines of code. Also I could re-write one
          of the old test to reduce the repetition with them.

Here are the functions and there implementations:

---

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
}

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 --rebase --autostash & rebase.autostash=true" '
        test_config rebase.autostash true &&
        test_pull --rebase --autostash
'

test_expect_success "pull --rebase --autostash & rebase.autostash=false" '
        test_config rebase.autostash false &&
        test_pull --rebase --autostash
'

test_expect_success 'pull --rebase: --autostash & rebase.autostash unset' '
        test_unconfig rebase.autostash &&
        test_pull --rebase --autostash
'

test_expect_success "pull --rebase --no-autostash & rebase.autostash=true" '
        test_config rebase.autostash true &&
        test_pull_fail --rebase --no-autostash
'

test_expect_success "pull --rebase --no-autostash & rebase.autostash=false" '
        test_config rebase.autostash false &&
        test_pull_fail --rebase --no-autostash
'

test_expect_success 'pull --rebase --no-autostash & rebase.autostash unset' '
        test_unconfig rebase.autostash &&
        test_pull_fail --rebase --no-autostash
'

test_expect_success 'pull --autostash & pull.rebase=true' '
        test_config pull.rebase true &&
        test_pull --autostash
'

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.

Also I've read your comments on other patches of this series, I will make changes accordingly ones above mention functions, tests looks fit for a re-roll.

Thanks, Mehul