threads / patch / 59370

v2, 2 partstest: cleanups of aggregate-results.sh

Subject: [PATCH v2 0/2] test: cleanups of aggregate-results.sh

## tl;dr

5 messages between Mar 9, 2023 and Mar 10, 2023. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Felipe Contreras· Mar 9, 2023, 17:30 UTC · lore

Basically $title.

Since v1 I'm now passing the directory, instead of the file pattern as Ævar suggested.

Also, while at it, let's silence the print of the command.

Felipe Contreras (2):
  test: simplify counts aggregation
  test: don't print aggregate-results command

 contrib/subtree/t/Makefile | 4 +---
 t/Makefile                 | 4 +---
 t/aggregate-results.sh     | 2 +-
 3 files changed, 3 insertions(+), 7 deletions(-)

-- 
2.39.2

Felipe Contreras· Mar 9, 2023, 17:30 UTC · re: Felipe Contreras · lore

[PATCH v2 1/2] test: simplify counts aggregation

When the list of files as input was implemented in 6508eedf67 (t/aggregate-results: accomodate systems with small max argument list length, 2010-06-01), a much simpler solution wasn't considered.

Let's just pass the directory as an argument.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/subtree/t/Makefile | 4 +---
 t/Makefile                 | 4 +---
 t/aggregate-results.sh     | 2 +-
 3 files changed, 3 insertions(+), 7 deletions(-)

Show changes to 3 files +3 −7

contrib/subtree/t/Makefile, t/Makefile, t/aggregate-results.sh

diff --git a/contrib/subtree/t/Makefile b/contrib/subtree/t/Makefile
index 4655e0987b..aa204d43b6 100644
--- a/contrib/subtree/t/Makefile
+++ b/contrib/subtree/t/Makefile
@@ -74,9 +74,7 @@ aggregate-results-and-cleanup: $(T)
 	$(MAKE) clean
 
 aggregate-results:
-	for f in '$(TEST_RESULTS_DIRECTORY_SQ)'/t*-*.counts; do \
-		echo "$$f"; \
-	done | '$(SHELL_PATH_SQ)' ../../../t/aggregate-results.sh
+	'$(SHELL_PATH_SQ)' ../../../t/aggregate-results.sh '$(TEST_RESULTS_DIRECTORY_SQ)'
 
 valgrind:
 	$(MAKE) GIT_TEST_OPTS="$(GIT_TEST_OPTS) --valgrind"
diff --git a/t/Makefile b/t/Makefile
index 2c2b252240..6ce79849e1 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -140,9 +140,7 @@ aggregate-results-and-cleanup: $(T)
 	$(MAKE) clean
 
 aggregate-results:
-	for f in '$(TEST_RESULTS_DIRECTORY_SQ)'/t*-*.counts; do \
-		echo "$$f"; \
-	done | '$(SHELL_PATH_SQ)' ./aggregate-results.sh
+	'$(SHELL_PATH_SQ)' ./aggregate-results.sh '$(TEST_RESULTS_DIRECTORY_SQ)'
 
 valgrind:
 	$(MAKE) GIT_TEST_OPTS="$(GIT_TEST_OPTS) --valgrind"
diff --git a/t/aggregate-results.sh b/t/aggregate-results.sh
index 7f2b83bdc8..6e3bcc4aec 100755
--- a/t/aggregate-results.sh
+++ b/t/aggregate-results.sh
@@ -8,7 +8,7 @@ broken=0
 total=0
 missing_prereq=
 
-while read file
+for file in "$1"/t*-*.counts
 do
 	while read type value
 	do

-- 
2.39.2

Felipe Contreras· Mar 9, 2023, 17:30 UTC · re: Felipe Contreras · lore

[PATCH v2 2/2] test: don't print aggregate-results command

There's no value in it.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/subtree/t/Makefile | 2 +-
 t/Makefile                 | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Show changes to 2 files +2 −2

contrib/subtree/t/Makefile, t/Makefile

diff --git a/contrib/subtree/t/Makefile b/contrib/subtree/t/Makefile
index aa204d43b6..093399c788 100644
--- a/contrib/subtree/t/Makefile
+++ b/contrib/subtree/t/Makefile
@@ -74,7 +74,7 @@ aggregate-results-and-cleanup: $(T)
 	$(MAKE) clean
 
 aggregate-results:
-	'$(SHELL_PATH_SQ)' ../../../t/aggregate-results.sh '$(TEST_RESULTS_DIRECTORY_SQ)'
+	@'$(SHELL_PATH_SQ)' ../../../t/aggregate-results.sh '$(TEST_RESULTS_DIRECTORY_SQ)'
 
 valgrind:
 	$(MAKE) GIT_TEST_OPTS="$(GIT_TEST_OPTS) --valgrind"
diff --git a/t/Makefile b/t/Makefile
index 6ce79849e1..88fa504957 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -140,7 +140,7 @@ aggregate-results-and-cleanup: $(T)
 	$(MAKE) clean
 
 aggregate-results:
-	'$(SHELL_PATH_SQ)' ./aggregate-results.sh '$(TEST_RESULTS_DIRECTORY_SQ)'
+	@'$(SHELL_PATH_SQ)' ./aggregate-results.sh '$(TEST_RESULTS_DIRECTORY_SQ)'
 
 valgrind:
 	$(MAKE) GIT_TEST_OPTS="$(GIT_TEST_OPTS) --valgrind"

-- 
2.39.2

Junio C Hamano· Mar 10, 2023, 19:25 UTC · re: Felipe Contreras · lore

Re: [PATCH v2 0/2] test: cleanups of aggregate-results.sh

Felipe Contreras <felipe.contreras@gmail.com> writes:

Show 15 quoted lines

> Basically $title.
>
> Since v1 I'm now passing the directory, instead of the file pattern as
> Ævar suggested.
>
> Also, while at it, let's silence the print of the command.
>
> Felipe Contreras (2):
>   test: simplify counts aggregation
>   test: don't print aggregate-results command
>
>  contrib/subtree/t/Makefile | 4 +---
>  t/Makefile                 | 4 +---
>  t/aggregate-results.sh     | 2 +-
>  3 files changed, 3 insertions(+), 7 deletions(-)

These two look sensible. Any comments on them by reviewers on the initial round?

Will queue.  Thanks, all.

Eric Wong· Mar 10, 2023, 21:51 UTC · re: Junio C Hamano · lore

Re: [PATCH v2 0/2] test: cleanups of aggregate-results.sh

Junio C Hamano <gitster@pobox.com> wrote:

> These two look sensible.  Any comments on them by reviewers on the
> initial round?

Looks fine to me.