[Asterisk-code-review] test.c: Add unit test registration checks for summary and de... (asterisk[13])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged.

Change subject: test.c: Add unit test registration checks for summary and description.
......................................................................


test.c: Add unit test registration checks for summary and description.

Added checks when a unit test is registered to see that the summary and
description strings do not end with a new-line '\n' for consistency.

The check generates a warning message and will cause the
/main/test/registrations unit test to fail.

* Updated struct ast_test_info member doxygen comments.

Change-Id: I295909b6bc013ed9b6882e85c05287082497534d
---
M include/asterisk/test.h
M main/test.c
2 files changed, 39 insertions(+), 10 deletions(-)

Approvals:
  Matt Jordan: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, approved; Verified



diff --git a/include/asterisk/test.h b/include/asterisk/test.h
index 69f8837..49731fe 100644
--- a/include/asterisk/test.h
+++ b/include/asterisk/test.h
@@ -224,13 +224,22 @@
 	/*!
 	 * \brief test category
 	 *
+	 * \details
 	 * Tests are categorized in a directory tree style hierarchy.  It is expected that
 	 * this string have both a leading and trailing forward slash ('/').
 	 */
 	const char *category;
-	/*! \brief optional short summary of test */
+	/*!
+	 * \brief Short summary of test
+	 *
+	 * \note The summary must not end with a newline.
+	 */
 	const char *summary;
-	/*! \brief optional brief detailed description of test */
+	/*!
+	 * \brief More detailed description of test
+	 *
+	 * \note The description must not end with a newline.
+	 */
 	const char *description;
 };
 
diff --git a/main/test.c b/main/test.c
index f7f7023..e9b56eb 100644
--- a/main/test.c
+++ b/main/test.c
@@ -640,33 +640,53 @@
 
 	if (ast_strlen_zero(test->info.category)) {
 		ast_log(LOG_ERROR, "Test %s has no category, test registration refused.\n",
-				test->info.name);
+			test->info.name);
 		return test_free(test);
 	}
 
 	if (test->info.category[0] != '/' || test->info.category[strlen(test->info.category) - 1] != '/') {
 		ast_log(LOG_WARNING, "Test category '%s' for test '%s' is missing a leading or trailing slash.\n",
-				test->info.category, test->info.name);
-		/* Flag an error anyways so test_registrations fails but allow the test to be
-		 * registered. */
-		registration_errors++;
+			test->info.category, test->info.name);
+		/*
+		 * Flag an error anyways so test_registrations fails but allow the
+		 * test to be registered.
+		 */
+		++registration_errors;
 	}
 
 	if (ast_strlen_zero(test->info.summary)) {
 		ast_log(LOG_ERROR, "Test %s%s has no summary, test registration refused.\n",
-				test->info.category, test->info.name);
+			test->info.category, test->info.name);
 		return test_free(test);
+	}
+	if (test->info.summary[strlen(test->info.summary) - 1] == '\n') {
+		ast_log(LOG_WARNING, "Test %s%s summary has a trailing newline.\n",
+			test->info.category, test->info.name);
+		/*
+		 * Flag an error anyways so test_registrations fails but allow the
+		 * test to be registered.
+		 */
+		++registration_errors;
 	}
 
 	if (ast_strlen_zero(test->info.description)) {
 		ast_log(LOG_ERROR, "Test %s%s has no description, test registration refused.\n",
-				test->info.category, test->info.name);
+			test->info.category, test->info.name);
 		return test_free(test);
+	}
+	if (test->info.description[strlen(test->info.description) - 1] == '\n') {
+		ast_log(LOG_WARNING, "Test %s%s description has a trailing newline.\n",
+			test->info.category, test->info.name);
+		/*
+		 * Flag an error anyways so test_registrations fails but allow the
+		 * test to be registered.
+		 */
+		++registration_errors;
 	}
 
 	if (!(test->status_str = ast_str_create(128))) {
 		ast_log(LOG_ERROR, "Failed to allocate status_str for %s%s, test registration failed.\n",
-				test->info.category, test->info.name);
+			test->info.category, test->info.name);
 		return test_free(test);
 	}
 

-- 
To view, visit https://gerrit.asterisk.org/715
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I295909b6bc013ed9b6882e85c05287082497534d
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>