Re: [PATCH v2 2/4] rebase: add tests for --no-rebase-merges

From: Alex Henrie <alexhenrie24@gmail.com>
Date: Feb 22, 2023, 01:37 UTC
Message-ID: <CAMMLpeSMO38acvnxK2QX18CdoaEiUojwLsV+AQZYOTe5h4Nq=Q@mail.gmail.com>
In-Reply-To: <24be0c50-9dfa-e9d9-9d78-1b993832884b@dunelm.org.uk>

On Tue, Feb 21, 2023 at 4:00 AM Phillip Wood <phillip.wood123@gmail.com> wrote:

Show 9 quoted lines

>
> On 21/02/2023 05:58, Alex Henrie wrote:
> > +test_expect_success 'do not rebase merges unless asked to' '
> > +     git checkout -b rebase-merges-default E &&
> > +     before="$(git rev-parse --verify HEAD)" &&
> > +     test_tick &&
> > +     git rebase --rebase-merges C &&
>
> I don't quite follow what this part of the test is for

The test is modeled after the existing test "do not rebase cousins unless asked for". First, it verifies that --rebase-merges rebases the merges, which in this case results in no changes to the branch. Then, it verifies that `git rebase` without arguments flattens the history.

Show 12 quoted lines

> > +test_expect_success '--no-rebase-merges countermands --rebase-merges' '
> > +     git checkout -b no-rebase-merges E &&
> > +     git rebase --rebase-merges --no-rebase-merges C &&
> > +     test_cmp_graph C.. <<-\EOF
> > +     * B
> > +     * D
> > +     o C
> > +     EOF
> > +'
>
> This test looks good. I think we could probably have just this second
> test

I like having a test for the default behavior. Nevertheless, I am happy to remove that test as requested. Does anyone else have an opinion?

> and squash this into the previous patch - improving the
> documentation and test coverage for --no-rebase-merges would make a nice
> self-contained commit.

Sure, squashing the first two patches is no problem.

Thanks for the feedback,

-Alex