| Message ID | 20201204061623.1170745-15-felipe.contreras@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | pull: default warning improvements | expand |
On Thu, Dec 3, 2020 at 10:16 PM Felipe Contreras <felipe.contreras@gmail.com> wrote: > > Commit d18c950a69 changed these tests, but it's unclear why. Probably > because earlier versions of the patch series died instead of printing a > warning. Another case where log --pretty=reference notation would be more helpful in referring to commits. Looking up that commit, I see that commit made a number of other test changes which seem to be better motivated by the commit message and code changes. I wonder if we could make that clearer in the commit message somehow. Perhaps Commit d18c950a69 ("pull: warn if the user didn't say whether to rebase or to merge", 2020-03-09) changed a number of tests in t5521 and added some new tests in t7601, but it is not clear why the changes in t5521 were made... > > Cc: Alex Henrie <alexhenrie24@gmail.com> > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> > --- > t/t5521-pull-options.sh | 22 +++++++++++----------- > 1 file changed, 11 insertions(+), 11 deletions(-) > > diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh > index db1a381cd9..1a4fe2583a 100755 > --- a/t/t5521-pull-options.sh > +++ b/t/t5521-pull-options.sh > @@ -11,10 +11,10 @@ test_expect_success 'setup' ' > git commit -m one) > ' > > -test_expect_success 'git pull -q --no-rebase' ' > +test_expect_success 'git pull -q' ' > mkdir clonedq && > (cd clonedq && git init && > - git pull -q --no-rebase "../parent" >out 2>err && > + git pull -q "../parent" >out 2>err && > test_must_be_empty err && > test_must_be_empty out) > ' > @@ -30,10 +30,10 @@ test_expect_success 'git pull -q --rebase' ' > test_must_be_empty out) > ' > > -test_expect_success 'git pull --no-rebase' ' > +test_expect_success 'git pull' ' > mkdir cloned && > (cd cloned && git init && > - git pull --no-rebase "../parent" >out 2>err && > + git pull "../parent" >out 2>err && > test -s err && > test_must_be_empty out) > ' > @@ -46,10 +46,10 @@ test_expect_success 'git pull --rebase' ' > test_must_be_empty out) > ' > > -test_expect_success 'git pull -v --no-rebase' ' > +test_expect_success 'git pull -v' ' > mkdir clonedv && > (cd clonedv && git init && > - git pull -v --no-rebase "../parent" >out 2>err && > + git pull -v "../parent" >out 2>err && > test -s err && > test_must_be_empty out) > ' > @@ -62,25 +62,25 @@ test_expect_success 'git pull -v --rebase' ' > test_must_be_empty out) > ' > > -test_expect_success 'git pull -v -q --no-rebase' ' > +test_expect_success 'git pull -v -q' ' > mkdir clonedvq && > (cd clonedvq && git init && > - git pull -v -q --no-rebase "../parent" >out 2>err && > + git pull -v -q "../parent" >out 2>err && > test_must_be_empty out && > test_must_be_empty err) > ' > > -test_expect_success 'git pull -q -v --no-rebase' ' > +test_expect_success 'git pull -q -v' ' > mkdir clonedqv && > (cd clonedqv && git init && > - git pull -q -v --no-rebase "../parent" >out 2>err && > + git pull -q -v "../parent" >out 2>err && > test_must_be_empty out && > test -s err) > ' > test_expect_success 'git pull --cleanup errors early on invalid argument' ' > mkdir clonedcleanup && > (cd clonedcleanup && git init && > - test_must_fail git pull --no-rebase --cleanup invalid "../parent" >out 2>err && > + test_must_fail git pull --cleanup invalid "../parent" >out 2>err && > test_must_be_empty out && > test -s err) > ' > -- > 2.29.2 >
On Fri, Dec 4, 2020 at 5:49 PM Elijah Newren <newren@gmail.com> wrote: > > On Thu, Dec 3, 2020 at 10:16 PM Felipe Contreras > <felipe.contreras@gmail.com> wrote: > > > > Commit d18c950a69 changed these tests, but it's unclear why. Probably > > because earlier versions of the patch series died instead of printing a > > warning. > > Another case where log --pretty=reference notation would be more > helpful in referring to commits. > > Looking up that commit, I see that commit made a number of other test > changes which seem to be better motivated by the commit message and > code changes. I wonder if we could make that clearer in the commit > message somehow. Perhaps > > Commit d18c950a69 ("pull: warn if the user didn't say whether to > rebase or to merge", 2020-03-09) changed a number of tests in t5521 > and added some new tests in t7601, but it is not clear why the changes > in t5521 were made... Fine by me. I had a mind to do some mail archeology and find the likely reason myself, but we all have limited time on this Earth.
diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh index db1a381cd9..1a4fe2583a 100755 --- a/t/t5521-pull-options.sh +++ b/t/t5521-pull-options.sh @@ -11,10 +11,10 @@ test_expect_success 'setup' ' git commit -m one) ' -test_expect_success 'git pull -q --no-rebase' ' +test_expect_success 'git pull -q' ' mkdir clonedq && (cd clonedq && git init && - git pull -q --no-rebase "../parent" >out 2>err && + git pull -q "../parent" >out 2>err && test_must_be_empty err && test_must_be_empty out) ' @@ -30,10 +30,10 @@ test_expect_success 'git pull -q --rebase' ' test_must_be_empty out) ' -test_expect_success 'git pull --no-rebase' ' +test_expect_success 'git pull' ' mkdir cloned && (cd cloned && git init && - git pull --no-rebase "../parent" >out 2>err && + git pull "../parent" >out 2>err && test -s err && test_must_be_empty out) ' @@ -46,10 +46,10 @@ test_expect_success 'git pull --rebase' ' test_must_be_empty out) ' -test_expect_success 'git pull -v --no-rebase' ' +test_expect_success 'git pull -v' ' mkdir clonedv && (cd clonedv && git init && - git pull -v --no-rebase "../parent" >out 2>err && + git pull -v "../parent" >out 2>err && test -s err && test_must_be_empty out) ' @@ -62,25 +62,25 @@ test_expect_success 'git pull -v --rebase' ' test_must_be_empty out) ' -test_expect_success 'git pull -v -q --no-rebase' ' +test_expect_success 'git pull -v -q' ' mkdir clonedvq && (cd clonedvq && git init && - git pull -v -q --no-rebase "../parent" >out 2>err && + git pull -v -q "../parent" >out 2>err && test_must_be_empty out && test_must_be_empty err) ' -test_expect_success 'git pull -q -v --no-rebase' ' +test_expect_success 'git pull -q -v' ' mkdir clonedqv && (cd clonedqv && git init && - git pull -q -v --no-rebase "../parent" >out 2>err && + git pull -q -v "../parent" >out 2>err && test_must_be_empty out && test -s err) ' test_expect_success 'git pull --cleanup errors early on invalid argument' ' mkdir clonedcleanup && (cd clonedcleanup && git init && - test_must_fail git pull --no-rebase --cleanup invalid "../parent" >out 2>err && + test_must_fail git pull --cleanup invalid "../parent" >out 2>err && test_must_be_empty out && test -s err) '
Commit d18c950a69 changed these tests, but it's unclear why. Probably because earlier versions of the patch series died instead of printing a warning. Cc: Alex Henrie <alexhenrie24@gmail.com> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> --- t/t5521-pull-options.sh | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)