C program implementing a simple test framework

Question

I have written a template for a C program implementing a simple test framework.

If the program is run with the first parameter "test", all subsequent parameters will be interpreted as a sequence of test cases (without the test_ prefix) to be executed. If no other parameters are specified after the first parameter "test", all functions named test_* will be executed in the sequence specified in the TEST_CASES macro.

If one of the test cases fails, the program will report this and stop. In no test case fails, All test cases completed successfully will be logged.

Normally I would split the program into a header, general function definitions and test case files, but for SE, I thought it would be easier post the amalgamated code.

What do you think?

/*template.c - program template with a basic test framework*/
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
/*To add a new test case for function foo(), add a line "X(foo) \" to the 
 TEST_CASES macro and then define the concrete test case function as -
 result_t *test_foo(result_t *result){
// Code to test foo() goes here. If the test is successful, set 
// result->rc = SUCCESS. Messages should be added to result->string using 
// the cat_message function
}
*/
#define TEST_CASES \
 X(init_string) \
X(cat_message) \
X(parse_tc) \
X(get_all_tc)
/*Return codes*/
typedef enum {
 SUCCESS,
 ERR_MALLOC,
 ERR_REALLOC,
 ERR_GENERAL
} rc_t;
/*String object : array of char with allocated size*/
typedef struct {
 size_t size;
 char *str;
} string_t;
/*Test results object : string object with return code*/
typedef struct {
 string_t *string;
 rc_t rc;
} result_t;
/*tc_func_t : test case function signature */
typedef result_t *(*tc_func_t)(result_t *result);
/*Test case execution sequence*/
typedef struct {
 int num_tc; /* number in test cases in the tc_funcs array */
 tc_func_t *tc_funcs; /*pointer to an array of test case functions*/
} tc_seq_t;
/*report_func_t : report results function signature*/
typedef rc_t (*report_func_t)(result_t *result);
/*Function prototypes*/
/*Initialise string object */
string_t *init_string(size_t size);
/*Concatenate a message to the string object*/
string_t *cat_message(const char *message, string_t *string);
/*Count number of test cases in TEST_CASES*/
int count_tc(void);
/*Create a test case set from the command line parameters*/
tc_seq_t *parse_tc(int argc, char **argv);
/*Create a test case set with all test cases specified in the TEST_CASES macro*/
tc_seq_t *get_all_tc(void);
/*Run all test cases in the test case sequence and report results*/
result_t *run_tests(tc_seq_t *tc_seq, report_func_t report_func, 
 result_t *result);
/*Write results to STDOUT*/
rc_t print_stdout(result_t *result);
/*Test case prototypes are generated by the X macro expansion*/
#define X(name) result_t *test_ ## name(result_t *result);
TEST_CASES
#undef X
string_t *init_string(size_t size){
 string_t *string = malloc(sizeof *string);
 /*If an error occurs, return a NULL pointer*/
 if (string == NULL) return NULL;
 string->str = malloc(size);
 if (string->str == NULL){
 free(string);
 return NULL;
 }
 string->size = size;
 *string->str = '0円';
 return string;
}
string_t *cat_message(const char *message, string_t *string){
 /*If the caller does supply the correct parameters, return the string object 
 * unchanged*/
 if (message == NULL || string == NULL || string->str == NULL) return string;
 while (strlen(message) >= string->size - strlen(string->str)){
 /*If the memory allocated to the string object is insufficient, keep 
 * doubling the allocation until it is enough to concancenate the message*/
 string->str = realloc(string->str, string->size * 2);
 if (string->str == NULL) return string; 
 string->size *= 2;
 }
 memcpy(string->str+strlen(string->str), message, strlen(message)+1);
 return string;
}
int count_tc(void){
 int num_tc = 0;
#define X(name) num_tc++;
 TEST_CASES
#undef X
 return num_tc;
}
tc_seq_t *parse_tc(int argc, char **argv){
 /*If the first parameter is not "test", return a NULL pointer*/
 if (argc == 1 || strcmp(argv[1], "test")) return NULL;
 /*if the first and only parameter is "test", return all test cases */
 if (argc == 2) return get_all_tc();
 /*return test cases specified in the parameters after "test"*/
 tc_seq_t *tc_set = malloc(sizeof *tc_set);
 tc_set->tc_funcs = malloc((argc - 2) * sizeof *tc_set->tc_funcs);
 /*If an error occurs, return a NULL pointer*/
 if (tc_set->tc_funcs == NULL) {
 free(tc_set);
 return NULL;
 }
 int tc_ind = 0;
 /*Fill the array testcase->tc_funcs[] with pointers to test functions*/
 for (int i = 2; i < argc; i++){
#define X(name) if (!strcmp(argv[i], #name)) \
 {tc_set->tc_funcs[tc_ind] = test_ ## name; tc_ind++; break;}
 TEST_CASES
#undef X
 }
 tc_set->num_tc = argc - 2;
 return tc_set;
}
tc_seq_t *get_all_tc(void){
 tc_seq_t *tc_set = malloc(sizeof *tc_set);
 if (tc_set == NULL) return NULL;
 tc_set->tc_funcs = malloc(count_tc() * sizeof *tc_set->tc_funcs);
 /*If an error occurs, return a NULL pointer*/
 if (tc_set->tc_funcs == NULL) {
 free(tc_set);
 return NULL;
 }
 tc_set->num_tc = count_tc();
 /*Fill the array testcase->tc_funcs[] with pointers to test functions*/
 int tc = -1;
#define X(name) tc++; tc_set->tc_funcs[tc] = test_ ## name;
 TEST_CASES
#undef X
 return tc_set;
}
result_t *run_tests(tc_seq_t *tc_seq, report_func_t report_func, 
 result_t *result){
 for (int tc_ind = 0; tc_ind < tc_seq->num_tc; tc_ind++) {
 result = tc_seq->tc_funcs[tc_ind](result);
 if (result->rc != SUCCESS) break;
 }
 if (result->rc == SUCCESS) cat_message(
 "All test cases completed successfully\n", result->string);
 free(tc_seq->tc_funcs);
 free(tc_seq);
 report_func(result);
 return result;
}
rc_t print_stdout(result_t *result){
 if (result == NULL) return ERR_GENERAL;
 if (result->string == NULL) return ERR_GENERAL;
 if (result->string->str == NULL) return ERR_GENERAL;
 printf("%s", result->string->str);
 printf("Return code: %d\n", result->rc);
 return SUCCESS;
}
int main(int argc, char **argv){
 string_t *string = init_string(1024);
 if (string == NULL) return ERR_MALLOC;
 string = cat_message("Start log\n", string);
 if (string == NULL) return ERR_REALLOC;
 result_t *result = malloc(sizeof *result);
 if (result == NULL) { 
 free(string->str);
 free(string);
 return ERR_GENERAL;
 } 
 result->string = string;
 result->rc = SUCCESS;
 tc_seq_t *tc_seq = parse_tc(argc, argv);
 if (tc_seq != NULL) result = run_tests(tc_seq, print_stdout, result);
 free(string->str);
 free(string);
 free(result);
}
result_t *test_init_string(result_t *result){
 string_t *string = init_string(10);
 if (string == NULL){
 result->rc = ERR_MALLOC;
 result->string = cat_message("init_string failed\n", result->string);
 return result;
 }
 if (string->size == 10 && !strcmp(string->str, "")){
 result->rc = SUCCESS;
 } else {
 result->rc = ERR_GENERAL;
 result->string = cat_message("init_string failed\n", result->string);
 }
 free(string->str);
 free(string);
 return result;
}
result_t *test_cat_message(result_t *result){
 string_t *string = init_string(10);
 if (string == NULL){
 result->rc = ERR_MALLOC;
 return result;
 }
 string = cat_message("foo", string);
 if (string == NULL){
 result->rc = ERR_REALLOC;
 result->string = cat_message("cat_message failed\n", result->string);
 return result;
 }
 string = cat_message("bar", string);
 if (string == NULL){
 result->string = cat_message("cat_message failed\n", result->string);
 result->rc = ERR_REALLOC;
 return result;
 }
 if (string->size == 10 && !strcmp(string->str, "foobar")){
 result->rc = SUCCESS;
 } else {
 result->string = cat_message("cat_message failed\n", result->string);
 result->rc = ERR_GENERAL;
 }
 free(string->str);
 free(string);
 return result;
}
result_t *test_parse_tc(result_t *result){
 char *argv[] = {"template", "test", "parce_tc", "get_all_tc"};
 tc_seq_t *tc = parse_tc(4, argv);
 if (tc == NULL){
 result->string = cat_message("parse_tc failed\n", result->string);
 result->rc = ERR_GENERAL;
 return result;
 }
 if (tc->num_tc == 2) {
 result->rc = SUCCESS;
 } else {
 result->rc = ERR_GENERAL;
 result->string = cat_message("parse_tc failed\n", result->string);
 }
 free(tc->tc_funcs);
 free(tc);
 return result;
}
result_t *test_get_all_tc(result_t *result){
 tc_seq_t *tc = get_all_tc();
 if (tc == NULL){
 result->rc = ERR_GENERAL;
 result->string = cat_message("get_all_tc failed\n", result->string);
 return result;
 }
 if (tc->num_tc == count_tc()) {
 result->rc = SUCCESS;
 } else {
 result->rc = ERR_GENERAL;
 result->string = cat_message("get_all_tc failed\n", result->string);
 }
 free(tc->tc_funcs);
 free(tc);
 return result;
}

• Toby Speight: Something I often do when posting as a single file is to write an empty comment line <!-- --> at each file break. This makes it easy to copy+paste into a file, but still leaves it clear where the boundaries are.

Answer 1

What do you think?

Well laid out consistently, but a bit tight.
Good basic error handling.
Good uses of sizeof *obj in allocation calculations.
Good used of (void) with such function declarations.
Compiled well without warnings.

---

Interface unclear

Certainly the comment was meant to be "... caller does not supply ..."

When cat_message() returns, how to determine it it succeeded or failed given:
"If the caller does supply the correct parameters, return the string object * unchanged"?
Recommended to return an error flag, value or bool.

Name space

Functions names init_string(), cat_message(), count_tc() and others share little to hint that they work together. Recommend a common prefix.

Doubling unclear.

In cat_message(), why repetitively call realloc() if code can determine the minimal needs? Just allocate once: maximum of 2x former size and size needed.

Inefficientcy

In cat_message(), code re-calculates strlen(string->str) and strlen(message) incurring a run down each string. Just save the lengths and re-use. Specifically the strlen(string->str) is an issue as code may have called realloc() and code cannot assume the 2nd call to strlen(string->str) returns the same length.

---

Minor

0 corner

init_string(0) leads to UB due to *string->str = '0円';. Suggest code gracefully handle this corner. Maybe return NULL.

unsigned math wrap-around

Watch out for underflow of unsigned math with size_t. Both below should work for OP, yet the 2nd form is more resilient to unexpected string->size, string->str pairs.

// while (strlen(message) >= string->size - strlen(string->str))
while (strlen(message) + strlen(string->str) >= string->size )

Assumption

Code assumes argc == 0 never occurs and so argv[1] is not guarded.

// if (argc == 1 || strcmp(argv[1], "test")) return NULL;
if (argc <= 1 || strcmp(argv[1], "test")) return NULL;

---

Later.

Information hiding

Code does not detail what might be the *.h part and implementation part. To this end:

typedef struct {
 size_t size;
 char *str;
} string_t;

and the other 3 typedefs only need to declare the pointer type with typedef struct string_t *string_t; and then declare the functions. The definition of the type can occurs later and not in the public *.h section.

How many rc_t?

Often code needs to assess the range of an enumerated type. Useful to add a _N as the last one.

/*Return codes*/
typedef enum {
 SUCCESS,
 ERR_MALLOC,
 ERR_REALLOC,
 ERR_GENERAL,
 ERR_N // add
} rc_t;

Non-unique error messages

cat_message("cat_message failed\n", result->string); used 3 times. Maybe cat_message("cat_message failed 1\n", result->string);, etc. to help distinguish reported error.

• user2309803: thanks @chux 1. yes, I missed the word "not" from the comment at the start of cat_message 2. I agree a shared name prefix would be useful for these functions. 3. In hindsight, I see the repetitive realloc is inefficient - I should realloc at least strlen(string->str) + strlen(message) 4. I assumed the compiler would optimise away the repeated strlen(x) calls but I agree, they should not there in the first place. 5. init_string(0) does not make sense, I will check size > 0. 6. You say watch out for underflow, how could this happen with my while(...) expression?
• user2309803: 7. I thought argv[0] was always the name of the executable so isn't argc always >= 1? 8. I was going put all code above string_t *init_string(size_t size){ in a header file since these declarations are used by the internal interfaces of the program, why is this not advisable? 8. I always put ERR_GENERAL in the last position of the enum declaration. 9. I agree error messages should be unique.
• chux: @user2309803 3: Off by 1, realloc at least strlen(string->str) + strlen(message) + 1 4 Compiler cannot optimize away 2nd strlen(string->str) as string->str may have changed. 6. Unexpected string->size, string->str pairs could result from broken code.
• chux: @user2309803 7 argc always >= 1 no. It may be 0. C spec: "The value of argc shall be nonnegative." 8. Not advisable as it is not needed for public to see it and potential mess with it (see # 6) or write code that depends on it. 8. I was speaking of enum declarations in general.
• user2309803: you could add a subsection "Potential execution of NULL function pointers", To avoid undefined behaviour, there should be a check that tc_funcs[tc_ind] and report_func are not NULL pointers before execution, agree?