Re: [PATCH v3 2/3] test: add test for --[no-]autostash flag
- From
Matthieu Moy <matthieu.moy@grenoble-inp.fr>
- Date
- Mar 3, 2016, 17:31 UTC
- Message-ID
- <vpq1t7rii3u.fsf@anie.imag.fr>
- In-Reply-To
- <1457021601-9099-2-git-send-email-mehul.jain2029@gmail.com>
Mehul Jain <mehul.jain2029@gmail.com> writes:
> Signed-off-by: Mehul Jain <mehul.jain2029@gmail.com> > --- > t/t5520-pull.sh | 19 +++++++++++++++++++ > t/t5521-pull-options.sh | 16 ++++++++++++++++
There's no need to split code/test/doc into separate patches, except if your patches are getting really huge. As a reviewer, I like having tests and doc in the same patch because they describe the intention of the programmer.
We try to have a history where each commit is equally good, and with your version there are two commits where --autostash exists and is undocumented (which is not catastrophic, though).
Show 18 quoted lines
> +test_expect_success 'pull --rebase --no-autostash fails with dirty working directory and rebase.autstash set true' ' > + test_config rebase.autostash true && > + git reset --hard before-rebase && > + echo dirty >new_file && > + git add new_file && > + test_must_fail git pull --rebase --no-autostash . copy > +' > + > +test_expect_success 'pull --rebase --autostash succeeds with dirty working directory and rebase.autstash set false' ' > + test_config rebase.autostash false && > + git reset --hard before-rebase && > + echo dirty >new_file && > + git add new_file && > + git pull --rebase --autostash . copy && > + test_cmp_rev HEAD^ copy && > + test "$(cat new_file)" = dirty && > + test "$(cat file)" = "modified again" > +'
Sounds good.
Show 21 quoted lines
> --- a/t/t5521-pull-options.sh > +++ b/t/t5521-pull-options.sh > @@ -62,6 +62,22 @@ test_expect_success 'git pull -v --rebase' ' > test_must_be_empty out) > ' > > +test_expect_success 'git pull --rebase --autostash' ' > + mkdir clonedrbas && > + (cd clonedrbas && git init && > + git pull --rebase --autostash "../parent" >out 2>err && > + test -s err && > + test_must_be_empty out) > +' > + > +test_expect_success 'git pull --rebase --no-autostash' ' > + mkdir clonedrbnas && > + (cd clonedrbnas && git init && > + git pull --rebase --no-autostash "../parent" >out 2>err && > + test -s err && > + test_must_be_empty out) > +'
Not sure these tests are needed if you have the ones in t/t5520-pull.sh. More tests means more time to run so testing twice the same thing has a cost.
-- Matthieu Moy http://www-verimag.imag.fr/~moy/