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:56 PM sunyucong@xxxxxxxxx <sunyucong@xxxxxxxxx> wrote:
>
> 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

Yep. We should be able to automate this once we have some sort of
baseline comparison functionality in BPF CI.

>
> >
> > >
> > > >                  ) > $$@)
> > > >  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