Re: [PATCH v5 24/30] t/perf/p7519: speed up test on Windows

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2/16/22 8:15 PM, Junio C Hamano wrote:
"Jeff Hostetler via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:

+touch_files () {
+	n=$1
+	d="$n"_files
+
+	(cd $d ; test_seq 1 $n | xargs touch )
+}
+
  test_expect_success "one time repo setup" '
  	# set untrackedCache depending on the environment
  	if test -n "$GIT_PERF_7519_UNTRACKED_CACHE"
@@ -119,10 +126,11 @@ test_expect_success "one time repo setup" '
  	fi &&
mkdir 1_file 10_files 100_files 1000_files 10000_files &&
-	for i in $(test_seq 1 10); do touch 10_files/$i || return 1; done &&
-	for i in $(test_seq 1 100); do touch 100_files/$i || return 1; done &&
-	for i in $(test_seq 1 1000); do touch 1000_files/$i || return 1; done &&
-	for i in $(test_seq 1 10000); do touch 10000_files/$i || return 1; done &&
+	touch_files 1 &&

This causes touch_files to chdir to 1_files and run "touch 1" in
there, but because there is no such directory (we have 1_file/
directory, but not 1_files/ directory), it would fail.

+	touch_files 10 &&
+	touch_files 100 &&
+	touch_files 1000 &&
+	touch_files 10000 &&

Apparently nobody has run this perf script recently since part #2
was posted.

  	git add 1_file 10_files 100_files 1000_files 10000_files &&

The original introduced at bb7cc7e7 (t/perf/fsmonitor: separate one
time repo initialization, 2020-10-26) created an empty directory 1_file
and without creating anything in it, ran "git add" on it.

If we are not doing anything to 1_file directory anyway, perhaps we
can get rid of it to avoid the breakage in "make perf"?

If we have a chance to reroll this series, we can squash in
something like this, perhaps (it does not deserve to be a separate
step).

Good catch!

It looks to me like there was an oversight/typo in the original
89afd5f5ad (t/perf: add fsmonitor perf test for git diff, 2020-10-20).
They created the "1_file" directory and didn't put anything in it.
Then later when they test it, they say "0_files" in the test name
and "1_file" in the "git diff" command.

I don't think it's worth keeping an empty directory here, since there
won't be anything in the index after the add and since the directory
is empty the untracked cache won't have anything to scan.

My version (without the typo) would have created 1 file in the directory
but I don't think that's worth keeping either, since we create thousands
of files in steps right after it.

I'll make a note to remove it.

Jeff




--- >8 ---
Subject: [PATCH] p7519: leave 1_file directory empty

The step "t/perf/p7519: speed up test on Windows" in the topic
builtin-fsmonitor-part-2 (not in 'next' yet) attempts to create one
file in 1_files directory, but the original introduced at bb7cc7e7
(t/perf/fsmonitor: separate one time repo initialization,
2020-10-26):

  (1) created 1_file directory,

  (2) left the directory empty, and

  (3) a later test expected (and still expects) that there is nothing
      in the directory.

Revert the behaviour back to what the original wanted to do.

Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---
  t/perf/p7519-fsmonitor.sh | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/perf/p7519-fsmonitor.sh b/t/perf/p7519-fsmonitor.sh
index 9a2288a622..a1c552129c 100755
--- a/t/perf/p7519-fsmonitor.sh
+++ b/t/perf/p7519-fsmonitor.sh
@@ -126,7 +126,7 @@ test_expect_success "one time repo setup" '
  	fi &&
mkdir 1_file 10_files 100_files 1000_files 10000_files &&
-	touch_files 1 &&
+	: 1_file directory should be left empty &&
  	touch_files 10 &&
  	touch_files 100 &&
  	touch_files 1000 &&




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux