Subject: Re: [PATCH v4 5/7] perf lint: add make test-lint to perf tests

From: Taylor Blau <[email-masked]>
Date: Tue, 20 Oct 2020 18:06:29 -0400

On Tue, Oct 20, 2020 at 01:41:02PM +0000, Nipunn Koorapati via GitGitGadget wrote:
> diff --git a/t/perf/Makefile b/t/perf/Makefile
> index 8c47155a7c..fcb0e8865e 100644
> --- a/t/perf/Makefile
> +++ b/t/perf/Makefile
> @@ -1,7 +1,7 @@
>  -include ../../config.mak
>  export GIT_TEST_OPTIONS
>
> -all: perf
> +all: test-lint perf
>
>  perf: pre-clean
>  	./run
> @@ -12,4 +12,7 @@ pre-clean:
>  clean:
>  	rm -rf build "trash directory".* test-results
>
> +test-lint:
> +	$(MAKE) -C .. test-lint
> +

Great; it sounds like adding a complete definition here was too much
effort to be worth it, but that adding a '$(MAKE) -C ..' is just right.
We can still run 'make test-lint' from within 't/perf', but there isn't
a bunch of clutter in this series to make that happen. Thanks.

>  .PHONY: all perf pre-clean clean
> diff --git a/t/perf/p3400-rebase.sh b/t/perf/p3400-rebase.sh
> index d202aaed06..7a0bb29448 100755
> --- a/t/perf/p3400-rebase.sh
> +++ b/t/perf/p3400-rebase.sh
> @@ -9,16 +9,16 @@ test_expect_success 'setup rebasing on top of a lot of changes' '
>  	git checkout -f -B base &&
>  	git checkout -B to-rebase &&
>  	git checkout -B upstream &&
> -	for i in $(seq 100)
> +	for i in $(test_seq 100)
>  	do
>  		# simulate huge diffs
>  		echo change$i >unrelated-file$i &&
> -		seq 1000 >>unrelated-file$i &&
> +		test_seq 1000 >>unrelated-file$i &&
>  		git add unrelated-file$i &&
>  		test_tick &&
>  		git commit -m commit$i unrelated-file$i &&
>  		echo change$i >unrelated-file$i &&
> -		seq 1000 | tac >>unrelated-file$i &&
> +		test_seq 1000 | tac >>unrelated-file$i &&

The rest of this all looks good, but I think adding 'tac' here is still
wrong; this isn't available everywhere, so we would want to find an
alternative before going further. Is there a reason that you couldn't
use a different 'N' in 'test_seq N' here?

Thanks,
Taylor