RE: [PATCH v2 10/13] test/telemetry_data: refactor for maintainability

["Power, Ciara" <ciara.power@intel.com>]

> -----Original Message-----
> From: Richardson, Bruce <bruce.richardson@intel.com>
> Sent: Monday 25 July 2022 17:36
> To: dev@dpdk.org
> Cc: Richardson, Bruce <bruce.richardson@intel.com>; Power, Ciara
> <ciara.power@intel.com>
> Subject: [PATCH v2 10/13] test/telemetry_data: refactor for maintainability
> 
> To help with the writing and maintaining of test cases in this file we can make
> the following changes to it:
> 
> - rename non-test-case functions i.e. the infrastructure functions, to
>   not start with "test_", so that each sub-test case can be identified
>   by starting with that prefix.
> - add a comment at the start of the file explaining how tests are to be
>   written and managed, so as to keep consistency.
> - add a trivial test-case for returning a simple string value to use as
>   a reference example for those wanting to add test cases.
> - improve the key macro used for validating the output from each
>   function, so that the standard json preamble can be skipped for each
>   function. This hides more of the infrastructure implementation from
>   the user i.e. they don't need to worry what the actual command used is
>   called, and also shortens the output strings so we can avoid line
>   splitting in most cases.
> - add clearing the "response_data" structure to the loop calling each
>   test to avoid each test function having to do so individually.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  app/test/test_telemetry_data.c | 101 ++++++++++++++++++++-------------
>  1 file changed, 60 insertions(+), 41 deletions(-)
> 
> diff --git a/app/test/test_telemetry_data.c
> b/app/test/test_telemetry_data.c index 73eee293a1..5a85e790d3 100644
> --- a/app/test/test_telemetry_data.c
> +++ b/app/test/test_telemetry_data.c
> @@ -21,18 +21,45 @@
>  #define TELEMETRY_VERSION "v2"
>  #define REQUEST_CMD "/test"
>  #define BUF_SIZE 1024
> -#define TEST_OUTPUT(exp) test_output(__func__, exp)
> +#define CHECK_OUTPUT(exp) check_output(__func__, "{\""
> REQUEST_CMD
> +"\":" exp "}")
> +
> +/*
> + * Runs a series of test cases, checking the output of telemetry for
> +various different types of
> + * responses. On init, a single connection to DPDK telemetry is made,
> +and a single telemetry
> + * callback "/test" is registered. That callback always returns the
> +value of the static global
> + * variable "response_data", so each test case builds up that
> +structure, and then calls the
> + * "check_output" function to ensure the response received over the
> +socket for "/test" matches
> + * that expected for the response_data value populated.
> + *
> + * NOTE:
> + * - each test case function in this file should be added to the "test_cases"
> array in
> + *   test_telemetry_data function at the bottom of the file.
> + * - each test case function should populate the "response_data" global
> variable (below)
> + *   with the appropriate values which would be returned from a simulated
> telemetry function.
> + *   Then the test case function should have "return
> TEST_OUTPUT(<expected_data>);" as it's


[CP] nit: I think this should be CHECK_OUTPUT based on the macro rename above.


> + *   last line. The test infrastructure will then validate that the output when
> returning
> + *   "response_data" structure matches that in "<expected_data>".
> + * - the response_data structure will be zeroed on entry to each test
> function, so each function
> + *   can begin with a call to "rte_tel_data_string/start_array/start_dict" as so
> desired.
> + * - the expected_output for each function can be just the actual json data
> from the
> + *   "response_data" value. The CHECK_OUTPUT macro will include the
> appropriate "{\"/test\": ... }"
> + *   structure around the json output.
> + *
> + *  See test_simple_string(), or test_case_array_int() for a basic examples
> of test cases.
> + */

<snip>