| Message ID | 20190308101655.9767-10-pclouds@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | And new command "restore" | expand |
On 3/8/19 11:16 AM, Nguyễn Thái Ngọc Duy wrote: > Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> > --- > t/lib-patch-mode.sh | 12 ++++ > t/t2070-restore.sh (new +x) | 77 ++++++++++++++++++++++ > t/t2071-restore-patch.sh (new +x) | 105 ++++++++++++++++++++++++++++++ > 3 files changed, 194 insertions(+) > > diff --git a/t/t2070-restore.sh b/t/t2070-restore.sh > new file mode 100755 > index 0000000000..df91bf54bc > --- /dev/null > +++ b/t/t2070-restore.sh > @@ -0,0 +1,77 @@ > +#!/bin/sh > + > +test_description='restore basic functionality' > + > +. ./test-lib.sh > + > +test_expect_success 'setup' ' > + test_commit first && > + echo first-and-a-half >>first.t && > + git add first.t && > + test_commit second && > + echo one >one && > + echo two >two && > + echo untracked >untracked && > + echo ignored >ignored && > + echo /ignored >.gitignore && > + git add one two .gitignore && > + git update-ref refs/heads/one master > +' > + [snip] > + > +test_expect_success 'restore a file, ignoring branch of same name' ' > + cat one >expected && > + echo dirty >>one && > + git restore one && > + test_cmp expected one > +' > + Branch 'one' has been created by update-ref invocation in the setup, OK. > +test_expect_success 'restore a file on worktree from another branch' ' > + test_when_finished git reset --hard && > + git cat-file blob first:./first.t >expected && > + git restore --source=first first.t && > + test_cmp expected first.t && > + git cat-file blob HEAD:./first.t >expected && > + git show :first.t >actual && > + test_cmp expected actual > +' Test description reads "from another branch". However "first", created by test_commit invocation is a tag. Maybe description should read "from another ref"? Same applies to other tests which utilize "first". > + > +test_expect_success 'restore a file in the index from another branch' ' > + test_when_finished git reset --hard && > + git cat-file blob first:./first.t >expected && > + git restore --source=first --index first.t && > + git show :first.t >actual && > + test_cmp expected actual && > + git cat-file blob HEAD:./first.t >expected && > + test_cmp expected first.t > +' > + > +test_expect_success 'restore a file in both the index and worktree from another branch' ' > + test_when_finished git reset --hard && > + git cat-file blob first:./first.t >expected && > + git restore --source=first --index --worktree first.t && > + git show :first.t >actual && > + test_cmp expected actual && > + test_cmp expected first.t > +' > + > +test_expect_success 'restore --index uses HEAD as source' ' > + test_when_finished git reset --hard && > + git cat-file blob :./first.t >expected && > + echo index-dirty >>first.t && > + git add first.t && > + git restore --index first.t && > + git cat-file blob :./first.t >actual && > + test_cmp expected actual > +' > + > +test_done -- Best regards, Andrei R.
Hi Duy, On Fri, 8 Mar 2019, Nguyễn Thái Ngọc Duy wrote: > diff --git a/t/t2070-restore.sh b/t/t2070-restore.sh > new file mode 100755 > index 0000000000..df91bf54bc > --- /dev/null > +++ b/t/t2070-restore.sh > @@ -0,0 +1,77 @@ > +#!/bin/sh > + > +test_description='restore basic functionality' > + > +. ./test-lib.sh > + > +test_expect_success 'setup' ' > + test_commit first && > + echo first-and-a-half >>first.t && > + git add first.t && > + test_commit second && > + echo one >one && > + echo two >two && > + echo untracked >untracked && > + echo ignored >ignored && > + echo /ignored >.gitignore && > + git add one two .gitignore && > + git update-ref refs/heads/one master > +' > + > +test_expect_success 'restore without pathspec is not ok' ' > + test_must_fail git restore && > + test_must_fail git restore --source=first > +' > + > +test_expect_success 'restore -p without pathspec is fine' ' > + echo q >cmd && > + git restore -p <cmd > +' This breaks with NO_PERL builds. See e.g. https://dev.azure.com/gitgitgadget/git/_build/results?buildId=4581 https://dev.azure.com/gitgitgadget/git/_build/results?buildId=4584 https://dev.azure.com/git/git/_build/results?buildId=386 You need this squashed in: diff --git a/t/t2070-restore.sh b/t/t2070-restore.sh index df91bf54bc06..f4766544c5de 100755 --- a/t/t2070-restore.sh +++ b/t/t2070-restore.sh @@ -23,7 +23,7 @@ test_expect_success 'restore without pathspec is not ok' ' test_must_fail git restore --source=first ' -test_expect_success 'restore -p without pathspec is fine' ' +test_expect_success PERL 'restore -p without pathspec is fine' ' echo q >cmd && git restore -p <cmd ' Junio, could you please add that as SQUASH??? on top of nd/switch-and-restore to unbreak the CI builds? Duy, have you thought about making use of the CI builds? You could catch those bugs before they hit the Git mailing list... Thanks, Dscho
On Wed, Mar 13, 2019 at 4:13 PM Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote: > Duy, have you thought about making use of the CI builds? You could catch > those bugs before they hit the Git mailing list... Using Azure? No.
Hi Duy, On Wed, 13 Mar 2019, Duy Nguyen wrote: > On Wed, Mar 13, 2019 at 4:13 PM Johannes Schindelin > <Johannes.Schindelin@gmx.de> wrote: > > Duy, have you thought about making use of the CI builds? You could catch > > those bugs before they hit the Git mailing list... > > Using Azure? No. This hostility is totally unnecessary. Especially since we still have Travis builds that you could also use to catch your bugs early, so that I would not have to catch them. You could also work on setting up Circle CI, or whatever else you have in mind. As long as you catch your bugs early. Thanks, Johannes
On Thu, Mar 14, 2019 at 5:17 AM Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote: > > Hi Duy, > > On Wed, 13 Mar 2019, Duy Nguyen wrote: > > > On Wed, Mar 13, 2019 at 4:13 PM Johannes Schindelin > > <Johannes.Schindelin@gmx.de> wrote: > > > Duy, have you thought about making use of the CI builds? You could catch > > > those bugs before they hit the Git mailing list... > > > > Using Azure? No. > > This hostility is totally unnecessary. Especially since we still have > Travis builds that you could also use to catch your bugs early, so that I > would not have to catch them. Thanks. I'll check back on github in a couple years. Adding CI to gitlab is on my todo list but it's very low.
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes: > diff --git a/t/t2070-restore.sh b/t/t2070-restore.sh > new file mode 100755 > index 0000000000..df91bf54bc > --- /dev/null > +++ b/t/t2070-restore.sh > @@ -0,0 +1,77 @@ > +#!/bin/sh > + > +test_description='restore basic functionality' > + > +. ./test-lib.sh > + ... > +test_expect_success 'restore -p without pathspec is fine' ' > + echo q >cmd && > + git restore -p <cmd > +' This stands out as a sore thumb, being an invocation with '-p' while all the other tests for the '-p' feature are in t2071. Shoudln't it move to somewhere near the beginning of 2071? > diff --git a/t/t2071-restore-patch.sh b/t/t2071-restore-patch.sh > new file mode 100755 > index 0000000000..46ebcb2413 > --- /dev/null > +++ b/t/t2071-restore-patch.sh
Junio C Hamano <gitster@pobox.com> writes: >> +test_expect_success 'restore -p without pathspec is fine' ' >> + echo q >cmd && >> + git restore -p <cmd >> +' > > This stands out as a sore thumb, being an invocation with '-p' while > all the other tests for the '-p' feature are in t2071. I'll have this inserted immediately after 09/11 in the meantime. I wonder if there is a clean way to make everything in 2071 with PERL prereq. I generally do not like the pattern used by some tests that manually check the prererq and have test_done early, though. Thanks. -- >8 -- From: Junio C Hamano <gitster@pobox.com> Date: Thu, 14 Mar 2019 14:48:47 +0900 Subject: [PATCH] SQUASH??? move -p test to 2071 from 2070 Signed-off-by: Junio C Hamano <gitster@pobox.com> --- t/t2070-restore.sh | 5 ----- t/t2071-restore-patch.sh | 5 +++++ 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/t/t2070-restore.sh b/t/t2070-restore.sh index df91bf54bc..3dc7032839 100755 --- a/t/t2070-restore.sh +++ b/t/t2070-restore.sh @@ -23,11 +23,6 @@ test_expect_success 'restore without pathspec is not ok' ' test_must_fail git restore --source=first ' -test_expect_success 'restore -p without pathspec is fine' ' - echo q >cmd && - git restore -p <cmd -' - test_expect_success 'restore a file, ignoring branch of same name' ' cat one >expected && echo dirty >>one && diff --git a/t/t2071-restore-patch.sh b/t/t2071-restore-patch.sh index 46ebcb2413..98b2476e7c 100755 --- a/t/t2071-restore-patch.sh +++ b/t/t2071-restore-patch.sh @@ -16,6 +16,11 @@ test_expect_success PERL 'setup' ' save_head ' +test_expect_success PERL 'restore -p without pathspec is fine' ' + echo q >cmd && + git restore -p <cmd +' + # note: bar sorts before dir/foo, so the first 'n' is always to skip 'bar' test_expect_success PERL 'saying "n" does nothing' '
diff --git a/t/lib-patch-mode.sh b/t/lib-patch-mode.sh index 06c3c91762..cfd76bf987 100644 --- a/t/lib-patch-mode.sh +++ b/t/lib-patch-mode.sh @@ -2,28 +2,40 @@ . ./test-lib.sh +# set_state <path> <worktree-content> <index-content> +# +# Prepare the content for path in worktree and the index as specified. set_state () { echo "$3" > "$1" && git add "$1" && echo "$2" > "$1" } +# save_state <path> +# +# Save index/worktree content of <path> in the files _worktree_<path> +# and _index_<path> save_state () { noslash="$(echo "$1" | tr / _)" && cat "$1" > _worktree_"$noslash" && git show :"$1" > _index_"$noslash" } +# set_and_save_state <path> <worktree-content> <index-content> set_and_save_state () { set_state "$@" && save_state "$1" } +# verify_state <path> <expected-worktree-content> <expected-index-content> verify_state () { test "$(cat "$1")" = "$2" && test "$(git show :"$1")" = "$3" } +# verify_saved_state <path> +# +# Call verify_state with expected contents from the last save_state verify_saved_state () { noslash="$(echo "$1" | tr / _)" && verify_state "$1" "$(cat _worktree_"$noslash")" "$(cat _index_"$noslash")" diff --git a/t/t2070-restore.sh b/t/t2070-restore.sh new file mode 100755 index 0000000000..df91bf54bc --- /dev/null +++ b/t/t2070-restore.sh @@ -0,0 +1,77 @@ +#!/bin/sh + +test_description='restore basic functionality' + +. ./test-lib.sh + +test_expect_success 'setup' ' + test_commit first && + echo first-and-a-half >>first.t && + git add first.t && + test_commit second && + echo one >one && + echo two >two && + echo untracked >untracked && + echo ignored >ignored && + echo /ignored >.gitignore && + git add one two .gitignore && + git update-ref refs/heads/one master +' + +test_expect_success 'restore without pathspec is not ok' ' + test_must_fail git restore && + test_must_fail git restore --source=first +' + +test_expect_success 'restore -p without pathspec is fine' ' + echo q >cmd && + git restore -p <cmd +' + +test_expect_success 'restore a file, ignoring branch of same name' ' + cat one >expected && + echo dirty >>one && + git restore one && + test_cmp expected one +' + +test_expect_success 'restore a file on worktree from another branch' ' + test_when_finished git reset --hard && + git cat-file blob first:./first.t >expected && + git restore --source=first first.t && + test_cmp expected first.t && + git cat-file blob HEAD:./first.t >expected && + git show :first.t >actual && + test_cmp expected actual +' + +test_expect_success 'restore a file in the index from another branch' ' + test_when_finished git reset --hard && + git cat-file blob first:./first.t >expected && + git restore --source=first --index first.t && + git show :first.t >actual && + test_cmp expected actual && + git cat-file blob HEAD:./first.t >expected && + test_cmp expected first.t +' + +test_expect_success 'restore a file in both the index and worktree from another branch' ' + test_when_finished git reset --hard && + git cat-file blob first:./first.t >expected && + git restore --source=first --index --worktree first.t && + git show :first.t >actual && + test_cmp expected actual && + test_cmp expected first.t +' + +test_expect_success 'restore --index uses HEAD as source' ' + test_when_finished git reset --hard && + git cat-file blob :./first.t >expected && + echo index-dirty >>first.t && + git add first.t && + git restore --index first.t && + git cat-file blob :./first.t >actual && + test_cmp expected actual +' + +test_done diff --git a/t/t2071-restore-patch.sh b/t/t2071-restore-patch.sh new file mode 100755 index 0000000000..46ebcb2413 --- /dev/null +++ b/t/t2071-restore-patch.sh @@ -0,0 +1,105 @@ +#!/bin/sh + +test_description='git restore --patch' + +. ./lib-patch-mode.sh + +test_expect_success PERL 'setup' ' + mkdir dir && + echo parent >dir/foo && + echo dummy >bar && + git add bar dir/foo && + git commit -m initial && + test_tick && + test_commit second dir/foo head && + set_and_save_state bar bar_work bar_index && + save_head +' + +# note: bar sorts before dir/foo, so the first 'n' is always to skip 'bar' + +test_expect_success PERL 'saying "n" does nothing' ' + set_and_save_state dir/foo work head && + test_write_lines n n | git restore -p && + verify_saved_state bar && + verify_saved_state dir/foo +' + +test_expect_success PERL 'git restore -p' ' + set_and_save_state dir/foo work head && + test_write_lines n y | git restore -p && + verify_saved_state bar && + verify_state dir/foo head head +' + +test_expect_success PERL 'git restore -p with staged changes' ' + set_state dir/foo work index && + test_write_lines n y | git restore -p && + verify_saved_state bar && + verify_state dir/foo index index +' + +test_expect_success PERL 'git restore -p --source=HEAD' ' + set_state dir/foo work index && + # the third n is to get out in case it mistakenly does not apply + test_write_lines n y n | git restore -p --source=HEAD && + verify_saved_state bar && + verify_state dir/foo head index +' + +test_expect_success PERL 'git restore -p --source=HEAD^' ' + set_state dir/foo work index && + # the third n is to get out in case it mistakenly does not apply + test_write_lines n y n | git restore -p --source=HEAD^ && + verify_saved_state bar && + verify_state dir/foo parent index +' + +test_expect_success PERL 'git restore -p handles deletion' ' + set_state dir/foo work index && + rm dir/foo && + test_write_lines n y | git restore -p && + verify_saved_state bar && + verify_state dir/foo index index +' + +# The idea in the rest is that bar sorts first, so we always say 'y' +# first and if the path limiter fails it'll apply to bar instead of +# dir/foo. There's always an extra 'n' to reject edits to dir/foo in +# the failure case (and thus get out of the loop). + +test_expect_success PERL 'path limiting works: dir' ' + set_state dir/foo work head && + test_write_lines y n | git restore -p dir && + verify_saved_state bar && + verify_state dir/foo head head +' + +test_expect_success PERL 'path limiting works: -- dir' ' + set_state dir/foo work head && + test_write_lines y n | git restore -p -- dir && + verify_saved_state bar && + verify_state dir/foo head head +' + +test_expect_success PERL 'path limiting works: HEAD^ -- dir' ' + set_state dir/foo work head && + # the third n is to get out in case it mistakenly does not apply + test_write_lines y n n | git restore -p --source=HEAD^ -- dir && + verify_saved_state bar && + verify_state dir/foo parent head +' + +test_expect_success PERL 'path limiting works: foo inside dir' ' + set_state dir/foo work head && + # the third n is to get out in case it mistakenly does not apply + test_write_lines y n n | (cd dir && git restore -p foo) && + verify_saved_state bar && + verify_state dir/foo head head +' + +test_expect_success PERL 'none of this moved HEAD' ' + verify_saved_head +' + +test_done
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- t/lib-patch-mode.sh | 12 ++++ t/t2070-restore.sh (new +x) | 77 ++++++++++++++++++++++ t/t2071-restore-patch.sh (new +x) | 105 ++++++++++++++++++++++++++++++ 3 files changed, 194 insertions(+)