From 975ee6bd44ca18c82ecf5aea91c361baf248a0ff Mon Sep 17 00:00:00 2001 From: Neels Hofmeyr Date: Mon, 9 Apr 2018 00:42:12 +0200 Subject: fsm_test: more thoroughly test FSM inst ids and names Place id and name testing in its separate section, test_id_api(). Add a test that actually allocates an FSM instance with a NULL id, which is allowed, but uncovers a bug of an unset FSM instance name. osmo_fsm_inst_name() falls back to the fsm struct's name on NULL, but osmo_fsm_inst_find_by_name() fails to match if the instance's name is NULL (and until recently even crashed). Show this in fsm_test.c with loud comments. Add test to clear the id by passing NULL. Add test for setting an empty id. Add test for setting an invalid identifier (osmo_identifier_valid() == false). Change-Id: I646ed918576ce196c395dc5f42a1507c52ace2c5 --- tests/fsm/fsm_test.c | 101 +++++++++++++++++++++++++++++++++++++++++++++---- tests/fsm/fsm_test.err | 44 +++++++++++++++++++++ 2 files changed, 138 insertions(+), 7 deletions(-) (limited to 'tests') diff --git a/tests/fsm/fsm_test.c b/tests/fsm/fsm_test.c index ef7bfe39..0f29a2e4 100644 --- a/tests/fsm/fsm_test.c +++ b/tests/fsm/fsm_test.c @@ -3,6 +3,7 @@ #include #include #include +#include #include #include @@ -16,6 +17,12 @@ enum { static void *g_ctx; +static int safe_strcmp(const char *a, const char *b) +{ + if (!a || !b) + return a == b ? 0 : 1; + return strcmp(a, b); +} enum test_fsm_states { ST_NULL = 0, @@ -118,7 +125,7 @@ static void assert_cmd_reply(const char *cmdstr, const char *expres) struct ctrl_cmd *cmd; cmd = exec_ctrl_cmd(cmdstr); - if (strcmp(cmd->reply, expres)) { + if (safe_strcmp(cmd->reply, expres)) { fprintf(stderr, "Reply '%s' doesn't match expected '%s'\n", cmd->reply, expres); OSMO_ASSERT(0); } @@ -165,6 +172,89 @@ static struct osmo_fsm_inst *foo(void) return fi; } +static void test_id_api() +{ + struct osmo_fsm_inst *fi; + + fprintf(stderr, "\n--- %s()\n", __func__); + +/* Assert the instance has this name and can be looked up by it */ +#define assert_name(expected_name) \ +do { \ + const char *name = osmo_fsm_inst_name(fi); \ + fprintf(stderr, " osmo_fsm_inst_name() == %s\n", osmo_quote_str(name, -1)); \ + if (safe_strcmp(name, expected_name)) { \ + fprintf(stderr, " ERROR: expected %s\n", osmo_quote_str(expected_name, -1)); \ + OSMO_ASSERT(false); \ + } \ + OSMO_ASSERT(osmo_fsm_inst_find_by_name(&fsm, expected_name) == fi); \ + fprintf(stderr, " osmo_fsm_inst_find_by_name(%s) == fi\n", osmo_quote_str(expected_name, -1)); \ +} while(0) + +/* Assert the instance can be looked up by this id string */ +#define assert_id(expected_id) \ +do { \ + OSMO_ASSERT(osmo_fsm_inst_find_by_id(&fsm, expected_id) == fi); \ + fprintf(stderr, " osmo_fsm_inst_find_by_id(%s) == fi\n", osmo_quote_str(expected_id, -1)); \ +} while(0) + +/* Update the id, assert the proper rc, and expect a resulting fsm inst name + lookup */ +#define test_id(new_id, expect_rc, expect_name_suffix) do { \ + int rc; \ + fprintf(stderr, "osmo_fsm_inst_update_id(%s)\n", osmo_quote_str(new_id, -1)); \ + rc = osmo_fsm_inst_update_id(fi, new_id); \ + fprintf(stderr, " rc == %d", rc); \ + if (rc == (expect_rc)) \ + fprintf(stderr, ", ok\n"); \ + else { \ + fprintf(stderr, ", ERROR: expected rc == %d\n", expect_rc); \ + OSMO_ASSERT(rc == expect_rc); \ + } \ + assert_name("Test_FSM" expect_name_suffix); \ + }while (0) + +/* Successfully set a new id, along with name and id lookup assertions */ +#define change_id(new_id) \ + test_id(new_id, 0, "(" new_id ")"); \ + assert_id(new_id) + + /* allocate FSM instance without id, there should be a name without id */ + fi = osmo_fsm_inst_alloc(&fsm, g_ctx, NULL, LOGL_DEBUG, NULL); + OSMO_ASSERT(fi); + /* CURRENT BUG: here I want to just do + assert_name("Test_FSM"); + * but when allocated with a NULL id, the fsm's name remains unset. Hence: */ + { + const char *expected_name = "Test_FSM"; + const char *name = osmo_fsm_inst_name(fi); + fprintf(stderr, " osmo_fsm_inst_name() == %s\n", osmo_quote_str(name, -1)); + if (safe_strcmp(name, expected_name)) { + fprintf(stderr, " ERROR: expected %s\n", osmo_quote_str(expected_name, -1)); + OSMO_ASSERT(false); + } + OSMO_ASSERT(osmo_fsm_inst_find_by_name(&fsm, "Test_FSM") == NULL); /* <- ERROR */ + fprintf(stderr, " osmo_fsm_inst_find_by_name(%s) == NULL\n", osmo_quote_str(expected_name, -1)); + } + + change_id("my_id"); + change_id("another_id"); + + test_id(NULL, 0, ""); + /* clear already cleared id */ + test_id(NULL, 0, ""); + + change_id("arbitrary_id"); + + /* clear id by empty string doesn't work */ + test_id("", -EINVAL, "(arbitrary_id)"); + + test_id("invalid.id", -EINVAL, "(arbitrary_id)"); + + fprintf(stderr, "\n--- %s() done\n\n", __func__); + + osmo_fsm_inst_term(fi, OSMO_FSM_TERM_REQUEST, NULL); +} + static const struct log_info_cat default_categories[] = { [DMAIN] = { .name = "DMAIN", @@ -198,17 +288,14 @@ int main(int argc, char **argv) OSMO_ASSERT(osmo_fsm_inst_find_by_name(&fsm, "my_id") == NULL); finst = foo(); - OSMO_ASSERT(osmo_fsm_inst_find_by_id(&fsm, "my_id") == finst); - OSMO_ASSERT(osmo_fsm_inst_find_by_name(&fsm, "Test_FSM(my_id)") == finst); - OSMO_ASSERT(osmo_fsm_inst_update_id(finst, "another_id") == 0); - OSMO_ASSERT(osmo_fsm_inst_find_by_id(&fsm, "another_id") == finst); - OSMO_ASSERT(osmo_fsm_inst_find_by_name(&fsm, "Test_FSM(another_id)") == finst); - OSMO_ASSERT(osmo_fsm_inst_update_id(finst, "my_id") == 0); while (main_loop_run) { osmo_select_main(0); } osmo_fsm_inst_free(finst); + + test_id_api(); + osmo_fsm_unregister(&fsm); exit(0); } diff --git a/tests/fsm/fsm_test.err b/tests/fsm/fsm_test.err index facc9f78..bc159bda 100644 --- a/tests/fsm/fsm_test.err +++ b/tests/fsm/fsm_test.err @@ -9,4 +9,48 @@ Checking FSM allocation Test_FSM(my_id){TWO}: Timeout of T2342 Timer Test_FSM(my_id){TWO}: Deallocated + +--- test_id_api() +Test_FSM{NULL}: Allocated + osmo_fsm_inst_name() == "Test_FSM" + osmo_fsm_inst_find_by_name("Test_FSM") == NULL +osmo_fsm_inst_update_id("my_id") + rc == 0, ok + osmo_fsm_inst_name() == "Test_FSM(my_id)" + osmo_fsm_inst_find_by_name("Test_FSM(my_id)") == fi + osmo_fsm_inst_find_by_id("my_id") == fi +osmo_fsm_inst_update_id("another_id") + rc == 0, ok + osmo_fsm_inst_name() == "Test_FSM(another_id)" + osmo_fsm_inst_find_by_name("Test_FSM(another_id)") == fi + osmo_fsm_inst_find_by_id("another_id") == fi +osmo_fsm_inst_update_id(NULL) + rc == 0, ok + osmo_fsm_inst_name() == "Test_FSM" + osmo_fsm_inst_find_by_name("Test_FSM") == fi +osmo_fsm_inst_update_id(NULL) + rc == 0, ok + osmo_fsm_inst_name() == "Test_FSM" + osmo_fsm_inst_find_by_name("Test_FSM") == fi +osmo_fsm_inst_update_id("arbitrary_id") + rc == 0, ok + osmo_fsm_inst_name() == "Test_FSM(arbitrary_id)" + osmo_fsm_inst_find_by_name("Test_FSM(arbitrary_id)") == fi + osmo_fsm_inst_find_by_id("arbitrary_id") == fi +osmo_fsm_inst_update_id("") +Attempting to allocate FSM instance of type 'Test_FSM' with illegal identifier '' + rc == -22, ok + osmo_fsm_inst_name() == "Test_FSM(arbitrary_id)" + osmo_fsm_inst_find_by_name("Test_FSM(arbitrary_id)") == fi +osmo_fsm_inst_update_id("invalid.id") +Attempting to allocate FSM instance of type 'Test_FSM' with illegal identifier 'invalid.id' + rc == -22, ok + osmo_fsm_inst_name() == "Test_FSM(arbitrary_id)" + osmo_fsm_inst_find_by_name("Test_FSM(arbitrary_id)") == fi + +--- test_id_api() done + +Test_FSM(arbitrary_id){NULL}: Terminating (cause = OSMO_FSM_TERM_REQUEST) +Test_FSM(arbitrary_id){NULL}: Freeing instance +Test_FSM(arbitrary_id){NULL}: Deallocated  \ No newline at end of file -- cgit v1.2.3