From 9e0c047add440fb5fbe84ef5131d5574f7cdcf37 Mon Sep 17 00:00:00 2001
From: Bruce Richardson <bruce.richardson@intel.com>
Date: Fri, 9 Sep 2022 10:35:20 +0100
Subject: test/telemetry_data: refactor for maintainability
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

[ upstream commit 1306df2cd1d9ae654c2e5b3b0f40ca19ab440006 ]

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>
Acked-by: Ciara Power <ciara.power@intel.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.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 18b93db8ef..75aca8791f 100644
--- a/app/test/test_telemetry_data.c
+++ b/app/test/test_telemetry_data.c
@@ -19,18 +19,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 CHECK_OUTPUT(<expected_data>);" as it's
+ *   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.
+ */
+
 
 static struct rte_tel_data response_data;
 static int sock;
 
+
 /*
  * This function is the callback registered with Telemetry to be used when
  * the /test command is requested. This callback returns the global data built
  * up by the individual test cases.
  */
 static int
-test_cb(const char *cmd __rte_unused, const char *params __rte_unused,
+telemetry_test_cb(const char *cmd __rte_unused, const char *params __rte_unused,
 		struct rte_tel_data *d)
 {
 	*d = response_data;
@@ -44,7 +71,7 @@ test_cb(const char *cmd __rte_unused, const char *params __rte_unused,
  * and is compared to the actual response received from Telemetry.
  */
 static int
-test_output(const char *func_name, const char *expected)
+check_output(const char *func_name, const char *expected)
 {
 	int bytes;
 	char buf[BUF_SIZE * 16];
@@ -64,6 +91,14 @@ test_output(const char *func_name, const char *expected)
 	return strncmp(expected, buf, sizeof(buf));
 }
 
+static int
+test_simple_string(void)
+{
+	rte_tel_data_string(&response_data, "Simple string");
+
+	return CHECK_OUTPUT("\"Simple string\"");
+}
+
 static int
 test_dict_with_array_int_values(void)
 {
@@ -75,7 +110,6 @@ test_dict_with_array_int_values(void)
 	struct rte_tel_data *child_data2 = rte_tel_data_alloc();
 	rte_tel_data_start_array(child_data2, RTE_TEL_INT_VAL);
 
-	memset(&response_data, 0, sizeof(response_data));
 	rte_tel_data_start_dict(&response_data);
 
 	for (i = 0; i < 5; i++) {
@@ -88,8 +122,7 @@ test_dict_with_array_int_values(void)
 	rte_tel_data_add_dict_container(&response_data, "dict_1",
 	 child_data2, 0);
 
-	return TEST_OUTPUT("{\"/test\":{\"dict_0\":[0,1,2,3,4],"
-			"\"dict_1\":[0,1,2,3,4]}}");
+	return CHECK_OUTPUT("{\"dict_0\":[0,1,2,3,4],\"dict_1\":[0,1,2,3,4]}");
 }
 
 static int
@@ -103,7 +136,6 @@ test_array_with_array_int_values(void)
 	struct rte_tel_data *child_data2 = rte_tel_data_alloc();
 	rte_tel_data_start_array(child_data2, RTE_TEL_INT_VAL);
 
-	memset(&response_data, 0, sizeof(response_data));
 	rte_tel_data_start_array(&response_data, RTE_TEL_CONTAINER);
 
 	for (i = 0; i < 5; i++) {
@@ -113,18 +145,18 @@ test_array_with_array_int_values(void)
 	rte_tel_data_add_array_container(&response_data, child_data, 0);
 	rte_tel_data_add_array_container(&response_data, child_data2, 0);
 
-	return TEST_OUTPUT("{\"/test\":[[0,1,2,3,4],[0,1,2,3,4]]}");
+	return CHECK_OUTPUT("[[0,1,2,3,4],[0,1,2,3,4]]");
 }
 
 static int
 test_case_array_int(void)
 {
 	int i;
-	memset(&response_data, 0, sizeof(response_data));
+
 	rte_tel_data_start_array(&response_data, RTE_TEL_INT_VAL);
 	for (i = 0; i < 5; i++)
 		rte_tel_data_add_array_int(&response_data, i);
-	return TEST_OUTPUT("{\"/test\":[0,1,2,3,4]}");
+	return CHECK_OUTPUT("[0,1,2,3,4]");
 }
 
 static int
@@ -133,7 +165,6 @@ test_case_add_dict_int(void)
 	int i = 0;
 	char name_of_value[8];
 
-	memset(&response_data, 0, sizeof(response_data));
 	rte_tel_data_start_dict(&response_data);
 
 	for (i = 0; i < 5; i++) {
@@ -141,14 +172,12 @@ test_case_add_dict_int(void)
 		rte_tel_data_add_dict_int(&response_data, name_of_value, i);
 	}
 
-	return TEST_OUTPUT("{\"/test\":{\"dict_0\":0,\"dict_1\":1,\"dict_2\":2,"
-			"\"dict_3\":3,\"dict_4\":4}}");
+	return CHECK_OUTPUT("{\"dict_0\":0,\"dict_1\":1,\"dict_2\":2,\"dict_3\":3,\"dict_4\":4}");
 }
 
 static int
 test_case_array_string(void)
 {
-	memset(&response_data, 0, sizeof(response_data));
 	rte_tel_data_start_array(&response_data, RTE_TEL_STRING_VAL);
 	rte_tel_data_add_array_string(&response_data, "aaaa");
 	rte_tel_data_add_array_string(&response_data, "bbbb");
@@ -156,14 +185,12 @@ test_case_array_string(void)
 	rte_tel_data_add_array_string(&response_data, "dddd");
 	rte_tel_data_add_array_string(&response_data, "eeee");
 
-	return TEST_OUTPUT("{\"/test\":[\"aaaa\",\"bbbb\",\"cccc\",\"dddd\","
-			"\"eeee\"]}");
+	return CHECK_OUTPUT("[\"aaaa\",\"bbbb\",\"cccc\",\"dddd\",\"eeee\"]");
 }
 
 static int
 test_case_add_dict_string(void)
 {
-	memset(&response_data, 0, sizeof(response_data));
 	rte_tel_data_start_dict(&response_data);
 
 	rte_tel_data_add_dict_string(&response_data, "dict_0", "aaaa");
@@ -171,8 +198,7 @@ test_case_add_dict_string(void)
 	rte_tel_data_add_dict_string(&response_data, "dict_2", "cccc");
 	rte_tel_data_add_dict_string(&response_data, "dict_3", "dddd");
 
-	return TEST_OUTPUT("{\"/test\":{\"dict_0\":\"aaaa\",\"dict_1\":"
-			"\"bbbb\",\"dict_2\":\"cccc\",\"dict_3\":\"dddd\"}}");
+	return CHECK_OUTPUT("{\"dict_0\":\"aaaa\",\"dict_1\":\"bbbb\",\"dict_2\":\"cccc\",\"dict_3\":\"dddd\"}");
 }
 
 
@@ -185,7 +211,6 @@ test_dict_with_array_string_values(void)
 	struct rte_tel_data *child_data2 = rte_tel_data_alloc();
 	rte_tel_data_start_array(child_data2, RTE_TEL_STRING_VAL);
 
-	memset(&response_data, 0, sizeof(response_data));
 	rte_tel_data_start_dict(&response_data);
 
 	rte_tel_data_add_array_string(child_data, "aaaa");
@@ -196,8 +221,7 @@ test_dict_with_array_string_values(void)
 	rte_tel_data_add_dict_container(&response_data, "dict_1",
 	 child_data2, 0);
 
-	return TEST_OUTPUT("{\"/test\":{\"dict_0\":[\"aaaa\"],\"dict_1\":"
-			"[\"bbbb\"]}}");
+	return CHECK_OUTPUT("{\"dict_0\":[\"aaaa\"],\"dict_1\":[\"bbbb\"]}");
 }
 
 static int
@@ -212,7 +236,6 @@ test_dict_with_dict_values(void)
 	struct rte_tel_data *child_data2 = rte_tel_data_alloc();
 	rte_tel_data_start_array(child_data2, RTE_TEL_STRING_VAL);
 
-	memset(&response_data, 0, sizeof(response_data));
 	rte_tel_data_start_dict(&response_data);
 
 	rte_tel_data_add_array_string(child_data, "aaaa");
@@ -224,8 +247,7 @@ test_dict_with_dict_values(void)
 	rte_tel_data_add_dict_container(&response_data, "dict_of_dicts",
 			dict_of_dicts, 0);
 
-	return TEST_OUTPUT("{\"/test\":{\"dict_of_dicts\":{\"dict_0\":"
-			"[\"aaaa\"],\"dict_1\":[\"bbbb\"]}}}");
+	return CHECK_OUTPUT("{\"dict_of_dicts\":{\"dict_0\":[\"aaaa\"],\"dict_1\":[\"bbbb\"]}}");
 }
 
 static int
@@ -237,7 +259,6 @@ test_array_with_array_string_values(void)
 	struct rte_tel_data *child_data2 = rte_tel_data_alloc();
 	rte_tel_data_start_array(child_data2, RTE_TEL_STRING_VAL);
 
-	memset(&response_data, 0, sizeof(response_data));
 	rte_tel_data_start_array(&response_data, RTE_TEL_CONTAINER);
 
 	rte_tel_data_add_array_string(child_data, "aaaa");
@@ -246,18 +267,18 @@ test_array_with_array_string_values(void)
 	rte_tel_data_add_array_container(&response_data, child_data, 0);
 	rte_tel_data_add_array_container(&response_data, child_data2, 0);
 
-	return TEST_OUTPUT("{\"/test\":[[\"aaaa\"],[\"bbbb\"]]}");
+	return CHECK_OUTPUT("[[\"aaaa\"],[\"bbbb\"]]");
 }
 
 static int
 test_case_array_u64(void)
 {
 	int i;
-	memset(&response_data, 0, sizeof(response_data));
+
 	rte_tel_data_start_array(&response_data, RTE_TEL_U64_VAL);
 	for (i = 0; i < 5; i++)
 		rte_tel_data_add_array_u64(&response_data, i);
-	return TEST_OUTPUT("{\"/test\":[0,1,2,3,4]}");
+	return CHECK_OUTPUT("[0,1,2,3,4]");
 }
 
 static int
