Re: [dpdk-dev] [PATCH v6 27/33] eal/trace: add unit test cases

[David Marchand <david.marchand@redhat.com>]

On Sun, Apr 19, 2020 at 12:03 PM <jerinj@marvell.com> wrote:
>
> From: Sunil Kumar Kori <skori@marvell.com>
>
> Example commands to run UT and check the traces with babeltrace viewer.
>
> - Delete the existing /root/dpdk-traces/ directory if needed.
> > sudo rm -rf /root/dpdk-traces/
>
> - Start the dpdk-test
> > sudo ./build/app/test/dpdk-test  -c 0x3 - --trace=.*

Unit tests are there to do sanity/non regression checks.
Here you describe a manual test procedure.
Could it work in the CI for the functional parts?


>
> - Run trace_autotest
> > trace_autotest
>
> - View the traces with babletrace viewer.
> > sudo babeltrace /root/dpdk-traces/
>
> Signed-off-by: Sunil Kumar Kori <skori@marvell.com>

[snip]

> diff --git a/app/test/test_trace.c b/app/test/test_trace.c
> new file mode 100644
> index 000000000..26b403e62
> --- /dev/null
> +++ b/app/test/test_trace.c
> @@ -0,0 +1,223 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(C) 2020 Marvell International Ltd.
> + */
> +
> +#include <rte_lcore.h>
> +#include <rte_trace.h>
> +#include <rte_trace_eal.h>
> +
> +#include "test.h"
> +#include "test_trace.h"
> +
> +static int32_t
> +test_trace_point_globbing(void)
> +{
> +       bool val;
> +       int rc;
> +
> +       rc = rte_trace_pattern("app.dpdk.test*", false);
> +       if (rc != 1)
> +               goto failed;
> +
> +       val = rte_trace_point_is_enabled(&__app_dpdk_test_tp);
> +       if (val == true)
> +               goto failed;

No need to test a boolean against true/false values.
Idem for the rest of this code.


> +
> +       rc = rte_trace_pattern("app.dpdk.test*", true);
> +       if (rc != 1)
> +               goto failed;
> +
> +       val = rte_trace_point_is_enabled(&__app_dpdk_test_tp);
> +       if (val == false)
> +               goto failed;
> +

[snip]

> +static int
> +test_trace_mode(void)
> +{
> +       enum rte_trace_mode current;
> +
> +       current = rte_trace_mode_get();
> +
> +       if (rte_trace_is_enabled() == false)
> +               return TEST_SKIPPED;

This test would always be skipped if we called it from the CI, as it
requires the dpdk-test binary to be started with traces on.


> +
> +       rte_trace_mode_set(RTE_TRACE_MODE_DISCARD);
> +       if (rte_trace_mode_get() != RTE_TRACE_MODE_DISCARD)
> +               goto failed;
> +

[snip]

> diff --git a/app/test/test_trace_register.c b/app/test/test_trace_register.c
> new file mode 100644
> index 000000000..1735149a2
> --- /dev/null
> +++ b/app/test/test_trace_register.c
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(C) 2020 Marvell International Ltd.
> + */
> +#define RTE_TRACE_POINT_REGISTER_SELECT /* Select trace point register macros */

I noticed this comment is copy/pasted in all trace points "register" code.
This does not help.

The documentation on the other side does not describe this macro which
is quite important for developers adding tracepoints to their code
(Thomas already commented about it on the doc patch).


> +
> +#include "test_trace.h"
> +
> +/* Define trace points */
> +RTE_TRACE_POINT_DEFINE(app_dpdk_test_tp);
> +RTE_TRACE_POINT_DEFINE(app_dpdk_test_fp);
> +
> +RTE_INIT(register_valid_trace_points)
> +{
> +       RTE_TRACE_POINT_REGISTER(app_dpdk_test_tp, app.dpdk.test.tp);
> +       RTE_TRACE_POINT_REGISTER(app_dpdk_test_fp, app.dpdk.test.fp);
> +}
> +
> --
> 2.25.1
>



-- 
David Marchand