From c42a3bf8dd584f49bc27c4182922075102ddeaa1 Mon Sep 17 00:00:00 2001 From: Ethan Gao Date: Wed, 22 Nov 2017 20:58:43 +0800 Subject: [PATCH 1/4] Optimize namespace node and topic validation make the valid result more accurate, which should be different from the default NULL use rmw_namespace_validation_result_string instead for namespace validation Signed-off-by: Ethan Gao --- rmw/src/validate_full_topic_name.c | 2 +- rmw/src/validate_namespace.c | 2 +- rmw/src/validate_node_name.c | 2 +- rmw/test/test_validate_full_topic_name.cpp | 3 ++- rmw/test/test_validate_namespace.cpp | 17 +++++++++-------- rmw/test/test_validate_node_name.cpp | 3 ++- 6 files changed, 16 insertions(+), 13 deletions(-) diff --git a/rmw/src/validate_full_topic_name.c b/rmw/src/validate_full_topic_name.c index 9a24a236..045cf4bf 100644 --- a/rmw/src/validate_full_topic_name.c +++ b/rmw/src/validate_full_topic_name.c @@ -118,7 +118,7 @@ rmw_full_topic_name_validation_result_string(int validation_result) { switch (validation_result) { case RMW_TOPIC_VALID: - return NULL; + return "valid topic name"; case RMW_TOPIC_INVALID_IS_EMPTY_STRING: return "topic name must not be empty"; case RMW_TOPIC_INVALID_NOT_ABSOLUTE: diff --git a/rmw/src/validate_namespace.c b/rmw/src/validate_namespace.c index c708cb05..dcfc4add 100644 --- a/rmw/src/validate_namespace.c +++ b/rmw/src/validate_namespace.c @@ -110,7 +110,7 @@ rmw_namespace_validation_result_string(int validation_result) { switch (validation_result) { case RMW_NAMESPACE_VALID: - return NULL; + return "valid namespace"; case RMW_NAMESPACE_INVALID_IS_EMPTY_STRING: return "namespace must not be empty"; case RMW_NAMESPACE_INVALID_NOT_ABSOLUTE: diff --git a/rmw/src/validate_node_name.c b/rmw/src/validate_node_name.c index df223fc7..4fc69cac 100644 --- a/rmw/src/validate_node_name.c +++ b/rmw/src/validate_node_name.c @@ -82,7 +82,7 @@ rmw_node_name_validation_result_string(int validation_result) { switch (validation_result) { case RMW_NODE_NAME_VALID: - return NULL; + return "valid node name"; case RMW_NODE_NAME_INVALID_IS_EMPTY_STRING: return "node name must not be empty"; case RMW_NODE_NAME_INVALID_CONTAINS_UNALLOWED_CHARACTERS: diff --git a/rmw/test/test_validate_full_topic_name.cpp b/rmw/test/test_validate_full_topic_name.cpp index c3def82d..f69e34e5 100644 --- a/rmw/test/test_validate_full_topic_name.cpp +++ b/rmw/test/test_validate_full_topic_name.cpp @@ -47,7 +47,8 @@ TEST(test_validate_topic_name, valid_topic) { ASSERT_EQ(RMW_RET_OK, ret); ASSERT_EQ(RMW_TOPIC_VALID, validation_result); - ASSERT_EQ((char *)NULL, rmw_full_topic_name_validation_result_string(validation_result)); + ret = strcmp("valid topic name", rmw_full_topic_name_validation_result_string(validation_result)); + ASSERT_EQ(0, ret); } TEST(test_validate_topic_name, empty_topic_name) { diff --git a/rmw/test/test_validate_namespace.cpp b/rmw/test/test_validate_namespace.cpp index d17300fe..55569c17 100644 --- a/rmw/test/test_validate_namespace.cpp +++ b/rmw/test/test_validate_namespace.cpp @@ -52,7 +52,8 @@ TEST(test_validate_namespace, valid_namespace) { ASSERT_EQ(RMW_RET_OK, ret); ASSERT_EQ(RMW_NAMESPACE_VALID, validation_result); - ASSERT_EQ((char *)NULL, rmw_full_topic_name_validation_result_string(validation_result)); + ret = strcmp("valid namespace", rmw_namespace_validation_result_string(validation_result)); + ASSERT_EQ(0, ret); } TEST(test_validate_namespace, empty_namespace) { @@ -71,7 +72,7 @@ TEST(test_validate_namespace, empty_namespace) { ASSERT_EQ(RMW_NAMESPACE_INVALID_IS_EMPTY_STRING, validation_result); ASSERT_EQ(0ul, invalid_index); - ASSERT_NE((char *)NULL, rmw_full_topic_name_validation_result_string(validation_result)); + ASSERT_NE((char *)NULL, rmw_namespace_validation_result_string(validation_result)); } TEST(test_validate_namespace, not_absolute) { @@ -95,7 +96,7 @@ TEST(test_validate_namespace, not_absolute) { ASSERT_EQ(RMW_NAMESPACE_INVALID_NOT_ABSOLUTE, validation_result); ASSERT_EQ(0ul, invalid_index); - ASSERT_NE((char *)NULL, rmw_full_topic_name_validation_result_string(validation_result)); + ASSERT_NE((char *)NULL, rmw_namespace_validation_result_string(validation_result)); } TEST(test_validate_namespace, ends_with_forward_slash) { @@ -114,7 +115,7 @@ TEST(test_validate_namespace, ends_with_forward_slash) { ASSERT_EQ(RMW_NAMESPACE_INVALID_ENDS_WITH_FORWARD_SLASH, validation_result); ASSERT_EQ(10ul, invalid_index); - ASSERT_NE((char *)NULL, rmw_full_topic_name_validation_result_string(validation_result)); + ASSERT_NE((char *)NULL, rmw_namespace_validation_result_string(validation_result)); } TEST(test_validate_namespace, unallowed_characters) { @@ -148,7 +149,7 @@ TEST(test_validate_namespace, unallowed_characters) { ASSERT_EQ(RMW_NAMESPACE_INVALID_CONTAINS_UNALLOWED_CHARACTERS, validation_result); ASSERT_EQ(5ul, invalid_index); - ASSERT_NE((char *)NULL, rmw_full_topic_name_validation_result_string(validation_result)); + ASSERT_NE((char *)NULL, rmw_namespace_validation_result_string(validation_result)); } TEST(test_validate_namespace, repeated_forward_slashes) { @@ -167,7 +168,7 @@ TEST(test_validate_namespace, repeated_forward_slashes) { ASSERT_EQ(RMW_NAMESPACE_INVALID_CONTAINS_REPEATED_FORWARD_SLASH, validation_result); ASSERT_EQ(10ul, invalid_index); - ASSERT_NE((char *)NULL, rmw_full_topic_name_validation_result_string(validation_result)); + ASSERT_NE((char *)NULL, rmw_namespace_validation_result_string(validation_result)); } TEST(test_validate_namespace, starts_with_number) { @@ -191,7 +192,7 @@ TEST(test_validate_namespace, starts_with_number) { ASSERT_EQ(RMW_NAMESPACE_INVALID_NAME_TOKEN_STARTS_WITH_NUMBER, validation_result); ASSERT_EQ(8ul, invalid_index); - ASSERT_NE((char *)NULL, rmw_full_topic_name_validation_result_string(validation_result)); + ASSERT_NE((char *)NULL, rmw_namespace_validation_result_string(validation_result)); } TEST(test_validate_namespace, topic_too_long) { @@ -228,5 +229,5 @@ TEST(test_validate_namespace, topic_too_long) { EXPECT_EQ(RMW_NAMESPACE_INVALID_TOO_LONG, validation_result); EXPECT_EQ(RMW_NAMESPACE_MAX_LENGTH - 1, invalid_index); - ASSERT_NE((char *)NULL, rmw_full_topic_name_validation_result_string(validation_result)); + ASSERT_NE((char *)NULL, rmw_namespace_validation_result_string(validation_result)); } diff --git a/rmw/test/test_validate_node_name.cpp b/rmw/test/test_validate_node_name.cpp index 2d6f5837..ce26b64f 100644 --- a/rmw/test/test_validate_node_name.cpp +++ b/rmw/test/test_validate_node_name.cpp @@ -47,7 +47,8 @@ TEST(test_validate_node_name, valid_node_name) { ASSERT_EQ(RMW_RET_OK, ret); ASSERT_EQ(RMW_NODE_NAME_VALID, validation_result); - ASSERT_EQ((char *)NULL, rmw_node_name_validation_result_string(validation_result)); + ret = strcmp("valid node name", rmw_node_name_validation_result_string(validation_result)); + ASSERT_EQ(0, ret); } TEST(test_validate_node_name, empty_node_name) { From 976a72d44bd86eb149cad51094dc51a4b1f90b36 Mon Sep 17 00:00:00 2001 From: Ethan Gao Date: Tue, 28 Nov 2017 22:45:05 +0800 Subject: [PATCH 2/4] no NULL returned from validation string and twaek the tests accordingly Signed-off-by: Ethan Gao --- rmw/src/validate_full_topic_name.c | 2 +- rmw/src/validate_namespace.c | 2 +- rmw/src/validate_node_name.c | 2 +- rmw/test/test_validate_full_topic_name.cpp | 31 +++++++++++++++++----- rmw/test/test_validate_namespace.cpp | 31 +++++++++++++++++----- rmw/test/test_validate_node_name.cpp | 19 ++++++++++--- 6 files changed, 66 insertions(+), 21 deletions(-) diff --git a/rmw/src/validate_full_topic_name.c b/rmw/src/validate_full_topic_name.c index 045cf4bf..2a3395bc 100644 --- a/rmw/src/validate_full_topic_name.c +++ b/rmw/src/validate_full_topic_name.c @@ -134,6 +134,6 @@ rmw_full_topic_name_validation_result_string(int validation_result) case RMW_TOPIC_INVALID_TOO_LONG: return "topic length should not exceed '" RMW_STRINGIFY(RMW_TOPIC_MAX_NAME_LENGTH) "'"; default: - return NULL; + return "undefined topic name type"; } } diff --git a/rmw/src/validate_namespace.c b/rmw/src/validate_namespace.c index dcfc4add..ffc41743 100644 --- a/rmw/src/validate_namespace.c +++ b/rmw/src/validate_namespace.c @@ -126,6 +126,6 @@ rmw_namespace_validation_result_string(int validation_result) case RMW_NAMESPACE_INVALID_TOO_LONG: return "namespace should not exceed '" RMW_STRINGIFY(RMW_NAMESPACE_MAX_NAME_LENGTH) "'"; default: - return NULL; + return "undefined namespace type"; } } diff --git a/rmw/src/validate_node_name.c b/rmw/src/validate_node_name.c index 4fc69cac..3f79dc48 100644 --- a/rmw/src/validate_node_name.c +++ b/rmw/src/validate_node_name.c @@ -93,6 +93,6 @@ rmw_node_name_validation_result_string(int validation_result) return "node name length should not exceed '" RMW_STRINGIFY(RMW_NODE_NAME_MAX_NAME_LENGTH) "'"; default: - return NULL; + return "undefined node name type"; } } diff --git a/rmw/test/test_validate_full_topic_name.cpp b/rmw/test/test_validate_full_topic_name.cpp index f69e34e5..225f33d6 100644 --- a/rmw/test/test_validate_full_topic_name.cpp +++ b/rmw/test/test_validate_full_topic_name.cpp @@ -67,7 +67,10 @@ TEST(test_validate_topic_name, empty_topic_name) { ASSERT_EQ(RMW_TOPIC_INVALID_IS_EMPTY_STRING, validation_result); ASSERT_EQ(0ul, invalid_index); - ASSERT_NE((char *)NULL, rmw_full_topic_name_validation_result_string(validation_result)); + ret = + strcmp("topic name must not be empty", rmw_full_topic_name_validation_result_string( + validation_result)); + ASSERT_EQ(0, ret); } TEST(test_validate_topic_name, not_absolute) { @@ -91,7 +94,9 @@ TEST(test_validate_topic_name, not_absolute) { ASSERT_EQ(RMW_TOPIC_INVALID_NOT_ABSOLUTE, validation_result); ASSERT_EQ(0ul, invalid_index); - ASSERT_NE((char *)NULL, rmw_full_topic_name_validation_result_string(validation_result)); + ret = strcmp("topic name must be absolute, it must lead with a '/'", + rmw_full_topic_name_validation_result_string(validation_result)); + ASSERT_EQ(0, ret); } TEST(test_validate_topic_name, ends_with_forward_slash) { @@ -115,7 +120,10 @@ TEST(test_validate_topic_name, ends_with_forward_slash) { ASSERT_EQ(RMW_TOPIC_INVALID_ENDS_WITH_FORWARD_SLASH, validation_result); ASSERT_EQ(0ul, invalid_index); - ASSERT_NE((char *)NULL, rmw_full_topic_name_validation_result_string(validation_result)); + ret = + strcmp("topic name must not end with a '/'", rmw_full_topic_name_validation_result_string( + validation_result)); + ASSERT_EQ(0, ret); } TEST(test_validate_topic_name, unallowed_characters) { @@ -149,7 +157,9 @@ TEST(test_validate_topic_name, unallowed_characters) { ASSERT_EQ(RMW_TOPIC_INVALID_CONTAINS_UNALLOWED_CHARACTERS, validation_result); ASSERT_EQ(5ul, invalid_index); - ASSERT_NE((char *)NULL, rmw_full_topic_name_validation_result_string(validation_result)); + ret = strcmp("topic name must not contain characters other than alphanumerics, '_', or '/'", + rmw_full_topic_name_validation_result_string(validation_result)); + ASSERT_EQ(0, ret); } TEST(test_validate_topic_name, repeated_forward_slashes) { @@ -168,7 +178,9 @@ TEST(test_validate_topic_name, repeated_forward_slashes) { ASSERT_EQ(RMW_TOPIC_INVALID_CONTAINS_REPEATED_FORWARD_SLASH, validation_result); ASSERT_EQ(10ul, invalid_index); - ASSERT_NE((char *)NULL, rmw_full_topic_name_validation_result_string(validation_result)); + ret = strcmp("topic name must not contain repeated '/'", + rmw_full_topic_name_validation_result_string(validation_result)); + ASSERT_EQ(0, ret); } TEST(test_validate_topic_name, starts_with_number) { @@ -192,7 +204,9 @@ TEST(test_validate_topic_name, starts_with_number) { ASSERT_EQ(RMW_TOPIC_INVALID_NAME_TOKEN_STARTS_WITH_NUMBER, validation_result); ASSERT_EQ(8ul, invalid_index); - ASSERT_NE((char *)NULL, rmw_full_topic_name_validation_result_string(validation_result)); + ret = strcmp("topic name must not have a token that starts with a number", + rmw_full_topic_name_validation_result_string(validation_result)); + ASSERT_EQ(0, ret); } TEST(test_validate_topic_name, topic_too_long) { @@ -229,5 +243,8 @@ TEST(test_validate_topic_name, topic_too_long) { EXPECT_EQ(RMW_TOPIC_INVALID_TOO_LONG, validation_result); EXPECT_EQ(RMW_TOPIC_MAX_NAME_LENGTH - 1, invalid_index); - ASSERT_NE((char *)NULL, rmw_full_topic_name_validation_result_string(validation_result)); + const char * result_string = "topic length should not exceed '" RMW_STRINGIFY( + RMW_TOPIC_MAX_NAME_LENGTH) "'"; + ret = strcmp(result_string, rmw_full_topic_name_validation_result_string(validation_result)); + ASSERT_EQ(0, ret); } diff --git a/rmw/test/test_validate_namespace.cpp b/rmw/test/test_validate_namespace.cpp index 55569c17..17c7465e 100644 --- a/rmw/test/test_validate_namespace.cpp +++ b/rmw/test/test_validate_namespace.cpp @@ -72,7 +72,10 @@ TEST(test_validate_namespace, empty_namespace) { ASSERT_EQ(RMW_NAMESPACE_INVALID_IS_EMPTY_STRING, validation_result); ASSERT_EQ(0ul, invalid_index); - ASSERT_NE((char *)NULL, rmw_namespace_validation_result_string(validation_result)); + ret = + strcmp("namespace must not be empty", + rmw_namespace_validation_result_string(validation_result)); + ASSERT_EQ(0, ret); } TEST(test_validate_namespace, not_absolute) { @@ -96,7 +99,9 @@ TEST(test_validate_namespace, not_absolute) { ASSERT_EQ(RMW_NAMESPACE_INVALID_NOT_ABSOLUTE, validation_result); ASSERT_EQ(0ul, invalid_index); - ASSERT_NE((char *)NULL, rmw_namespace_validation_result_string(validation_result)); + ret = strcmp("namespace must be absolute, it must lead with a '/'", + rmw_namespace_validation_result_string(validation_result)); + ASSERT_EQ(0, ret); } TEST(test_validate_namespace, ends_with_forward_slash) { @@ -115,7 +120,9 @@ TEST(test_validate_namespace, ends_with_forward_slash) { ASSERT_EQ(RMW_NAMESPACE_INVALID_ENDS_WITH_FORWARD_SLASH, validation_result); ASSERT_EQ(10ul, invalid_index); - ASSERT_NE((char *)NULL, rmw_namespace_validation_result_string(validation_result)); + ret = strcmp("namespace must not end with a '/', unless only a '/'", + rmw_namespace_validation_result_string(validation_result)); + ASSERT_EQ(0, ret); } TEST(test_validate_namespace, unallowed_characters) { @@ -149,7 +156,9 @@ TEST(test_validate_namespace, unallowed_characters) { ASSERT_EQ(RMW_NAMESPACE_INVALID_CONTAINS_UNALLOWED_CHARACTERS, validation_result); ASSERT_EQ(5ul, invalid_index); - ASSERT_NE((char *)NULL, rmw_namespace_validation_result_string(validation_result)); + ret = strcmp("namespace must not contain characters other than alphanumerics, '_', or '/'", + rmw_namespace_validation_result_string(validation_result)); + ASSERT_EQ(0, ret); } TEST(test_validate_namespace, repeated_forward_slashes) { @@ -168,7 +177,10 @@ TEST(test_validate_namespace, repeated_forward_slashes) { ASSERT_EQ(RMW_NAMESPACE_INVALID_CONTAINS_REPEATED_FORWARD_SLASH, validation_result); ASSERT_EQ(10ul, invalid_index); - ASSERT_NE((char *)NULL, rmw_namespace_validation_result_string(validation_result)); + ret = + strcmp("namespace must not contain repeated '/'", rmw_namespace_validation_result_string( + validation_result)); + ASSERT_EQ(0, ret); } TEST(test_validate_namespace, starts_with_number) { @@ -192,7 +204,9 @@ TEST(test_validate_namespace, starts_with_number) { ASSERT_EQ(RMW_NAMESPACE_INVALID_NAME_TOKEN_STARTS_WITH_NUMBER, validation_result); ASSERT_EQ(8ul, invalid_index); - ASSERT_NE((char *)NULL, rmw_namespace_validation_result_string(validation_result)); + ret = strcmp("namespace must not have a token that starts with a number", + rmw_namespace_validation_result_string(validation_result)); + ASSERT_EQ(0, ret); } TEST(test_validate_namespace, topic_too_long) { @@ -229,5 +243,8 @@ TEST(test_validate_namespace, topic_too_long) { EXPECT_EQ(RMW_NAMESPACE_INVALID_TOO_LONG, validation_result); EXPECT_EQ(RMW_NAMESPACE_MAX_LENGTH - 1, invalid_index); - ASSERT_NE((char *)NULL, rmw_namespace_validation_result_string(validation_result)); + const char * result_string = "namespace should not exceed '" RMW_STRINGIFY( + RMW_NAMESPACE_MAX_NAME_LENGTH) "'"; + ret = strcmp(result_string, rmw_namespace_validation_result_string(validation_result)); + ASSERT_EQ(0, ret); } diff --git a/rmw/test/test_validate_node_name.cpp b/rmw/test/test_validate_node_name.cpp index ce26b64f..af38243a 100644 --- a/rmw/test/test_validate_node_name.cpp +++ b/rmw/test/test_validate_node_name.cpp @@ -67,7 +67,10 @@ TEST(test_validate_node_name, empty_node_name) { ASSERT_EQ(RMW_NODE_NAME_INVALID_IS_EMPTY_STRING, validation_result); ASSERT_EQ(0ul, invalid_index); - ASSERT_NE((char *)NULL, rmw_node_name_validation_result_string(validation_result)); + ret = + strcmp("node name must not be empty", + rmw_node_name_validation_result_string(validation_result)); + ASSERT_EQ(0, ret); } TEST(test_validate_node_name, unallowed_characters) { @@ -106,7 +109,9 @@ TEST(test_validate_node_name, unallowed_characters) { ASSERT_EQ(RMW_NODE_NAME_INVALID_CONTAINS_UNALLOWED_CHARACTERS, validation_result); ASSERT_EQ(4ul, invalid_index); - ASSERT_NE((char *)NULL, rmw_node_name_validation_result_string(validation_result)); + ret = strcmp("node name must not contain characters other than alphanumerics or '_'", + rmw_node_name_validation_result_string(validation_result)); + ASSERT_EQ(0, ret); } TEST(test_validate_node_name, starts_with_number) { @@ -125,7 +130,10 @@ TEST(test_validate_node_name, starts_with_number) { ASSERT_EQ(RMW_NODE_NAME_INVALID_STARTS_WITH_NUMBER, validation_result); ASSERT_EQ(0ul, invalid_index); - ASSERT_NE((char *)NULL, rmw_node_name_validation_result_string(validation_result)); + ret = + strcmp("node name must not start with a number", rmw_node_name_validation_result_string( + validation_result)); + ASSERT_EQ(0, ret); } TEST(test_validate_node_name, node_name_too_long) { @@ -163,5 +171,8 @@ TEST(test_validate_node_name, node_name_too_long) { EXPECT_EQ(RMW_NODE_NAME_INVALID_TOO_LONG, validation_result); EXPECT_EQ(RMW_NODE_NAME_MAX_NAME_LENGTH - 1U, invalid_index); - ASSERT_NE((char *)NULL, rmw_node_name_validation_result_string(validation_result)); + const char * result_string = "node name length should not exceed '" RMW_STRINGIFY( + RMW_NODE_NAME_MAX_NAME_LENGTH) "'"; + ret = strcmp(result_string, rmw_node_name_validation_result_string(validation_result)); + ASSERT_EQ(0, ret); } From c49b6f7a22ea5fcdd6a28d2351150038b2e10717 Mon Sep 17 00:00:00 2001 From: Ethan Gao Date: Wed, 29 Nov 2017 23:29:33 +0800 Subject: [PATCH 3/4] keep NULL for the valid case and adapt tests Signed-off-by: Ethan Gao --- rmw/src/validate_full_topic_name.c | 2 +- rmw/src/validate_namespace.c | 2 +- rmw/src/validate_node_name.c | 2 +- rmw/test/test_validate_full_topic_name.cpp | 34 +++++----------------- rmw/test/test_validate_namespace.cpp | 34 +++++----------------- rmw/test/test_validate_node_name.cpp | 22 ++++---------- 6 files changed, 24 insertions(+), 72 deletions(-) diff --git a/rmw/src/validate_full_topic_name.c b/rmw/src/validate_full_topic_name.c index 2a3395bc..71b4200d 100644 --- a/rmw/src/validate_full_topic_name.c +++ b/rmw/src/validate_full_topic_name.c @@ -118,7 +118,7 @@ rmw_full_topic_name_validation_result_string(int validation_result) { switch (validation_result) { case RMW_TOPIC_VALID: - return "valid topic name"; + return NULL; case RMW_TOPIC_INVALID_IS_EMPTY_STRING: return "topic name must not be empty"; case RMW_TOPIC_INVALID_NOT_ABSOLUTE: diff --git a/rmw/src/validate_namespace.c b/rmw/src/validate_namespace.c index ffc41743..7e33b7de 100644 --- a/rmw/src/validate_namespace.c +++ b/rmw/src/validate_namespace.c @@ -110,7 +110,7 @@ rmw_namespace_validation_result_string(int validation_result) { switch (validation_result) { case RMW_NAMESPACE_VALID: - return "valid namespace"; + return NULL; case RMW_NAMESPACE_INVALID_IS_EMPTY_STRING: return "namespace must not be empty"; case RMW_NAMESPACE_INVALID_NOT_ABSOLUTE: diff --git a/rmw/src/validate_node_name.c b/rmw/src/validate_node_name.c index 3f79dc48..33fde125 100644 --- a/rmw/src/validate_node_name.c +++ b/rmw/src/validate_node_name.c @@ -82,7 +82,7 @@ rmw_node_name_validation_result_string(int validation_result) { switch (validation_result) { case RMW_NODE_NAME_VALID: - return "valid node name"; + return NULL; case RMW_NODE_NAME_INVALID_IS_EMPTY_STRING: return "node name must not be empty"; case RMW_NODE_NAME_INVALID_CONTAINS_UNALLOWED_CHARACTERS: diff --git a/rmw/test/test_validate_full_topic_name.cpp b/rmw/test/test_validate_full_topic_name.cpp index 225f33d6..dfb1f77f 100644 --- a/rmw/test/test_validate_full_topic_name.cpp +++ b/rmw/test/test_validate_full_topic_name.cpp @@ -47,8 +47,7 @@ TEST(test_validate_topic_name, valid_topic) { ASSERT_EQ(RMW_RET_OK, ret); ASSERT_EQ(RMW_TOPIC_VALID, validation_result); - ret = strcmp("valid topic name", rmw_full_topic_name_validation_result_string(validation_result)); - ASSERT_EQ(0, ret); + ASSERT_EQ((char *)nullptr, rmw_full_topic_name_validation_result_string(validation_result)); } TEST(test_validate_topic_name, empty_topic_name) { @@ -67,10 +66,7 @@ TEST(test_validate_topic_name, empty_topic_name) { ASSERT_EQ(RMW_TOPIC_INVALID_IS_EMPTY_STRING, validation_result); ASSERT_EQ(0ul, invalid_index); - ret = - strcmp("topic name must not be empty", rmw_full_topic_name_validation_result_string( - validation_result)); - ASSERT_EQ(0, ret); + ASSERT_NE((char *)nullptr, rmw_full_topic_name_validation_result_string(validation_result)); } TEST(test_validate_topic_name, not_absolute) { @@ -94,9 +90,7 @@ TEST(test_validate_topic_name, not_absolute) { ASSERT_EQ(RMW_TOPIC_INVALID_NOT_ABSOLUTE, validation_result); ASSERT_EQ(0ul, invalid_index); - ret = strcmp("topic name must be absolute, it must lead with a '/'", - rmw_full_topic_name_validation_result_string(validation_result)); - ASSERT_EQ(0, ret); + ASSERT_NE((char *)nullptr, rmw_full_topic_name_validation_result_string(validation_result)); } TEST(test_validate_topic_name, ends_with_forward_slash) { @@ -120,10 +114,7 @@ TEST(test_validate_topic_name, ends_with_forward_slash) { ASSERT_EQ(RMW_TOPIC_INVALID_ENDS_WITH_FORWARD_SLASH, validation_result); ASSERT_EQ(0ul, invalid_index); - ret = - strcmp("topic name must not end with a '/'", rmw_full_topic_name_validation_result_string( - validation_result)); - ASSERT_EQ(0, ret); + ASSERT_NE((char *)nullptr, rmw_full_topic_name_validation_result_string(validation_result)); } TEST(test_validate_topic_name, unallowed_characters) { @@ -157,9 +148,7 @@ TEST(test_validate_topic_name, unallowed_characters) { ASSERT_EQ(RMW_TOPIC_INVALID_CONTAINS_UNALLOWED_CHARACTERS, validation_result); ASSERT_EQ(5ul, invalid_index); - ret = strcmp("topic name must not contain characters other than alphanumerics, '_', or '/'", - rmw_full_topic_name_validation_result_string(validation_result)); - ASSERT_EQ(0, ret); + ASSERT_NE((char *)nullptr, rmw_full_topic_name_validation_result_string(validation_result)); } TEST(test_validate_topic_name, repeated_forward_slashes) { @@ -178,9 +167,7 @@ TEST(test_validate_topic_name, repeated_forward_slashes) { ASSERT_EQ(RMW_TOPIC_INVALID_CONTAINS_REPEATED_FORWARD_SLASH, validation_result); ASSERT_EQ(10ul, invalid_index); - ret = strcmp("topic name must not contain repeated '/'", - rmw_full_topic_name_validation_result_string(validation_result)); - ASSERT_EQ(0, ret); + ASSERT_NE((char *)nullptr, rmw_full_topic_name_validation_result_string(validation_result)); } TEST(test_validate_topic_name, starts_with_number) { @@ -204,9 +191,7 @@ TEST(test_validate_topic_name, starts_with_number) { ASSERT_EQ(RMW_TOPIC_INVALID_NAME_TOKEN_STARTS_WITH_NUMBER, validation_result); ASSERT_EQ(8ul, invalid_index); - ret = strcmp("topic name must not have a token that starts with a number", - rmw_full_topic_name_validation_result_string(validation_result)); - ASSERT_EQ(0, ret); + ASSERT_NE((char *)nullptr, rmw_full_topic_name_validation_result_string(validation_result)); } TEST(test_validate_topic_name, topic_too_long) { @@ -243,8 +228,5 @@ TEST(test_validate_topic_name, topic_too_long) { EXPECT_EQ(RMW_TOPIC_INVALID_TOO_LONG, validation_result); EXPECT_EQ(RMW_TOPIC_MAX_NAME_LENGTH - 1, invalid_index); - const char * result_string = "topic length should not exceed '" RMW_STRINGIFY( - RMW_TOPIC_MAX_NAME_LENGTH) "'"; - ret = strcmp(result_string, rmw_full_topic_name_validation_result_string(validation_result)); - ASSERT_EQ(0, ret); + ASSERT_NE((char *)nullptr, rmw_full_topic_name_validation_result_string(validation_result)); } diff --git a/rmw/test/test_validate_namespace.cpp b/rmw/test/test_validate_namespace.cpp index 17c7465e..13c32a4a 100644 --- a/rmw/test/test_validate_namespace.cpp +++ b/rmw/test/test_validate_namespace.cpp @@ -52,8 +52,7 @@ TEST(test_validate_namespace, valid_namespace) { ASSERT_EQ(RMW_RET_OK, ret); ASSERT_EQ(RMW_NAMESPACE_VALID, validation_result); - ret = strcmp("valid namespace", rmw_namespace_validation_result_string(validation_result)); - ASSERT_EQ(0, ret); + ASSERT_EQ((char *)nullptr, rmw_namespace_validation_result_string(validation_result)); } TEST(test_validate_namespace, empty_namespace) { @@ -72,10 +71,7 @@ TEST(test_validate_namespace, empty_namespace) { ASSERT_EQ(RMW_NAMESPACE_INVALID_IS_EMPTY_STRING, validation_result); ASSERT_EQ(0ul, invalid_index); - ret = - strcmp("namespace must not be empty", - rmw_namespace_validation_result_string(validation_result)); - ASSERT_EQ(0, ret); + ASSERT_NE((char *)nullptr, rmw_namespace_validation_result_string(validation_result)); } TEST(test_validate_namespace, not_absolute) { @@ -99,9 +95,7 @@ TEST(test_validate_namespace, not_absolute) { ASSERT_EQ(RMW_NAMESPACE_INVALID_NOT_ABSOLUTE, validation_result); ASSERT_EQ(0ul, invalid_index); - ret = strcmp("namespace must be absolute, it must lead with a '/'", - rmw_namespace_validation_result_string(validation_result)); - ASSERT_EQ(0, ret); + ASSERT_NE((char *)nullptr, rmw_namespace_validation_result_string(validation_result)); } TEST(test_validate_namespace, ends_with_forward_slash) { @@ -120,9 +114,7 @@ TEST(test_validate_namespace, ends_with_forward_slash) { ASSERT_EQ(RMW_NAMESPACE_INVALID_ENDS_WITH_FORWARD_SLASH, validation_result); ASSERT_EQ(10ul, invalid_index); - ret = strcmp("namespace must not end with a '/', unless only a '/'", - rmw_namespace_validation_result_string(validation_result)); - ASSERT_EQ(0, ret); + ASSERT_NE((char *)nullptr, rmw_namespace_validation_result_string(validation_result)); } TEST(test_validate_namespace, unallowed_characters) { @@ -156,9 +148,7 @@ TEST(test_validate_namespace, unallowed_characters) { ASSERT_EQ(RMW_NAMESPACE_INVALID_CONTAINS_UNALLOWED_CHARACTERS, validation_result); ASSERT_EQ(5ul, invalid_index); - ret = strcmp("namespace must not contain characters other than alphanumerics, '_', or '/'", - rmw_namespace_validation_result_string(validation_result)); - ASSERT_EQ(0, ret); + ASSERT_NE((char *)nullptr, rmw_namespace_validation_result_string(validation_result)); } TEST(test_validate_namespace, repeated_forward_slashes) { @@ -177,10 +167,7 @@ TEST(test_validate_namespace, repeated_forward_slashes) { ASSERT_EQ(RMW_NAMESPACE_INVALID_CONTAINS_REPEATED_FORWARD_SLASH, validation_result); ASSERT_EQ(10ul, invalid_index); - ret = - strcmp("namespace must not contain repeated '/'", rmw_namespace_validation_result_string( - validation_result)); - ASSERT_EQ(0, ret); + ASSERT_NE((char *)nullptr, rmw_namespace_validation_result_string(validation_result)); } TEST(test_validate_namespace, starts_with_number) { @@ -204,9 +191,7 @@ TEST(test_validate_namespace, starts_with_number) { ASSERT_EQ(RMW_NAMESPACE_INVALID_NAME_TOKEN_STARTS_WITH_NUMBER, validation_result); ASSERT_EQ(8ul, invalid_index); - ret = strcmp("namespace must not have a token that starts with a number", - rmw_namespace_validation_result_string(validation_result)); - ASSERT_EQ(0, ret); + ASSERT_NE((char *)nullptr, rmw_namespace_validation_result_string(validation_result)); } TEST(test_validate_namespace, topic_too_long) { @@ -243,8 +228,5 @@ TEST(test_validate_namespace, topic_too_long) { EXPECT_EQ(RMW_NAMESPACE_INVALID_TOO_LONG, validation_result); EXPECT_EQ(RMW_NAMESPACE_MAX_LENGTH - 1, invalid_index); - const char * result_string = "namespace should not exceed '" RMW_STRINGIFY( - RMW_NAMESPACE_MAX_NAME_LENGTH) "'"; - ret = strcmp(result_string, rmw_namespace_validation_result_string(validation_result)); - ASSERT_EQ(0, ret); + ASSERT_NE((char *)nullptr, rmw_namespace_validation_result_string(validation_result)); } diff --git a/rmw/test/test_validate_node_name.cpp b/rmw/test/test_validate_node_name.cpp index af38243a..da56fdbe 100644 --- a/rmw/test/test_validate_node_name.cpp +++ b/rmw/test/test_validate_node_name.cpp @@ -47,8 +47,7 @@ TEST(test_validate_node_name, valid_node_name) { ASSERT_EQ(RMW_RET_OK, ret); ASSERT_EQ(RMW_NODE_NAME_VALID, validation_result); - ret = strcmp("valid node name", rmw_node_name_validation_result_string(validation_result)); - ASSERT_EQ(0, ret); + ASSERT_EQ((char *)nullptr, rmw_node_name_validation_result_string(validation_result)); } TEST(test_validate_node_name, empty_node_name) { @@ -67,10 +66,7 @@ TEST(test_validate_node_name, empty_node_name) { ASSERT_EQ(RMW_NODE_NAME_INVALID_IS_EMPTY_STRING, validation_result); ASSERT_EQ(0ul, invalid_index); - ret = - strcmp("node name must not be empty", - rmw_node_name_validation_result_string(validation_result)); - ASSERT_EQ(0, ret); + ASSERT_NE((char *)nullptr, rmw_node_name_validation_result_string(validation_result)); } TEST(test_validate_node_name, unallowed_characters) { @@ -109,9 +105,7 @@ TEST(test_validate_node_name, unallowed_characters) { ASSERT_EQ(RMW_NODE_NAME_INVALID_CONTAINS_UNALLOWED_CHARACTERS, validation_result); ASSERT_EQ(4ul, invalid_index); - ret = strcmp("node name must not contain characters other than alphanumerics or '_'", - rmw_node_name_validation_result_string(validation_result)); - ASSERT_EQ(0, ret); + ASSERT_NE((char *)nullptr, rmw_node_name_validation_result_string(validation_result)); } TEST(test_validate_node_name, starts_with_number) { @@ -130,10 +124,7 @@ TEST(test_validate_node_name, starts_with_number) { ASSERT_EQ(RMW_NODE_NAME_INVALID_STARTS_WITH_NUMBER, validation_result); ASSERT_EQ(0ul, invalid_index); - ret = - strcmp("node name must not start with a number", rmw_node_name_validation_result_string( - validation_result)); - ASSERT_EQ(0, ret); + ASSERT_NE((char *)nullptr, rmw_node_name_validation_result_string(validation_result)); } TEST(test_validate_node_name, node_name_too_long) { @@ -171,8 +162,5 @@ TEST(test_validate_node_name, node_name_too_long) { EXPECT_EQ(RMW_NODE_NAME_INVALID_TOO_LONG, validation_result); EXPECT_EQ(RMW_NODE_NAME_MAX_NAME_LENGTH - 1U, invalid_index); - const char * result_string = "node name length should not exceed '" RMW_STRINGIFY( - RMW_NODE_NAME_MAX_NAME_LENGTH) "'"; - ret = strcmp(result_string, rmw_node_name_validation_result_string(validation_result)); - ASSERT_EQ(0, ret); + ASSERT_NE((char *)nullptr, rmw_node_name_validation_result_string(validation_result)); } From 59df7e62603fb82bc2f5dccadf9d4f1d3ef7cdb1 Mon Sep 17 00:00:00 2001 From: Ethan Gao Date: Thu, 30 Nov 2017 18:29:05 +0800 Subject: [PATCH 4/4] tweak the default error string returned Signed-off-by: Ethan Gao --- rmw/src/validate_full_topic_name.c | 2 +- rmw/src/validate_namespace.c | 2 +- rmw/src/validate_node_name.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/rmw/src/validate_full_topic_name.c b/rmw/src/validate_full_topic_name.c index 71b4200d..8b59825d 100644 --- a/rmw/src/validate_full_topic_name.c +++ b/rmw/src/validate_full_topic_name.c @@ -134,6 +134,6 @@ rmw_full_topic_name_validation_result_string(int validation_result) case RMW_TOPIC_INVALID_TOO_LONG: return "topic length should not exceed '" RMW_STRINGIFY(RMW_TOPIC_MAX_NAME_LENGTH) "'"; default: - return "undefined topic name type"; + return "unknown result code for rwm topic name validation"; } } diff --git a/rmw/src/validate_namespace.c b/rmw/src/validate_namespace.c index 7e33b7de..3a7a23f7 100644 --- a/rmw/src/validate_namespace.c +++ b/rmw/src/validate_namespace.c @@ -126,6 +126,6 @@ rmw_namespace_validation_result_string(int validation_result) case RMW_NAMESPACE_INVALID_TOO_LONG: return "namespace should not exceed '" RMW_STRINGIFY(RMW_NAMESPACE_MAX_NAME_LENGTH) "'"; default: - return "undefined namespace type"; + return "unknown result code for rmw namespace validation"; } } diff --git a/rmw/src/validate_node_name.c b/rmw/src/validate_node_name.c index 33fde125..f86335c2 100644 --- a/rmw/src/validate_node_name.c +++ b/rmw/src/validate_node_name.c @@ -93,6 +93,6 @@ rmw_node_name_validation_result_string(int validation_result) return "node name length should not exceed '" RMW_STRINGIFY(RMW_NODE_NAME_MAX_NAME_LENGTH) "'"; default: - return "undefined node name type"; + return "unknown result code for rmw node name validation"; } }