Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Zhangjin Wu <falcon@tinylab.org>
To:     w@1wt.eu
Cc:     falcon@tinylab.org, arnd@arndb.de, linux-kernel@vger.kernel.org,
        linux-kselftest@vger.kernel.org, linux-riscv@lists.infradead.org,
        thomas@t-8ch.de
Subject: Re: [PATCH 1/4] selftests/nolibc: add a test-report target
Date:   Wed,  7 Jun 2023 13:52:00 +0800
Message-Id: <20230607055200.667447-1-falcon@tinylab.org>
In-Reply-To: <20230605065459.153001-1-falcon@tinylab.org>
References: <20230605065459.153001-1-falcon@tinylab.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Feedback-ID: bizesmtp:tinylab.org:qybglogicsvrsz:qybglogicsvrsz3a-3
Precedence: bulk

Hi, Willy

> > On Mon, Jun 05, 2023 at 11:48:52AM +0800, Zhangjin Wu wrote:
> > > A standalone test-report target is added to let the run, run-user and
> > > rerun targets share them.
> > > 
> > > Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> > > ---
> > >  tools/testing/selftests/nolibc/Makefile | 26 ++++++++++++-------------
> > >  1 file changed, 13 insertions(+), 13 deletions(-)
> > > 
> > > diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> > > index be4159837494..8149ace2938a 100644
> > > --- a/tools/testing/selftests/nolibc/Makefile
> > > +++ b/tools/testing/selftests/nolibc/Makefile
> > > @@ -127,14 +127,18 @@ nolibc-test: nolibc-test.c sysroot/$(ARCH)/include
> > >  libc-test: nolibc-test.c
> > >  	$(QUIET_CC)$(CC) -o $@ $<
> > >  
> > > -# qemu user-land test
> > > -run-user: nolibc-test
> > > -	$(Q)qemu-$(QEMU_ARCH) ./nolibc-test > "$(CURDIR)/run.out" || :
> > > +test-report:
> > >  	$(Q)awk '/\[OK\]$$/{p++} /\[FAIL\]$$/{f++} /\[SKIPPED\]$$/{s++} \
> > >  	         END{ printf("%d test(s) passed, %d skipped, %d failed.", p, s, f); \
> > >  	         if (s+f > 0) printf(" See all results in %s\n", ARGV[1]); else print; }' \
> > >  	         $(CURDIR)/run.out
> > >  
> > > +# qemu user-land test
> > > +_run-user: nolibc-test
> > > +	$(Q)qemu-$(QEMU_ARCH) ./nolibc-test > "$(CURDIR)/run.out" || :
> > > +
> > > +run-user: _run-user test-report
> > > +
> > 
> > This will not reliably work, there's no ordering here, nothing guarantees
> > that test-report will run *after* _run-user (e.g. make -j). Another
> > approach is needed if you want to factor this, but in general creating
> > sequences in makefiles is difficult and often more painful than having
> > 3 times the same 3 lines.
> >
> 
> Ok, thanks, what about this?
> 
>     # LOG_REPORT: report the test results
>     LOG_REPORT   := awk '/\[OK\][\r]*$$/{p++} /\[FAIL\][\r]*$$/{f++} /\[SKIPPED\][\r]*$$/{s++} \
> 	                 END{ printf("%d test(s) passed, %d skipped, %d failed.", p, s, f); \
> 	                 printf(" See all results in %s\n", ARGV[1]); }'
> 
>     run-user: nolibc-test
> 	$(Q)qemu-$(QEMU_ARCH) ./nolibc-test > "$(CURDIR)/run.out" || :
> 	$(Q)$(LOG_REPORT) $(CURDIR)/run.out
> 
>     run: kernel
> 	$(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(srctree)/$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
> 	$(Q)$(LOG_REPORT) $(CURDIR)/run.out
> 
>     rerun:
> 	$(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(srctree)/$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
> 	$(Q)$(LOG_REPORT) $(CURDIR)/run.out
> 
> Or we directly add a standalone test report script? something like
> tools/testing/selftests/nolibc/report.sh
> 
>     #!/bin/sh
>     #
>     # report.sh -- report the test results of nolibc-test
>     #
>     
>     LOG_FILE=$1
>     [ ! -f "$LOG_FILE" ] && echo "Usage: $0 /path/to/run.out"
>     
>     awk '
>         /\[OK\][\r]*$$/{ p++ }
>         /\[FAIL\][\r]*$$/{ f++ }
>         /\[SKIPPED\][\r]*$$/{ s++ }
>     
>         END {
>             printf("%d test(s) passed, %d skipped, %d failed.", p, s, f);
>             printf(" See all results in %s\n", ARGV[1]);
>         }' $LOG_FILE
> 
> And use it like this:
> 
>     LOG_REPORT           = $(CURDIR)/report.sh
>

I plan to renew this patchset, which one of the above methods do you
prefer?

For the always print statement:

    printf(" See all results in %s\n", ARGV[1]); }'

I will paste the reason why I need it, as mentioned in [1], if you still
need a clean test report, I will give up this change ;-)

Thanks,
Zhangjin
---
[1]: https://lore.kernel.org/linux-riscv/20230605070508.153407-1-falcon@tinylab.org/ 

> Best regards,
> Zhangjin
>
> > Willy