diff mbox series

[v4,5/6] core.fsyncobjectfiles: tests for batch mode

Message ID afb0028e79648c1f7be8d77df5c6d675bd27d983.1632176111.git.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series Implement a batched fsync option for core.fsyncObjectFiles | expand

Commit Message

Neeraj Singh (WINDOWS-SFS) Sept. 20, 2021, 10:15 p.m. UTC
From: Neeraj Singh <neerajsi@microsoft.com>

Add test cases to exercise batch mode for 'git add'
and 'git stash'. These tests ensure that the added
data winds up in the object database.

I verified the tests by introducing an incorrect rename
in do_sync_and_rename.

Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
---
 t/lib-unique-files.sh | 34 ++++++++++++++++++++++++++++++++++
 t/t3700-add.sh        | 11 +++++++++++
 t/t3903-stash.sh      | 14 ++++++++++++++
 3 files changed, 59 insertions(+)
 create mode 100644 t/lib-unique-files.sh

Comments

Ævar Arnfjörð Bjarmason Sept. 21, 2021, 11:54 p.m. UTC | #1
On Mon, Sep 20 2021, Neeraj Singh via GitGitGadget wrote:

> From: Neeraj Singh <neerajsi@microsoft.com>
>
> Add test cases to exercise batch mode for 'git add'
> and 'git stash'. These tests ensure that the added
> data winds up in the object database.
>
> I verified the tests by introducing an incorrect rename
> in do_sync_and_rename.
>
> Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
> ---
>  t/lib-unique-files.sh | 34 ++++++++++++++++++++++++++++++++++
>  t/t3700-add.sh        | 11 +++++++++++
>  t/t3903-stash.sh      | 14 ++++++++++++++
>  3 files changed, 59 insertions(+)
>  create mode 100644 t/lib-unique-files.sh
>
> diff --git a/t/lib-unique-files.sh b/t/lib-unique-files.sh
> new file mode 100644
> index 00000000000..a8a25eba61d
> --- /dev/null
> +++ b/t/lib-unique-files.sh
> @@ -0,0 +1,34 @@
> +# Helper to create files with unique contents
> +
> +test_create_unique_files_base__=$(date -u)
> +test_create_unique_files_counter__=0
> +
> +# Create multiple files with unique contents. Takes the number of
> +# directories, the number of files in each directory, and the base
> +# directory.
> +#
> +# test_create_unique_files 2 3 . -- Creates 2 directories with 3 files
> +#				    each in the specified directory, all
> +#				    with unique contents.
> +
> +test_create_unique_files() {
> +	test "$#" -ne 3 && BUG "3 param"
> +
> +	local dirs=$1
> +	local files=$2
> +	local basedir=$3
> +
> +	rm -rf $basedir >/dev/null

Why the >/dev/null? It's not a "-rfv", and any errors would go to
stderr.

> +		mkdir -p "$dir" > /dev/null

Ditto.

> +		for j in $(test_seq $files)
> +		do
> +			test_create_unique_files_counter__=$((test_create_unique_files_counter__ + 1))
> +			echo "$test_create_unique_files_base__.$test_create_unique_files_counter__"  >"$dir/file$j.txt"

Would be much more readable if we these variables were shorter.

But actually, why are we trying to create files as a function of "date
-u" at all? This is all in the trash directory, which is rm -rf'd beween
runs, why aren't names created with test_seq or whatever OK? I.e. just
1.txt, 2.txt....

> +test_expect_success 'stash with core.fsyncobjectfiles=batch' "
> +	test_create_unique_files 2 4 fsync-files &&
> +	git -c core.fsyncobjectfiles=batch stash push -u -- ./fsync-files/ &&
> +	rm -f fsynced_files &&
> +
> +	# The files were untracked, so use the third parent,
> +	# which contains the untracked files
> +	git ls-tree -r stash^3 -- ./fsync-files/ > fsynced_files &&
> +	test_line_count = 8 fsynced_files &&
> +	cat fsynced_files | awk '{print \$3}' | xargs -n1 git cat-file -e
> +"
> +
> +
>  test_expect_success 'stash -c stash.useBuiltin=false warning ' '
>  	expected="stash.useBuiltin support has been removed" &&

We really prefer our tests to create the same data each time if
possible, but as noted with the "date -u" comment above you're
explicitly bypassing that, but I still can't see why...
Neeraj Singh Sept. 22, 2021, 1:30 a.m. UTC | #2
On Tue, Sep 21, 2021 at 4:58 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
>
> On Mon, Sep 20 2021, Neeraj Singh via GitGitGadget wrote:
>
> > From: Neeraj Singh <neerajsi@microsoft.com>
> >
> > Add test cases to exercise batch mode for 'git add'
> > and 'git stash'. These tests ensure that the added
> > data winds up in the object database.
> >
> > I verified the tests by introducing an incorrect rename
> > in do_sync_and_rename.
> >
> > Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
> > ---
> >  t/lib-unique-files.sh | 34 ++++++++++++++++++++++++++++++++++
> >  t/t3700-add.sh        | 11 +++++++++++
> >  t/t3903-stash.sh      | 14 ++++++++++++++
> >  3 files changed, 59 insertions(+)
> >  create mode 100644 t/lib-unique-files.sh
> >
> > diff --git a/t/lib-unique-files.sh b/t/lib-unique-files.sh
> > new file mode 100644
> > index 00000000000..a8a25eba61d
> > --- /dev/null
> > +++ b/t/lib-unique-files.sh
> > @@ -0,0 +1,34 @@
> > +# Helper to create files with unique contents
> > +
> > +test_create_unique_files_base__=$(date -u)
> > +test_create_unique_files_counter__=0
> > +
> > +# Create multiple files with unique contents. Takes the number of
> > +# directories, the number of files in each directory, and the base
> > +# directory.
> > +#
> > +# test_create_unique_files 2 3 . -- Creates 2 directories with 3 files
> > +#                                each in the specified directory, all
> > +#                                with unique contents.
> > +
> > +test_create_unique_files() {
> > +     test "$#" -ne 3 && BUG "3 param"
> > +
> > +     local dirs=$1
> > +     local files=$2
> > +     local basedir=$3
> > +
> > +     rm -rf $basedir >/dev/null
>
> Why the >/dev/null? It's not a "-rfv", and any errors would go to
> stderr.

Will fix. Clearly I don't know UNIX very well.

>
> > +             mkdir -p "$dir" > /dev/null
>
> Ditto.

Will fix.

>
> > +             for j in $(test_seq $files)
> > +             do
> > +                     test_create_unique_files_counter__=$((test_create_unique_files_counter__ + 1))
> > +                     echo "$test_create_unique_files_base__.$test_create_unique_files_counter__"  >"$dir/file$j.txt"
>
> Would be much more readable if we these variables were shorter.
>
> But actually, why are we trying to create files as a function of "date
> -u" at all? This is all in the trash directory, which is rm -rf'd beween
> runs, why aren't names created with test_seq or whatever OK? I.e. just
> 1.txt, 2.txt....
>

The uniqueness is in the contents of the file.  I wanted to make sure that
we are really creating new objects and not reusing old ones.  Is the scope
of the "trash repo" small enough that I can be guaranteed that a new one
is created before my test since the last time I tried adding something to
the ODB?

> > +test_expect_success 'stash with core.fsyncobjectfiles=batch' "
> > +     test_create_unique_files 2 4 fsync-files &&
> > +     git -c core.fsyncobjectfiles=batch stash push -u -- ./fsync-files/ &&
> > +     rm -f fsynced_files &&
> > +
> > +     # The files were untracked, so use the third parent,
> > +     # which contains the untracked files
> > +     git ls-tree -r stash^3 -- ./fsync-files/ > fsynced_files &&
> > +     test_line_count = 8 fsynced_files &&
> > +     cat fsynced_files | awk '{print \$3}' | xargs -n1 git cat-file -e
> > +"
> > +
> > +
> >  test_expect_success 'stash -c stash.useBuiltin=false warning ' '
> >       expected="stash.useBuiltin support has been removed" &&
>
> We really prefer our tests to create the same data each time if
> possible, but as noted with the "date -u" comment above you're
> explicitly bypassing that, but I still can't see why...

I'm trying to make sure we get new object contents. Is there a better
way to achieve what I want without the risk of finding that the contents
are already in the database from a previous test run?

Thanks again for the thorough review,
-Neeraj
Ævar Arnfjörð Bjarmason Sept. 22, 2021, 1:58 a.m. UTC | #3
On Tue, Sep 21 2021, Neeraj Singh wrote:

> On Tue, Sep 21, 2021 at 4:58 PM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>>
>>
>> On Mon, Sep 20 2021, Neeraj Singh via GitGitGadget wrote:
>>
>> > From: Neeraj Singh <neerajsi@microsoft.com>
>> >
>> > Add test cases to exercise batch mode for 'git add'
>> > and 'git stash'. These tests ensure that the added
>> > data winds up in the object database.
>> >
>> > I verified the tests by introducing an incorrect rename
>> > in do_sync_and_rename.
>> >
>> > Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
>> > ---
>> >  t/lib-unique-files.sh | 34 ++++++++++++++++++++++++++++++++++
>> >  t/t3700-add.sh        | 11 +++++++++++
>> >  t/t3903-stash.sh      | 14 ++++++++++++++
>> >  3 files changed, 59 insertions(+)
>> >  create mode 100644 t/lib-unique-files.sh
>> >
>> > diff --git a/t/lib-unique-files.sh b/t/lib-unique-files.sh
>> > new file mode 100644
>> > index 00000000000..a8a25eba61d
>> > --- /dev/null
>> > +++ b/t/lib-unique-files.sh
>> > @@ -0,0 +1,34 @@
>> > +# Helper to create files with unique contents
>> > +
>> > +test_create_unique_files_base__=$(date -u)
>> > +test_create_unique_files_counter__=0
>> > +
>> > +# Create multiple files with unique contents. Takes the number of
>> > +# directories, the number of files in each directory, and the base
>> > +# directory.
>> > +#
>> > +# test_create_unique_files 2 3 . -- Creates 2 directories with 3 files
>> > +#                                each in the specified directory, all
>> > +#                                with unique contents.
>> > +
>> > +test_create_unique_files() {
>> > +     test "$#" -ne 3 && BUG "3 param"
>> > +
>> > +     local dirs=$1
>> > +     local files=$2
>> > +     local basedir=$3
>> > +
>> > +     rm -rf $basedir >/dev/null
>>
>> Why the >/dev/null? It's not a "-rfv", and any errors would go to
>> stderr.
>
> Will fix. Clearly I don't know UNIX very well.
>
>>
>> > +             mkdir -p "$dir" > /dev/null
>>
>> Ditto.
>
> Will fix.
>
>>
>> > +             for j in $(test_seq $files)
>> > +             do
>> > +                     test_create_unique_files_counter__=$((test_create_unique_files_counter__ + 1))
>> > +                     echo "$test_create_unique_files_base__.$test_create_unique_files_counter__"  >"$dir/file$j.txt"
>>
>> Would be much more readable if we these variables were shorter.
>>
>> But actually, why are we trying to create files as a function of "date
>> -u" at all? This is all in the trash directory, which is rm -rf'd beween
>> runs, why aren't names created with test_seq or whatever OK? I.e. just
>> 1.txt, 2.txt....
>>
>
> The uniqueness is in the contents of the file.  I wanted to make sure that
> we are really creating new objects and not reusing old ones.  Is the scope
> of the "trash repo" small enough that I can be guaranteed that a new one
> is created before my test since the last time I tried adding something to
> the ODB?
>
>> > +test_expect_success 'stash with core.fsyncobjectfiles=batch' "
>> > +     test_create_unique_files 2 4 fsync-files &&
>> > +     git -c core.fsyncobjectfiles=batch stash push -u -- ./fsync-files/ &&
>> > +     rm -f fsynced_files &&
>> > +
>> > +     # The files were untracked, so use the third parent,
>> > +     # which contains the untracked files
>> > +     git ls-tree -r stash^3 -- ./fsync-files/ > fsynced_files &&
>> > +     test_line_count = 8 fsynced_files &&
>> > +     cat fsynced_files | awk '{print \$3}' | xargs -n1 git cat-file -e
>> > +"
>> > +
>> > +
>> >  test_expect_success 'stash -c stash.useBuiltin=false warning ' '
>> >       expected="stash.useBuiltin support has been removed" &&
>>
>> We really prefer our tests to create the same data each time if
>> possible, but as noted with the "date -u" comment above you're
>> explicitly bypassing that, but I still can't see why...
>
> I'm trying to make sure we get new object contents. Is there a better
> way to achieve what I want without the risk of finding that the contents
> are already in the database from a previous test run?

You can just do something like:

test_expect_success 'setup data' '
	test_commit A &&
	test_commit B
'

Which will create files A.t, B.t etc, or create them via:

    obj=$(echo foo | git hash-object -w --stdin)

etc.

I.e. the uniqueness you're doing here seems to assume that tests are
re-using the same object store across runs, but we create a new trash
directory for each one, if you run the test with "-d" you can see it
being left behind for inspection. This is already ensured for the test.

The only potential caveat I can imagine is that some filesystem like say
btrfs-like that does some COW or object de-duplication would behave
differently, but other than that...
Neeraj Singh Sept. 22, 2021, 5:55 p.m. UTC | #4
On Tue, Sep 21, 2021 at 7:02 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
>
> On Tue, Sep 21 2021, Neeraj Singh wrote:
>
> > On Tue, Sep 21, 2021 at 4:58 PM Ævar Arnfjörð Bjarmason
> > <avarab@gmail.com> wrote:
> >>
> >>
> >> On Mon, Sep 20 2021, Neeraj Singh via GitGitGadget wrote:
> >>
> >> > From: Neeraj Singh <neerajsi@microsoft.com>
> >> >
> >> > Add test cases to exercise batch mode for 'git add'
> >> > and 'git stash'. These tests ensure that the added
> >> > data winds up in the object database.
> >> >
> >> > I verified the tests by introducing an incorrect rename
> >> > in do_sync_and_rename.
> >> >
> >> > Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
> >> > ---
> >> >  t/lib-unique-files.sh | 34 ++++++++++++++++++++++++++++++++++
> >> >  t/t3700-add.sh        | 11 +++++++++++
> >> >  t/t3903-stash.sh      | 14 ++++++++++++++
> >> >  3 files changed, 59 insertions(+)
> >> >  create mode 100644 t/lib-unique-files.sh
> >> >
> >> > diff --git a/t/lib-unique-files.sh b/t/lib-unique-files.sh
> >> > new file mode 100644
> >> > index 00000000000..a8a25eba61d
> >> > --- /dev/null
> >> > +++ b/t/lib-unique-files.sh
> >> > @@ -0,0 +1,34 @@
> >> > +# Helper to create files with unique contents
> >> > +
> >> > +test_create_unique_files_base__=$(date -u)
> >> > +test_create_unique_files_counter__=0
> >> > +
> >> > +# Create multiple files with unique contents. Takes the number of
> >> > +# directories, the number of files in each directory, and the base
> >> > +# directory.
> >> > +#
> >> > +# test_create_unique_files 2 3 . -- Creates 2 directories with 3 files
> >> > +#                                each in the specified directory, all
> >> > +#                                with unique contents.
> >> > +
> >> > +test_create_unique_files() {
> >> > +     test "$#" -ne 3 && BUG "3 param"
> >> > +
> >> > +     local dirs=$1
> >> > +     local files=$2
> >> > +     local basedir=$3
> >> > +
> >> > +     rm -rf $basedir >/dev/null
> >>
> >> Why the >/dev/null? It's not a "-rfv", and any errors would go to
> >> stderr.
> >
> > Will fix. Clearly I don't know UNIX very well.
> >
> >>
> >> > +             mkdir -p "$dir" > /dev/null
> >>
> >> Ditto.
> >
> > Will fix.
> >
> >>
> >> > +             for j in $(test_seq $files)
> >> > +             do
> >> > +                     test_create_unique_files_counter__=$((test_create_unique_files_counter__ + 1))
> >> > +                     echo "$test_create_unique_files_base__.$test_create_unique_files_counter__"  >"$dir/file$j.txt"
> >>
> >> Would be much more readable if we these variables were shorter.
> >>
> >> But actually, why are we trying to create files as a function of "date
> >> -u" at all? This is all in the trash directory, which is rm -rf'd beween
> >> runs, why aren't names created with test_seq or whatever OK? I.e. just
> >> 1.txt, 2.txt....
> >>
> >
> > The uniqueness is in the contents of the file.  I wanted to make sure that
> > we are really creating new objects and not reusing old ones.  Is the scope
> > of the "trash repo" small enough that I can be guaranteed that a new one
> > is created before my test since the last time I tried adding something to
> > the ODB?
> >
> >> > +test_expect_success 'stash with core.fsyncobjectfiles=batch' "
> >> > +     test_create_unique_files 2 4 fsync-files &&
> >> > +     git -c core.fsyncobjectfiles=batch stash push -u -- ./fsync-files/ &&
> >> > +     rm -f fsynced_files &&
> >> > +
> >> > +     # The files were untracked, so use the third parent,
> >> > +     # which contains the untracked files
> >> > +     git ls-tree -r stash^3 -- ./fsync-files/ > fsynced_files &&
> >> > +     test_line_count = 8 fsynced_files &&
> >> > +     cat fsynced_files | awk '{print \$3}' | xargs -n1 git cat-file -e
> >> > +"
> >> > +
> >> > +
> >> >  test_expect_success 'stash -c stash.useBuiltin=false warning ' '
> >> >       expected="stash.useBuiltin support has been removed" &&
> >>
> >> We really prefer our tests to create the same data each time if
> >> possible, but as noted with the "date -u" comment above you're
> >> explicitly bypassing that, but I still can't see why...
> >
> > I'm trying to make sure we get new object contents. Is there a better
> > way to achieve what I want without the risk of finding that the contents
> > are already in the database from a previous test run?
>
> You can just do something like:
>
> test_expect_success 'setup data' '
>         test_commit A &&
>         test_commit B
> '
>
> Which will create files A.t, B.t etc, or create them via:
>
>     obj=$(echo foo | git hash-object -w --stdin)
>
> etc.
>
> I.e. the uniqueness you're doing here seems to assume that tests are
> re-using the same object store across runs, but we create a new trash
> directory for each one, if you run the test with "-d" you can see it
> being left behind for inspection. This is already ensured for the test.
>
> The only potential caveat I can imagine is that some filesystem like say
> btrfs-like that does some COW or object de-duplication would behave
> differently, but other than that...

It looks like the same repo is reused for each test_expect_success
line in the top-level t*.sh script.
So for test_create_unique_files to be maximally useful, it should have
some state that is different for
each invocation.  How about I use the test_tick mechanism to produce
this uniqueness?  It wouldn't
be globally unique like the date method, but it should be good enough
if the repo is recycled every time
test-lib is reinitialized.

I'm changing lib-unique-files to use test_tick and to be a little more
readable as you suggested. Please
let me know if you have any other suggestions.
Ævar Arnfjörð Bjarmason Sept. 22, 2021, 8:01 p.m. UTC | #5
On Wed, Sep 22 2021, Neeraj Singh wrote:

> On Tue, Sep 21, 2021 at 7:02 PM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>>
>>
>> On Tue, Sep 21 2021, Neeraj Singh wrote:
>>
>> > On Tue, Sep 21, 2021 at 4:58 PM Ævar Arnfjörð Bjarmason
>> > <avarab@gmail.com> wrote:
>> >>
>> >>
>> >> On Mon, Sep 20 2021, Neeraj Singh via GitGitGadget wrote:
>> >>
>> >> > From: Neeraj Singh <neerajsi@microsoft.com>
>> >> >
>> >> > Add test cases to exercise batch mode for 'git add'
>> >> > and 'git stash'. These tests ensure that the added
>> >> > data winds up in the object database.
>> >> >
>> >> > I verified the tests by introducing an incorrect rename
>> >> > in do_sync_and_rename.
>> >> >
>> >> > Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
>> >> > ---
>> >> >  t/lib-unique-files.sh | 34 ++++++++++++++++++++++++++++++++++
>> >> >  t/t3700-add.sh        | 11 +++++++++++
>> >> >  t/t3903-stash.sh      | 14 ++++++++++++++
>> >> >  3 files changed, 59 insertions(+)
>> >> >  create mode 100644 t/lib-unique-files.sh
>> >> >
>> >> > diff --git a/t/lib-unique-files.sh b/t/lib-unique-files.sh
>> >> > new file mode 100644
>> >> > index 00000000000..a8a25eba61d
>> >> > --- /dev/null
>> >> > +++ b/t/lib-unique-files.sh
>> >> > @@ -0,0 +1,34 @@
>> >> > +# Helper to create files with unique contents
>> >> > +
>> >> > +test_create_unique_files_base__=$(date -u)
>> >> > +test_create_unique_files_counter__=0
>> >> > +
>> >> > +# Create multiple files with unique contents. Takes the number of
>> >> > +# directories, the number of files in each directory, and the base
>> >> > +# directory.
>> >> > +#
>> >> > +# test_create_unique_files 2 3 . -- Creates 2 directories with 3 files
>> >> > +#                                each in the specified directory, all
>> >> > +#                                with unique contents.
>> >> > +
>> >> > +test_create_unique_files() {
>> >> > +     test "$#" -ne 3 && BUG "3 param"
>> >> > +
>> >> > +     local dirs=$1
>> >> > +     local files=$2
>> >> > +     local basedir=$3
>> >> > +
>> >> > +     rm -rf $basedir >/dev/null
>> >>
>> >> Why the >/dev/null? It's not a "-rfv", and any errors would go to
>> >> stderr.
>> >
>> > Will fix. Clearly I don't know UNIX very well.
>> >
>> >>
>> >> > +             mkdir -p "$dir" > /dev/null
>> >>
>> >> Ditto.
>> >
>> > Will fix.
>> >
>> >>
>> >> > +             for j in $(test_seq $files)
>> >> > +             do
>> >> > +                     test_create_unique_files_counter__=$((test_create_unique_files_counter__ + 1))
>> >> > +                     echo "$test_create_unique_files_base__.$test_create_unique_files_counter__"  >"$dir/file$j.txt"
>> >>
>> >> Would be much more readable if we these variables were shorter.
>> >>
>> >> But actually, why are we trying to create files as a function of "date
>> >> -u" at all? This is all in the trash directory, which is rm -rf'd beween
>> >> runs, why aren't names created with test_seq or whatever OK? I.e. just
>> >> 1.txt, 2.txt....
>> >>
>> >
>> > The uniqueness is in the contents of the file.  I wanted to make sure that
>> > we are really creating new objects and not reusing old ones.  Is the scope
>> > of the "trash repo" small enough that I can be guaranteed that a new one
>> > is created before my test since the last time I tried adding something to
>> > the ODB?
>> >
>> >> > +test_expect_success 'stash with core.fsyncobjectfiles=batch' "
>> >> > +     test_create_unique_files 2 4 fsync-files &&
>> >> > +     git -c core.fsyncobjectfiles=batch stash push -u -- ./fsync-files/ &&
>> >> > +     rm -f fsynced_files &&
>> >> > +
>> >> > +     # The files were untracked, so use the third parent,
>> >> > +     # which contains the untracked files
>> >> > +     git ls-tree -r stash^3 -- ./fsync-files/ > fsynced_files &&
>> >> > +     test_line_count = 8 fsynced_files &&
>> >> > +     cat fsynced_files | awk '{print \$3}' | xargs -n1 git cat-file -e
>> >> > +"
>> >> > +
>> >> > +
>> >> >  test_expect_success 'stash -c stash.useBuiltin=false warning ' '
>> >> >       expected="stash.useBuiltin support has been removed" &&
>> >>
>> >> We really prefer our tests to create the same data each time if
>> >> possible, but as noted with the "date -u" comment above you're
>> >> explicitly bypassing that, but I still can't see why...
>> >
>> > I'm trying to make sure we get new object contents. Is there a better
>> > way to achieve what I want without the risk of finding that the contents
>> > are already in the database from a previous test run?
>>
>> You can just do something like:
>>
>> test_expect_success 'setup data' '
>>         test_commit A &&
>>         test_commit B
>> '
>>
>> Which will create files A.t, B.t etc, or create them via:
>>
>>     obj=$(echo foo | git hash-object -w --stdin)
>>
>> etc.
>>
>> I.e. the uniqueness you're doing here seems to assume that tests are
>> re-using the same object store across runs, but we create a new trash
>> directory for each one, if you run the test with "-d" you can see it
>> being left behind for inspection. This is already ensured for the test.
>>
>> The only potential caveat I can imagine is that some filesystem like say
>> btrfs-like that does some COW or object de-duplication would behave
>> differently, but other than that...
>
> It looks like the same repo is reused for each test_expect_success
> line in the top-level t*.sh script.
> So for test_create_unique_files to be maximally useful, it should have
> some state that is different for
> each invocation.  How about I use the test_tick mechanism to produce
> this uniqueness?  It wouldn't
> be globally unique like the date method, but it should be good enough
> if the repo is recycled every time
> test-lib is reinitialized.
>
> I'm changing lib-unique-files to use test_tick and to be a little more
> readable as you suggested. Please
> let me know if you have any other suggestions.

Ah, sorry, I thought you meant you wanted uniqueness within the test
file, but no, by default we'll create *one* repo for you, and each
test_expect_success reuses that.

Generally tests that want that do one of (in each test_expect_success):

# I'm making my own repo
git init new-repo 1 &&
(
	cd new-repo-1 &&
	[...]
)

# Or, in the first one
<setup the repo data>
# Then, in a second one
git clone . new-repo-1

I.e. just using "git clone" to ferry the data around, or cp -R if you'd
like to retain the exact file layout etc.
diff mbox series

Patch

diff --git a/t/lib-unique-files.sh b/t/lib-unique-files.sh
new file mode 100644
index 00000000000..a8a25eba61d
--- /dev/null
+++ b/t/lib-unique-files.sh
@@ -0,0 +1,34 @@ 
+# Helper to create files with unique contents
+
+test_create_unique_files_base__=$(date -u)
+test_create_unique_files_counter__=0
+
+# Create multiple files with unique contents. Takes the number of
+# directories, the number of files in each directory, and the base
+# directory.
+#
+# test_create_unique_files 2 3 . -- Creates 2 directories with 3 files
+#				    each in the specified directory, all
+#				    with unique contents.
+
+test_create_unique_files() {
+	test "$#" -ne 3 && BUG "3 param"
+
+	local dirs=$1
+	local files=$2
+	local basedir=$3
+
+	rm -rf $basedir >/dev/null
+
+	for i in $(test_seq $dirs)
+	do
+		local dir=$basedir/dir$i
+
+		mkdir -p "$dir" > /dev/null
+		for j in $(test_seq $files)
+		do
+			test_create_unique_files_counter__=$((test_create_unique_files_counter__ + 1))
+			echo "$test_create_unique_files_base__.$test_create_unique_files_counter__"  >"$dir/file$j.txt"
+		done
+	done
+}
diff --git a/t/t3700-add.sh b/t/t3700-add.sh
index 4086e1ebbc9..2122acc3e9e 100755
--- a/t/t3700-add.sh
+++ b/t/t3700-add.sh
@@ -7,6 +7,8 @@  test_description='Test of git add, including the -- option.'
 
 . ./test-lib.sh
 
+. $TEST_DIRECTORY/lib-unique-files.sh
+
 # Test the file mode "$1" of the file "$2" in the index.
 test_mode_in_index () {
 	case "$(git ls-files -s "$2")" in
@@ -33,6 +35,15 @@  test_expect_success \
     'Test that "git add -- -q" works' \
     'touch -- -q && git add -- -q'
 
+test_expect_success 'git add: core.fsyncobjectfiles=batch' "
+	test_create_unique_files 2 4 fsync-files &&
+	git -c core.fsyncobjectfiles=batch add -- ./fsync-files/ &&
+	rm -f fsynced_files &&
+	git ls-files --stage fsync-files/ > fsynced_files &&
+	test_line_count = 8 fsynced_files &&
+	cat fsynced_files | awk '{print \$2}' | xargs -n1 git cat-file -e
+"
+
 test_expect_success \
 	'git add: Test that executable bit is not used if core.filemode=0' \
 	'git config core.filemode 0 &&
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 873aa56e359..0b4e8bb55b8 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -9,6 +9,7 @@  GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
+. $TEST_DIRECTORY/lib-unique-files.sh
 
 diff_cmp () {
 	for i in "$1" "$2"
@@ -1293,6 +1294,19 @@  test_expect_success 'stash handles skip-worktree entries nicely' '
 	git rev-parse --verify refs/stash:A.t
 '
 
+test_expect_success 'stash with core.fsyncobjectfiles=batch' "
+	test_create_unique_files 2 4 fsync-files &&
+	git -c core.fsyncobjectfiles=batch stash push -u -- ./fsync-files/ &&
+	rm -f fsynced_files &&
+
+	# The files were untracked, so use the third parent,
+	# which contains the untracked files
+	git ls-tree -r stash^3 -- ./fsync-files/ > fsynced_files &&
+	test_line_count = 8 fsynced_files &&
+	cat fsynced_files | awk '{print \$3}' | xargs -n1 git cat-file -e
+"
+
+
 test_expect_success 'stash -c stash.useBuiltin=false warning ' '
 	expected="stash.useBuiltin support has been removed" &&