| Message ID | 20210828052006.1313788-7-davemarchevsky@fb.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | bpf: implement variadic printk helper | expand |
On Fri, Aug 27, 2021 at 10:20 PM Dave Marchevsky <davemarchevsky@fb.com> wrote: > > Guidance for new tests is to use ASSERT macros instead of CHECK. Since > trace_vprintk test will borrow heavily from trace_printk's, migrate its > CHECKs so it remains obvious that the two are closely related. > > Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com> > --- Great, thanks! Acked-by: Andrii Nakryiko <andrii@kernel.org> > .../selftests/bpf/prog_tests/trace_printk.c | 24 +++++++------------ > 1 file changed, 9 insertions(+), 15 deletions(-) > > diff --git a/tools/testing/selftests/bpf/prog_tests/trace_printk.c b/tools/testing/selftests/bpf/prog_tests/trace_printk.c > index d39bc00feb45..e47835f0a674 100644 > --- a/tools/testing/selftests/bpf/prog_tests/trace_printk.c > +++ b/tools/testing/selftests/bpf/prog_tests/trace_printk.c > @@ -10,7 +10,7 @@ > > void test_trace_printk(void) > { > - int err, iter = 0, duration = 0, found = 0; > + int err = 0, iter = 0, found = 0; > struct trace_printk__bss *bss; > struct trace_printk *skel; > char *buf = NULL; > @@ -18,25 +18,24 @@ void test_trace_printk(void) > size_t buflen; > > skel = trace_printk__open(); > - if (CHECK(!skel, "skel_open", "failed to open skeleton\n")) > + if (!ASSERT_OK_PTR(skel, "trace_printk__open")) > return; > > - ASSERT_EQ(skel->rodata->fmt[0], 'T', "invalid printk fmt string"); > + ASSERT_EQ(skel->rodata->fmt[0], 'T', "skel->rodata->fmt[0]"); > skel->rodata->fmt[0] = 't'; > > err = trace_printk__load(skel); > - if (CHECK(err, "skel_load", "failed to load skeleton: %d\n", err)) > + if (!ASSERT_OK(err, "trace_printk__load")) > goto cleanup; > > bss = skel->bss; > > err = trace_printk__attach(skel); > - if (CHECK(err, "skel_attach", "skeleton attach failed: %d\n", err)) > + if (!ASSERT_OK(err, "trace_printk__attach")) > goto cleanup; > > fp = fopen(TRACEBUF, "r"); > - if (CHECK(fp == NULL, "could not open trace buffer", > - "error %d opening %s", errno, TRACEBUF)) > + if (!ASSERT_OK_PTR(fp, "fopen(TRACEBUF)")) > goto cleanup; > > /* We do not want to wait forever if this test fails... */ > @@ -46,14 +45,10 @@ void test_trace_printk(void) > usleep(1); > trace_printk__detach(skel); > > - if (CHECK(bss->trace_printk_ran == 0, > - "bpf_trace_printk never ran", > - "ran == %d", bss->trace_printk_ran)) > + if (!ASSERT_GT(bss->trace_printk_ran, 0, "bss->trace_printk_ran")) > goto cleanup; > > - if (CHECK(bss->trace_printk_ret <= 0, > - "bpf_trace_printk returned <= 0 value", > - "got %d", bss->trace_printk_ret)) > + if (!ASSERT_GT(bss->trace_printk_ret, 0, "bss->trace_printk_ret")) > goto cleanup; > > /* verify our search string is in the trace buffer */ > @@ -66,8 +61,7 @@ void test_trace_printk(void) > break; > } > > - if (CHECK(!found, "message from bpf_trace_printk not found", > - "no instance of %s in %s", SEARCHMSG, TRACEBUF)) > + if (!ASSERT_EQ(found, bss->trace_printk_ran, "found")) > goto cleanup; > > cleanup: > -- > 2.30.2 >
diff --git a/tools/testing/selftests/bpf/prog_tests/trace_printk.c b/tools/testing/selftests/bpf/prog_tests/trace_printk.c index d39bc00feb45..e47835f0a674 100644 --- a/tools/testing/selftests/bpf/prog_tests/trace_printk.c +++ b/tools/testing/selftests/bpf/prog_tests/trace_printk.c @@ -10,7 +10,7 @@ void test_trace_printk(void) { - int err, iter = 0, duration = 0, found = 0; + int err = 0, iter = 0, found = 0; struct trace_printk__bss *bss; struct trace_printk *skel; char *buf = NULL; @@ -18,25 +18,24 @@ void test_trace_printk(void) size_t buflen; skel = trace_printk__open(); - if (CHECK(!skel, "skel_open", "failed to open skeleton\n")) + if (!ASSERT_OK_PTR(skel, "trace_printk__open")) return; - ASSERT_EQ(skel->rodata->fmt[0], 'T', "invalid printk fmt string"); + ASSERT_EQ(skel->rodata->fmt[0], 'T', "skel->rodata->fmt[0]"); skel->rodata->fmt[0] = 't'; err = trace_printk__load(skel); - if (CHECK(err, "skel_load", "failed to load skeleton: %d\n", err)) + if (!ASSERT_OK(err, "trace_printk__load")) goto cleanup; bss = skel->bss; err = trace_printk__attach(skel); - if (CHECK(err, "skel_attach", "skeleton attach failed: %d\n", err)) + if (!ASSERT_OK(err, "trace_printk__attach")) goto cleanup; fp = fopen(TRACEBUF, "r"); - if (CHECK(fp == NULL, "could not open trace buffer", - "error %d opening %s", errno, TRACEBUF)) + if (!ASSERT_OK_PTR(fp, "fopen(TRACEBUF)")) goto cleanup; /* We do not want to wait forever if this test fails... */ @@ -46,14 +45,10 @@ void test_trace_printk(void) usleep(1); trace_printk__detach(skel); - if (CHECK(bss->trace_printk_ran == 0, - "bpf_trace_printk never ran", - "ran == %d", bss->trace_printk_ran)) + if (!ASSERT_GT(bss->trace_printk_ran, 0, "bss->trace_printk_ran")) goto cleanup; - if (CHECK(bss->trace_printk_ret <= 0, - "bpf_trace_printk returned <= 0 value", - "got %d", bss->trace_printk_ret)) + if (!ASSERT_GT(bss->trace_printk_ret, 0, "bss->trace_printk_ret")) goto cleanup; /* verify our search string is in the trace buffer */ @@ -66,8 +61,7 @@ void test_trace_printk(void) break; } - if (CHECK(!found, "message from bpf_trace_printk not found", - "no instance of %s in %s", SEARCHMSG, TRACEBUF)) + if (!ASSERT_EQ(found, bss->trace_printk_ran, "found")) goto cleanup; cleanup:
Guidance for new tests is to use ASSERT macros instead of CHECK. Since trace_vprintk test will borrow heavily from trace_printk's, migrate its CHECKs so it remains obvious that the two are closely related. Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com> --- .../selftests/bpf/prog_tests/trace_printk.c | 24 +++++++------------ 1 file changed, 9 insertions(+), 15 deletions(-)