Re: [PATCH bpf-next 2/4] selftests/bpf: support multiple tests per file

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Oct 25, 2021 at 1:39 PM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
>
> On Mon, Oct 25, 2021 at 1:13 PM sunyucong@xxxxxxxxx <sunyucong@xxxxxxxxx> wrote:
> >
> > On Fri, Oct 22, 2021 at 3:33 PM Andrii Nakryiko <andrii@xxxxxxxxxx> wrote:
> > >
> > > Revamp how test discovery works for test_progs and allow multiple test
> > > entries per file. Any global void function with no arguments and
> > > serial_test_ or test_ prefix is considered a test.
> > >
> > > Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> > > ---
> > >  tools/testing/selftests/bpf/Makefile | 7 +++----
> > >  1 file changed, 3 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > > index 498222543c37..ac47cf9760fc 100644
> > > --- a/tools/testing/selftests/bpf/Makefile
> > > +++ b/tools/testing/selftests/bpf/Makefile
> > > @@ -421,10 +421,9 @@ ifeq ($($(TRUNNER_TESTS_DIR)-tests-hdr),)
> > >  $(TRUNNER_TESTS_DIR)-tests-hdr := y
> > >  $(TRUNNER_TESTS_HDR): $(TRUNNER_TESTS_DIR)/*.c
> > >         $$(call msg,TEST-HDR,$(TRUNNER_BINARY),$$@)
> > > -       $$(shell ( cd $(TRUNNER_TESTS_DIR);                             \
> > > -                 echo '/* Generated header, do not edit */';           \
> > > -                 ls *.c 2> /dev/null |                                 \
> > > -                       sed -e 's@\([^\.]*\)\.c@DEFINE_TEST(\1)@';      \
> > > +       $$(shell (echo '/* Generated header, do not edit */';                                   \
> > > +                 sed -n -E 's/^void (serial_)?test_([a-zA-Z0-9_]+)\((void)?\).*/DEFINE_TEST(\2)/p'     \
> >
> > probably not that important :  allow \s* before void and after void.
> > Or,  maybe we can just  (?!static)  instead of anchoring to line
> > start.
>
> Selftests source code is pretty strict with formatting, so I don't
> think we'll deviate from the strict `^void <name>` pattern (and we
> certainly don't want to deviate). So I didn't want to overcomplicate
> regexes unnecessarily.
>
> >
> > > +                       $(TRUNNER_TESTS_DIR)/*.c | sort ;       \
> >
> > to be super safe : maybe add a check here to ensure each file contains
> > at least one test function.
>
> It's actually a useful property to have .c files that don't have
> tests. This can be used for adding various shared helpers. Currently
> all *_helpers.c are in selftests/bpf/ directory and have to be
> explicitly wired in Makefile, which is a bit annoying. With this setup
> we can just put a new .c file in the selftests/bpf/prog_tests/ and it
> will be automatically compiled and linked.
>
> It also will significantly hurt readability to add some sort of
> per-file check in there, do you think it's worth it?

You are right, probably not really worth it. we just have to watch the
total test numbers, it should always goes up :-D

>
> >
> > >                  ) > $$@)
> > >  endif
> > >
> > > --
> > > 2.30.2
> > >



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux