test: add sanity check for existing state machines

We want to ensure that our usb state machines
 - do not have any cycles
 - do not have any completely empty states
 - have names for every print statement if called

These new unit tests allow us to have build-times checks for the above.

BRANCH=none
BUG=none
TEST=tests pass. Made each test fail locally to ensure that tests were
actually working.

Change-Id: Idd2c4d69e83cf38c97278edd1727d86b52a85db9
Signed-off-by: Jett Rink <jettrink@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1744657
Commit-Queue: Denis Brockus <dbrockus@chromium.org>
Reviewed-by: Denis Brockus <dbrockus@chromium.org>

10 files changed, 398 insertions, 23 deletions

diff --git a/common/usbc/usb_pe_ctvpd_sm.c b/common/usbc/usb_pe_ctvpd_sm.c
index ab62a5bc6f..90e22fb78c 100644
--- a/common/usbc/usb_pe_ctvpd_sm.c
+++ b/common/usbc/usb_pe_ctvpd_sm.c
@@ -202,3 +202,13 @@ static const struct usb_state pe_states[] = {
 		.run    = pe_request_run,
 	},
 };
+
+#ifdef TEST_BUILD
+const struct test_sm_data test_pe_sm_data[] = {
+	{
+		.base = pe_states,
+		.size = ARRAY_SIZE(pe_states),
+	},
+};
+const int test_pe_sm_data_size = ARRAY_SIZE(test_pe_sm_data);
+#endif
diff --git a/common/usbc/usb_prl_sm.c b/common/usbc/usb_prl_sm.c
index 5c0caeee21..52a1e607a5 100644
--- a/common/usbc/usb_prl_sm.c
+++ b/common/usbc/usb_prl_sm.c
@@ -1628,3 +1628,26 @@ static const struct usb_state tch_states[] = {
 		.run    = tch_message_received_run,
 	},
 };
+
+#ifdef TEST_BUILD
+const struct test_sm_data test_prl_sm_data[] = {
+	{
+		.base = prl_tx_states,
+		.size = ARRAY_SIZE(prl_tx_states),
+	},
+	{
+		.base = prl_hr_states,
+		.size = ARRAY_SIZE(prl_hr_states),
+	},
+	{
+		.base = rch_states,
+		.size = ARRAY_SIZE(rch_states),
+	},
+	{
+		.base = tch_states,
+		.size = ARRAY_SIZE(tch_states),
+	},
+};
+const int test_prl_sm_data_size = ARRAY_SIZE(test_prl_sm_data);
+#endif
+
diff --git a/common/usbc/usb_tc_ctvpd_sm.c b/common/usbc/usb_tc_ctvpd_sm.c
index f837b27dbc..46be033514 100644
--- a/common/usbc/usb_tc_ctvpd_sm.c
+++ b/common/usbc/usb_tc_ctvpd_sm.c
@@ -231,7 +231,7 @@ static enum usb_tc_state get_last_state_tc(const int port)
 	return tc[port].ctx.previous - &tc_states[0];
 }
 
-static void print_current_state(const int port)
+test_mockable_static void print_current_state(const int port)
 {
 	CPRINTS("C%d: %s", port, tc_state_names[get_state_tc(port)]);
 }
@@ -1684,3 +1684,15 @@ static const struct usb_state tc_states[] = {
 		.parent = &tc_states[TC_HOST_RP3_CT_RD],
 	},
 };
+
+#ifdef TEST_BUILD
+const struct test_sm_data test_tc_sm_data[] = {
+	{
+		.base = tc_states,
+		.size = ARRAY_SIZE(tc_states),
+		.names = tc_state_names,
+		.names_size = ARRAY_SIZE(tc_state_names),
+	},
+};
+const int test_tc_sm_data_size = ARRAY_SIZE(test_tc_sm_data);
+#endif
diff --git a/common/usbc/usb_tc_vpd_sm.c b/common/usbc/usb_tc_vpd_sm.c
index 1abbf7d4f2..1b72c207e8 100644
--- a/common/usbc/usb_tc_vpd_sm.c
+++ b/common/usbc/usb_tc_vpd_sm.c
@@ -159,7 +159,7 @@ test_export_static enum usb_tc_state get_state_tc(const int port)
 	return tc[port].ctx.current - &tc_states[0];
 }
 
-static void print_current_state(const int port)
+test_mockable_static void print_current_state(const int port)
 {
 	CPRINTS("C%d: %s", port, tc_state_names[get_state_tc(port)]);
 }
@@ -398,3 +398,15 @@ static const struct usb_state tc_states[] = {
 		.exit   = tc_attached_snk_exit,
 	},
 };
+
+#ifdef TEST_BUILD
+const struct test_sm_data test_tc_sm_data[] = {
+	{
+		.base = tc_states,
+		.size = ARRAY_SIZE(tc_states),
+		.names = tc_state_names,
+		.names_size = ARRAY_SIZE(tc_state_names),
+	},
+};
+const int test_tc_sm_data_size = ARRAY_SIZE(test_tc_sm_data);
+#endif
diff --git a/include/usb_sm.h b/include/usb_sm.h
index f9f10b439f..c0248830ec 100644
--- a/include/usb_sm.h
+++ b/include/usb_sm.h
@@ -65,4 +65,21 @@ void set_state(int port, struct sm_ctx *ctx, usb_state_ptr new_state);
  */
 void exe_state(int port, struct sm_ctx *ctx);
 
+#ifdef TEST_BUILD
+/*
+ * Struct for test builds that allow unit tests to easily iterate through
+ * state machines
+ */
+struct test_sm_data {
+	/* Base pointer of the state machine array */
+	const usb_state_ptr base;
+	/* Size fo the state machine array above */
+	const int size;
+	/* The array of names for states, can be NULL */
+	const char * const * const names;
+	/* The size of the above names array */
+	const int names_size;
+};
+#endif
+
 #endif /* __CROS_EC_USB_SM_H */
diff --git a/test/build.mk b/test/build.mk
index ba57220e29..b90f976430 100644
--- a/test/build.mk
+++ b/test/build.mk
@@ -143,9 +143,9 @@ usb_sm_framework_h3-y=usb_sm_framework_h3.o
 usb_sm_framework_h2-y=usb_sm_framework_h3.o
 usb_sm_framework_h1-y=usb_sm_framework_h3.o
 usb_sm_framework_h0-y=usb_sm_framework_h3.o
-usb_typec_vpd-y=usb_typec_ctvpd.o vpd_api.o
-usb_typec_ctvpd-y=usb_typec_ctvpd.o vpd_api.o
-usb_prl-y=usb_prl.o
+usb_typec_vpd-y=usb_typec_ctvpd.o vpd_api.o usb_sm_checks.o
+usb_typec_ctvpd-y=usb_typec_ctvpd.o vpd_api.o usb_sm_checks.o
+usb_prl-y=usb_prl.o usb_sm_checks.o
 utils-y=utils.o
 utils_str-y=utils_str.o
 vboot-y=vboot.o
diff --git a/test/usb_prl.c b/test/usb_prl.c
index 0851e0cfc2..72f4fd6c8f 100644
--- a/test/usb_prl.c
+++ b/test/usb_prl.c
@@ -15,6 +15,7 @@
 #include "usb_pd.h"
 #include "usb_pd_test_util.h"
 #include "usb_prl_sm.h"
+#include "usb_sm_checks.h"
 #include "util.h"
 
 #define PORT0 0
@@ -1349,6 +1350,11 @@ void run_test(void)
 
 	/* TODO(shurst): More PD 3.0 Tests */
 
+	/* Do basic state machine sanity checks last. */
+	RUN_TEST(test_prl_no_parent_cycles);
+	RUN_TEST(test_prl_no_empty_state);
+	RUN_TEST(test_prl_all_states_named);
+
 	test_print_result();
 }
 
diff --git a/test/usb_sm_checks.c b/test/usb_sm_checks.c
new file mode 100644
index 0000000000..498d56e323
--- /dev/null
+++ b/test/usb_sm_checks.c
@@ -0,0 +1,254 @@
+/* Copyright 2019 The Chromium OS Authors. All rights reserved.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ *
+ * Test USB Type-C VPD and CTVPD module.
+ */
+#include "common.h"
+#include "task.h"
+#include "test_util.h"
+#include "timer.h"
+#include "usb_pd.h"
+#include "usb_sm.h"
+#include "usb_tc_sm.h"
+#include "util.h"
+#include "usb_pd_test_util.h"
+#include "vpd_api.h"
+
+#ifdef CONFIG_USB_TYPEC_SM
+extern const struct test_sm_data test_tc_sm_data[];
+extern const int test_tc_sm_data_size;
+#else
+const struct test_sm_data test_tc_sm_data[] = {};
+const int test_tc_sm_data_size;
+#endif
+
+#ifdef CONFIG_USB_PRL_SM
+extern const struct test_sm_data test_prl_sm_data[];
+extern const int test_prl_sm_data_size;
+#else
+const struct test_sm_data test_prl_sm_data[] = {};
+const int test_prl_sm_data_size;
+#endif
+
+#ifdef CONFIG_USB_PE_SM
+extern const struct test_sm_data test_pe_sm_data[];
+extern const int test_pe_sm_data_size;
+#else
+const struct test_sm_data test_pe_sm_data[] = {};
+const int test_pe_sm_data_size;
+#endif
+
+test_static int test_no_parent_cycles(const struct test_sm_data * const sm_data)
+{
+	int i;
+
+	for (i = 0; i < sm_data->size; ++i) {
+		int depth = 0;
+		usb_state_ptr current = &sm_data->base[i];
+
+		while (current != NULL && ++depth <= sm_data->size)
+			current = current->parent;
+
+		if (depth > sm_data->size)
+			break;
+	}
+
+	/* Ensure all states end, otherwise the ith state has a cycle. */
+	TEST_EQ(i, sm_data->size, "%d");
+
+	return EC_SUCCESS;
+}
+
+int test_tc_no_parent_cycles(void)
+{
+	int i;
+
+	for (i = 0; i < test_tc_sm_data_size; ++i) {
+		const int rv = test_no_parent_cycles(&test_tc_sm_data[i]);
+
+		if (rv) {
+			ccprintf("TC State machine %d has a cycle!\n", i);
+			TEST_ASSERT(0);
+		}
+	}
+
+	return EC_SUCCESS;
+}
+
+int test_prl_no_parent_cycles(void)
+{
+	int i;
+
+	for (i = 0; i < test_prl_sm_data_size; ++i) {
+		const int rv = test_no_parent_cycles(&test_prl_sm_data[i]);
+
+		if (rv) {
+			ccprintf("PRL State machine %d has a cycle!\n", i);
+			TEST_ASSERT(0);
+		}
+	}
+
+	return EC_SUCCESS;
+}
+
+int test_pe_no_parent_cycles(void)
+{
+	int i;
+
+	for (i = 0; i < test_pe_sm_data_size; ++i) {
+		const int rv = test_no_parent_cycles(&test_pe_sm_data[i]);
+
+		if (rv) {
+			ccprintf("PE State machine %d has a cycle!\n", i);
+			TEST_ASSERT(0);
+		}
+	}
+
+	return EC_SUCCESS;
+}
+
+static int test_no_empty_state(const struct test_sm_data * const sm_data)
+{
+	int i;
+	const struct usb_state empty = { 0 };
+
+	for (i = 0; i < sm_data->size; ++i) {
+		if (memcmp(&sm_data->base[i], &empty, sizeof(empty)) == 0)
+			break;
+	}
+
+	/* Ensure all states had data, otherwise the ith state is empty. */
+	TEST_EQ(i, sm_data->size, "%d");
+
+	return EC_SUCCESS;
+}
+
+int test_tc_no_empty_state(void)
+{
+	int i;
+
+	for (i = 0; i < test_tc_sm_data_size; ++i) {
+		const int rv = test_no_empty_state(&test_tc_sm_data[i]);
+
+		if (rv) {
+			ccprintf("TC State machine %d has empty state!\n", i);
+			TEST_ASSERT(0);
+		}
+	}
+
+	return EC_SUCCESS;
+}
+
+int test_prl_no_empty_state(void)
+{
+	int i;
+
+	for (i = 0; i < test_prl_sm_data_size; ++i) {
+		const int rv = test_no_empty_state(&test_prl_sm_data[i]);
+
+		if (rv) {
+			ccprintf("PRL State machine %d has empty state!\n", i);
+			TEST_ASSERT(0);
+		}
+	}
+
+	return EC_SUCCESS;
+}
+
+int test_pe_no_empty_state(void)
+{
+	int i;
+
+	for (i = 0; i < test_pe_sm_data_size; ++i) {
+		const int rv = test_no_empty_state(&test_pe_sm_data[i]);
+
+		if (rv) {
+			ccprintf("PE State machine %d has empty state!\n", i);
+			TEST_ASSERT(0);
+		}
+	}
+
+	return EC_SUCCESS;
+}
+
+static volatile int state_printed;
+
+/* Override the implement version of print */
+__override void print_current_state(const int port)
+{
+	state_printed = 1;
+}
+
+static int test_all_states_named(const struct test_sm_data * const sm_data)
+{
+	int i;
+
+	for (i = 0; i < sm_data->size; ++i) {
+		usb_state_ptr current = &sm_data->base[i];
+
+		state_printed = 0;
+
+		if (current->entry)
+			current->entry(0);
+
+		if (state_printed) {
+			if (i >= sm_data->names_size ||
+			    sm_data->names[i] == NULL) {
+				ccprintf("State %d does not have a name!\n", i);
+				TEST_ASSERT(0);
+			}
+		}
+	}
+
+	return EC_SUCCESS;
+}
+
+int test_tc_all_states_named(void)
+{
+	int i;
+
+	for (i = 0; i < test_tc_sm_data_size; ++i) {
+		const int rv = test_all_states_named(&test_tc_sm_data[i]);
+
+		if (rv) {
+			ccprintf("TC State machine %d has empty name!\n", i);
+			TEST_ASSERT(0);
+		}
+	}
+
+	return EC_SUCCESS;
+}
+
+int test_prl_all_states_named(void)
+{
+	int i;
+
+	for (i = 0; i < test_prl_sm_data_size; ++i) {
+		const int rv = test_all_states_named(&test_prl_sm_data[i]);
+
+		if (rv) {
+			ccprintf("PRL State machine %d has empty name!\n", i);
+			TEST_ASSERT(0);
+		}
+	}
+
+	return EC_SUCCESS;
+}
+
+int test_pe_all_states_named(void)
+{
+	int i;
+
+	for (i = 0; i < test_pe_sm_data_size; ++i) {
+		const int rv = test_all_states_named(&test_pe_sm_data[i]);
+
+		if (rv) {
+			ccprintf("PE State machine %d has empty name!\n", i);
+			TEST_ASSERT(0);
+		}
+	}
+
+	return EC_SUCCESS;
+}
+
diff --git a/test/usb_sm_checks.h b/test/usb_sm_checks.h
new file mode 100644
index 0000000000..0a556518a4
--- /dev/null
+++ b/test/usb_sm_checks.h
@@ -0,0 +1,25 @@
+/* Copyright 2019 The Chromium OS Authors. All rights reserved.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+/* Sanity tests for a state machine definition */
+
+#ifndef __CROS_EC_USB_SM_CHECKS_H
+#define __CROS_EC_USB_SM_CHECKS_H
+
+int test_tc_no_parent_cycles(void);
+int test_tc_no_empty_state(void);
+int test_tc_all_states_named(void);
+
+
+int test_prl_no_parent_cycles(void);
+int test_prl_no_empty_state(void);
+int test_prl_all_states_named(void);
+
+
+int test_pe_no_parent_cycles(void);
+int test_pe_no_empty_state(void);
+int test_pe_all_states_named(void);
+
+#endif /* __CROS_EC_USB_SM_CHECKS_H */
\ No newline at end of file
diff --git a/test/usb_typec_ctvpd.c b/test/usb_typec_ctvpd.c
index 2d45b4f882..8c8b5e3897 100644
--- a/test/usb_typec_ctvpd.c
+++ b/test/usb_typec_ctvpd.c
@@ -15,6 +15,7 @@
 #include "util.h"
 #include "usb_pd_tcpm.h"
 #include "usb_pd_test_util.h"