@@ -266,15 +287,13 @@ test_case_add_dict_u64(void)
 	int i = 0;
 	char name_of_value[8];
 
-	memset(&response_data, 0, sizeof(response_data));
 	rte_tel_data_start_dict(&response_data);
 
 	for (i = 0; i < 5; i++) {
 		sprintf(name_of_value, "dict_%d", i);
 		rte_tel_data_add_dict_u64(&response_data, name_of_value, i);
 	}
-	return TEST_OUTPUT("{\"/test\":{\"dict_0\":0,\"dict_1\":1,\"dict_2\":2,"
-			"\"dict_3\":3,\"dict_4\":4}}");
+	return CHECK_OUTPUT("{\"dict_0\":0,\"dict_1\":1,\"dict_2\":2,\"dict_3\":3,\"dict_4\":4}");
 }
 
 static int
@@ -288,7 +307,6 @@ test_dict_with_array_u64_values(void)
 	struct rte_tel_data *child_data2 = rte_tel_data_alloc();
 	rte_tel_data_start_array(child_data2, RTE_TEL_U64_VAL);
 
-	memset(&response_data, 0, sizeof(response_data));
 	rte_tel_data_start_dict(&response_data);
 
 	for (i = 0; i < 10; i++) {
@@ -301,8 +319,7 @@ test_dict_with_array_u64_values(void)
 	rte_tel_data_add_dict_container(&response_data, "dict_1",
 	 child_data2, 0);
 
-	return TEST_OUTPUT("{\"/test\":{\"dict_0\":[0,1,2,3,4,5,6,7,8,9],"
-			"\"dict_1\":[0,1,2,3,4,5,6,7,8,9]}}");
+	return CHECK_OUTPUT("{\"dict_0\":[0,1,2,3,4,5,6,7,8,9],\"dict_1\":[0,1,2,3,4,5,6,7,8,9]}");
 }
 
 static int
