Re: [PATCH v5 1/1] sequencer: finish parsing the todo list despite an invalid first line

> > +test_expect_success 'the first command cannot be a fixup' '
> > +     rebase_setup_and_clean fixup-first &&
> > +     (
> > +             cat >orig <<-EOF &&
> > +             fixup $(git log -1 --format="%h %s" B)
> > +             pick $(git log -1 --format="%h %s" C)
> > +             EOF
> > +
> > +             (
> > +                     set_replace_editor orig &&
> > +                     test_must_fail git rebase -i A 2>actual
> > +             ) &&
> > +             grep "cannot .fixup. without a previous commit" actual &&
> > +             grep "You can fix this with .git rebase --edit-todo.." actual &&
> > +             # verify that the todo list has not been truncated
> > +             grep -v "^#" .git/rebase-merge/git-rebase-todo >actual &&
> > +             test_cmp orig actual &&
> > +
> > +             test_must_fail git rebase --edit-todo 2>actual &&
> > +             grep "cannot .fixup. without a previous commit" actual &&
> > +             grep "You can fix this with .git rebase --edit-todo.." actual &&
> > +             # verify that the todo list has not been truncated
> > +             grep -v "^#" .git/rebase-merge/git-rebase-todo >actual &&
> > +             test_cmp orig actual
> > +     )
> > +'
>
> The structure of this new test piece, including the use of "log -1
> --format", seems to follow existing tests, and very readable.  Why
> do we have one extra level of subshell, though?  There is no "cd"
> that may affect the later test pieces, and set_something_editor that
> touches environment that may affect the later test pieces is called
> in its own subshell already.
>
> Other than that, looking good (there may be a valid reason why the
> test piece needs the subshell around it, but it was just not apparent
> to me).