Re: [PATCH v4 5/7] perf lint: add make test-lint to perf tests
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Oct 20, 2020, 22:06 UTC
- Message-ID
- <20201020220629.GF75186@nand.local>
- In-Reply-To
- <b534cd137a833de802d6d95c1affb8d2d8f7de85.1603201265.git.gitgitgadget@gmail.com>
On Tue, Oct 20, 2020 at 01:41:02PM +0000, Nipunn Koorapati via GitGitGadget wrote:
Show 20 quoted lines
> 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.
Show 22 quoted lines
> .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