@@ -316,7 +333,6 @@ test_array_with_array_u64_values(void)
 	struct rte_tel_data *child_data2 = rte_tel_data_alloc();
 	rte_tel_data_start_array(child_data2, RTE_TEL_U64_VAL);
 
-	memset(&response_data, 0, sizeof(response_data));
 	rte_tel_data_start_array(&response_data, RTE_TEL_CONTAINER);
 
 	for (i = 0; i < 5; i++) {
@@ -326,7 +342,7 @@ test_array_with_array_u64_values(void)
 	rte_tel_data_add_array_container(&response_data, child_data, 0);
 	rte_tel_data_add_array_container(&response_data, child_data2, 0);
 
-	return TEST_OUTPUT("{\"/test\":[[0,1,2,3,4],[0,1,2,3,4]]}");
+	return CHECK_OUTPUT("[[0,1,2,3,4],[0,1,2,3,4]]");
 }
 
 static int
@@ -367,7 +383,7 @@ connect_to_socket(void)
 }
 
 static int
-test_telemetry_data(void)
+telemetry_data_autotest(void)
 {
 	typedef int (*test_case)(void);
 	unsigned int i = 0;
@@ -376,7 +392,9 @@ test_telemetry_data(void)
 	if (sock <= 0)
 		return -1;
 
-	test_case test_cases[] = {test_case_array_string,
+	test_case test_cases[] = {
+			test_simple_string,
+			test_case_array_string,
 			test_case_array_int, test_case_array_u64,
 			test_case_add_dict_int, test_case_add_dict_u64,
 			test_case_add_dict_string,
@@ -388,8 +406,9 @@ test_telemetry_data(void)
 			test_array_with_array_u64_values,
 			test_array_with_array_string_values };
 
-	rte_telemetry_register_cmd(REQUEST_CMD, test_cb, "Test");
+	rte_telemetry_register_cmd(REQUEST_CMD, telemetry_test_cb, "Test");
 	for (i = 0; i < RTE_DIM(test_cases); i++) {
+		memset(&response_data, 0, sizeof(response_data));
 		if (test_cases[i]() != 0) {
 			close(sock);
 			return -1;
@@ -399,4 +418,4 @@ test_telemetry_data(void)
 	return 0;
 }
 
-REGISTER_TEST_COMMAND(telemetry_data_autotest, test_telemetry_data);
+REGISTER_TEST_COMMAND(telemetry_data_autotest, telemetry_data_autotest);
-- 
2.23.0