+#include "usb_sm_checks.h"
 #include "vpd_api.h"
 
 #define PORT0	0
@@ -658,9 +659,9 @@ static int test_ctvpd_behavior_case1(void)
 
 	/*
 	 * CASE 1: The following tests the behavior when a DRP is connected to a
-	 *         Charge-Through VCONN-Powered USB Device (abbreviated CTVPD),
-	 *         with no Power Source attached to the ChargeThrough port on
-	 *         the CTVPD.
+	 *	 Charge-Through VCONN-Powered USB Device (abbreviated CTVPD),
+	 *	 with no Power Source attached to the ChargeThrough port on
+	 *	 the CTVPD.
 	 */
 
 	/* 1. DRP and CTVPD are both in the unattached state */
@@ -797,9 +798,9 @@ static int test_ctvpd_behavior_case2(void)
 
 	/*
 	 * CASE 2: The following tests the behavior when a Power Source is
-	 *         connected to a Charge-Through VCONN-Powered USB Device
-	 *         (abbreviated CTVPD), with a Host already attached to the
-	 *         Host-Side port on the CTVPD.
+	 *	 connected to a Charge-Through VCONN-Powered USB Device
+	 *	 (abbreviated CTVPD), with a Host already attached to the
+	 *	 Host-Side port on the CTVPD.
 	 */
 
 	/*
@@ -919,9 +920,9 @@ static int test_ctvpd_behavior_case3(void)
 
 	/*
 	 * CASE 3: The following describes the behavior when a Power Source is
-	 *         connected to a ChargeThrough VCONN-Powered USB Device
-	 *         (abbreviated CTVPD), with no Host attached to the Host-side
-	 *         port on the CTVPD.
+	 *	 connected to a ChargeThrough VCONN-Powered USB Device
+	 *	 (abbreviated CTVPD), with no Host attached to the Host-side
+	 *	 port on the CTVPD.
 	 */
 
 	/*
@@ -1022,9 +1023,9 @@ static int test_ctvpd_behavior_case4(void)
 
 	/*
 	 * CASE 4: The following describes the behavior when a DRP is connected
-	 *         to a Charge-Through VCONN-Powered USB Device
-	 *         (abbreviated CTVPD), with a Power Source already attached to
-	 *         the Charge-Through side on the CTVPD.
+	 *	 to a Charge-Through VCONN-Powered USB Device
+	 *	 (abbreviated CTVPD), with a Power Source already attached to
+	 *	 the Charge-Through side on the CTVPD.
 	 */
 
 	/*
@@ -1177,9 +1178,9 @@ static int test_ctvpd_behavior_case5(void)
 
 	/*
 	 * CASE 5: The following describes the behavior when a Power Source is
-	 *         connected to a ChargeThrough VCONN-Powered USB Device
-	 *         (abbreviated CTVPD), with a DRP (with dead battery) attached
-	 *         to the Host-side port on the CTVPD.
+	 *	 connected to a ChargeThrough VCONN-Powered USB Device
+	 *	 (abbreviated CTVPD), with a DRP (with dead battery) attached
+	 *	 to the Host-side port on the CTVPD.
 	 */
 
 	/*
@@ -1317,9 +1318,9 @@ static int test_ctvpd_behavior_case6(void)
 
 	/*
 	 * CASE 6: The following describes the behavior when a DRP is connected
-	 *         to a Charge-Through VCONN-Powered USB Device
-	 *         (abbreviated CTVPD) and a Sink is attached to the
-	 *         Charge-Through port on the CTVPD.
+	 *	 to a Charge-Through VCONN-Powered USB Device
+	 *	 (abbreviated CTVPD) and a Sink is attached to the
+	 *	 Charge-Through port on the CTVPD.
 	 */
 
 	/*
@@ -1513,6 +1514,21 @@ void run_test(void)
 	RUN_TEST(test_ctvpd_behavior_case5);
 	RUN_TEST(test_ctvpd_behavior_case6);
 #endif
+
+	/* Do basic state machine sanity checks last. */
+	RUN_TEST(test_tc_no_parent_cycles);
+	RUN_TEST(test_tc_no_empty_state);
+	RUN_TEST(test_tc_all_states_named);
+
+	/*
+	 * Since you have to include TypeC layer when adding PE layer, the
+	 * PE test would have the same build dependencies, so go ahead and test
+	 * te PE statemachine here so we don't have to create another test exe
+	 */
+	RUN_TEST(test_pe_no_parent_cycles);
+	RUN_TEST(test_pe_no_empty_state);
+	RUN_TEST(test_pe_all_states_named);
+
 	test_print_result();
 }