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

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

[Felipe Contreras]

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

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

[Felipe Contreras]

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(-)

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

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

[Felipe Contreras]

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(-)

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

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

[Junio C Hamano]

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

> 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.

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

[Eric Wong]

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.