Re: [PATCH 05/15] t/Makefile: optimize chainlint self-test
- From
Fabian Stelzer <fs@gigacodes.de>
- Date
- Dec 13, 2021, 10:22 UTC
- Message-ID
- <20211213102224.y5psbojmivlxe5px@fs>
- In-Reply-To
- <20211213063059.19424-6-sunshine@sunshineco.com>
On 13.12.2021 01:30, Eric Sunshine wrote:
Show 29 quoted lines
>Rather than running `chainlint` and `diff` once per self-test -- which >may become expensive as more tests are added -- instead run `chainlint` >a single time over all tests bodies collectively and compare the result >to the collective "expected" output. > >Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> >--- > t/Makefile | 10 ++++------ > 1 file changed, 4 insertions(+), 6 deletions(-) > >diff --git a/t/Makefile b/t/Makefile >index 882d26eee3..f4ae40be46 100644 >--- a/t/Makefile >+++ b/t/Makefile >@@ -71,12 +71,10 @@ clean-chainlint: > > check-chainlint: > @mkdir -p '$(CHAINLINTTMP_SQ)' && \ >- err=0 && \ >- for i in $(CHAINLINTTESTS); do \ >- $(CHAINLINT) <chainlint/$$i.test | \ >- sed -e '/^# LINT: /d' >'$(CHAINLINTTMP_SQ)'/$$i.actual && \ >- diff -u chainlint/$$i.expect '$(CHAINLINTTMP_SQ)'/$$i.actual || err=1; \ >- done && exit $$err >+ sed -e '/^# LINT: /d' $(patsubst %,chainlint/%.test,$(CHAINLINTTESTS)) >'$(CHAINLINTTMP_SQ)'/tests && \ >+ cat $(patsubst %,chainlint/%.expect,$(CHAINLINTTESTS)) >'$(CHAINLINTTMP_SQ)'/expect && \ >+ $(CHAINLINT) '$(CHAINLINTTMP_SQ)'/tests >'$(CHAINLINTTMP_SQ)'/actual && \ >+ diff -u '$(CHAINLINTTMP_SQ)'/expect '$(CHAINLINTTMP_SQ)'/actual >
If I read this right you are relying on the order of the .test & .expect files to match. I did something similar in a test prereq which resulted in a bug when setting the test_output_dir to something residing in /dev/shm, since the order of files in /dev/shm is reversed (at least on some platforms). Even though this should work as is I could see this leading to a similar bug in the future.