8
\$\begingroup\$

Simple UnitTest macros utility in C. I decided to make this to be able to more easily organize my tests and I emphasized on readability. Please tell me what you think and if this utility would perform well in big unit tests.

timer.h

/* Simple timer macros utility */

#ifndef CMC_TIMER_H
#define CMC_TIMER_H

#include <time.h>

typedef struct timer_s
{
    clock_t start;
    clock_t stop;
    double result;
} timer_t;

#define TIMER_START(timer) \
    timer.start = clock();

#define TIMER_STOP(timer) \
    timer.stop = clock();

#define TIMER_CALC(timer) \
    timer.result = (double)(((timer.stop - timer.start) * 1000.0) / CLOCKS_PER_SEC);

#endif /* CMC_TIMER_H */

test.h

/**
 * Simple Unit Test Utility
 *
 * CMC_CREATE_UNIT
 *     Create a UnitTest
 *     Parameters:
 *         - UNAME   : UnitTest name (const char *)
 *         - VERBOSE : Print PASSED / FAILED messages if true (bool)
 *         - BODY    : Body containing all tests related to this UnitTest
 *
 * CMC_CREATE_TEST
 *     Create a Test inside a UnitTest.
 *     Parameters:
 *         - TNAME : Test name (const char *)
 *         - BODY  : Block of code containing a specific test
 *     Inside the BODY use the following macros:
 *         - CMC_TEST_PASS
 *             Pass current test.
 *         - CMC_TEST_FAIL
 *             Fail current test.
 *         - CMC_TEST_PASS_ELSE_FAIL
 *             If given expression is true, pass current test, else fail.
 *         - CMC_TEST_ABORT
 *             Abort current unit test and displays error message. All tests
 *             after the abort won't be called.
 *     All of these macros can only be called once per test.
 *
 * CMC_TEST_COLOR
 *      Define this macro to allow colored output.
 */

#ifndef CMC_TEST_H
#define CMC_TEST_H

#include "timer.h"
#include <inttypes.h>
#include <stdbool.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#ifdef CMC_TEST_COLOR
static const char *cmc_test_color[] = {"\x1b[32m", "\x1b[31m", "\x1b[35m"};
#endif

typedef struct cmc_test_info_s
{
    uintmax_t total;
    uintmax_t passed;
    uintmax_t failed;
    bool aborted;
    bool verbose;
} cmc_test_info;

/* Logging function */
static void cmc_test_log(const char *unit_name, const char *current_test, bool aborted, bool passed)
{
    if (aborted)
    {
#ifdef CMC_TEST_COLOR
        printf("UNIT_TEST %s %sABORTED\x1b[0m AT %s\n",
               unit_name,
               cmc_test_color[2],
               current_test);
#else
        printf("UNIT_TEST %s ABORTED at %s\n", unit_name, current_test);
#endif
    }
    else
    {
#ifdef CMC_TEST_COLOR
        printf("Test from %s %s%7s\x1b[0m %s\n",
               unit_name,
               passed ? cmc_test_color[0] : cmc_test_color[1],
               passed ? "PASSED" : "FAILED",
               current_test);
#else
        printf("Test from %s %7s %s\n", unit_name, passed ? "PASSED" : "FAILED", current_test);
#endif
    }
}

#define CMC_CREATE_UNIT(UNAME, VERBOSE, BODY)                                               \
    void UNAME(void)                                                                        \
    {                                                                                       \
        const char *unit_name = #UNAME;                                                     \
        const char *current_test = NULL;                                                    \
                                                                                            \
        cmc_test_info tinfo = {0};                                                          \
        timer_t timer = {0};                                                                \
                                                                                            \
        tinfo.verbose = VERBOSE;                                                            \
                                                                                            \
        /* Tests */                                                                         \
        TIMER_START(timer);                                                                 \
        BODY;                                                                               \
        TIMER_STOP(timer);                                                                  \
        TIMER_CALC(timer);                                                                  \
                                                                                            \
        unittest_abort:                                                                     \
        if (tinfo.aborted)                                                                  \
        {                                                                                   \
            cmc_test_log(unit_name, current_test, true, false);                             \
        }                                                                                   \
                                                                                            \
        printf("+--------------------------------------------------+\n");                   \
        printf("| Unit Test Report : %-30s|\n", unit_name);                                 \
        printf("|   TOTAL  : %10" PRIuMAX "                            |\n", tinfo.total);  \
        printf("|   PASSED : %10" PRIuMAX "                            |\n", tinfo.passed); \
        printf("|   FAILED : %10" PRIuMAX "                            |\n", tinfo.failed); \
        printf("|                                                  |\n");                   \
        printf("| Total Unit Test Runtime : %9.0f milliseconds |\n", timer.result);         \
        printf("+--------------------------------------------------+\n");                   \
    }

#define CMC_CREATE_TEST(TNAME, BODY) \
    do                               \
    {                                \
        current_test = #TNAME;       \
                                     \
        tinfo.total += 1;            \
                                     \
        BODY;                        \
                                     \
    } while (0);

#define CMC_TEST_PASS()                                         \
    do                                                          \
    {                                                           \
        tinfo.passed += 1;                                      \
        if (tinfo.verbose)                                      \
            cmc_test_log(unit_name, current_test, false, true); \
                                                                \
    } while (0)

#define CMC_TEST_FAIL()                                          \
    do                                                           \
    {                                                            \
        tinfo.failed += 1;                                       \
        if (tinfo.verbose)                                       \
            cmc_test_log(unit_name, current_test, false, false); \
                                                                 \
    } while (0)

#define CMC_TEST_PASS_ELSE_FAIL(EXPRESSION) \
    do                                      \
    {                                       \
        if (EXPRESSION)                     \
            CMC_TEST_PASS();                \
        else                                \
            CMC_TEST_FAIL();                \
                                            \
    } while (0)

#define CMC_TEST_ABORT()      \
    do                        \
    {                         \
                              \
        tinfo.aborted = true; \
        tinfo.total -= 1;     \
        goto unittest_abort;  \
                              \
    } while (0)

#endif /* CMC_TEST_H */

example.c

#include "test.h"

uintmax_t gcd(uintmax_t p, uintmax_t q)
{
    if (p == 0)
    {
        return q;
    }

    if (q == 0)
    {
        return p;
    }

    uintmax_t r = p % q;

    while (r != 0)
    {
        p = q;
        q = r;
        r = p % q;
    }

    return q;
}

CMC_CREATE_UNIT(gcd_test, true, {

    CMC_CREATE_TEST(edge_cases, {
        uintmax_t r1 = gcd(0, 0);
        uintmax_t r2 = gcd(0, 9);

        CMC_TEST_PASS_ELSE_FAIL(r1 == 0 && r2 == 9);
    })

    CMC_CREATE_TEST(test1, {

        CMC_TEST_PASS_ELSE_FAIL(gcd(42, 56) == 14);
    })

    CMC_CREATE_TEST(test3, {
        CMC_TEST_FAIL();
    })

    CMC_CREATE_TEST(test2, {
        uintmax_t r = gcd(12, 0);

        if (r == 12)
            CMC_TEST_ABORT();
        else
            CMC_TEST_PASS();
    })

    CMC_CREATE_TEST(test4, {

        uintmax_t r1 = gcd(461952, 116298);
        uintmax_t r2 = gcd(7966496, 314080416);

        CMC_TEST_PASS_ELSE_FAIL(r1 + r2 == 50);
    })

})

int main(void)
{
    gcd_test();
    return 0;
}

This is part of the C Macro Collections library hosted here.

\$\endgroup\$

2 Answers 2

9
\$\begingroup\$

Statement-like macros should normally be wrapped in do...while(0). They should also avoid multiple expansion of arguments. So instead of

#define TIMER_START(timer) \
    timer.start = clock();

#define TIMER_STOP(timer) \
    timer.stop = clock();

#define TIMER_CALC(timer) \
    timer.result = (double)(((timer.stop - timer.start) * 1000.0) / CLOCKS_PER_SEC);

I'd recommend

#define TIMER_START(timer)                      \
    do { (timer).start = clock(); } while (0)

#define TIMER_STOP(timer)                       \
    do { (timer).stop = clock(); } while (0)

#define TIMER_CALC(timer)                               \
    do {                                                \
        struct timer_s* t = &(timer);                   \
        t->result = (double)(t->stop - t->start)        \
            * 1000.0 / CLOCKS_PER_SEC;                  \
    } while (0)

or (more likely) a set of real functions instead.

(I fixed the cast that was in the wrong place - it's the result of the subtraction that's an integer value that may lose precision when promoted to double for the multiplication).


typedef struct timer_s timer_t collides with a POSIX reserved identifier, so probably worth avoiding.


Please don't embed terminal-specific codes like this:

static const char *cmc_test_color[] = {"\x1b[32m", "\x1b[31m", "\x1b[35m"};

That becomes an impenetrable mess (or worse) on terminal types other than the ones you've considered. It's better to adapt to the actual known terminal type, perhaps using a library such as termcap.


It seems strange that VERBOSE is a compile-time setting, rather than a run-time parameter here:

#define CMC_CREATE_UNIT(UNAME, VERBOSE, BODY)

One of the statement-like macros has a stray semicolon after its definition:

#define CMC_CREATE_TEST(TNAME, BODY) \
    {                                \
    ...                              \
    } while (0);

+= 1 is more idiomatically written as ++:

    ++tinfo.total;

Similarly, write -= 1 using --.


We'd like the unit-test program to exit with success status only if all tests succeeded. If any fail, we want to know (e.g. to stop the build at that point). I suggest creating the test function with an int return type to support this, and ending with return tinfo.aborted || tinfo.failed.


The behaviour when CMC_TEST_ABORT() is used is strange. It's the only code that's setting tinfo.aborted, so we could move the if (tinfo.aborted) block directly into that code. OTOH, we shouldn't be printing the elapsed time in that case, as we've skipped the TIMER_STOP(timer); and TIMER_CALC(timer); lines.


Finally, I know it's not really up for review, but I couldn't resist making observations on the gcd() function used for testing:

There's no need for a specific p==0 test - the flow without the test already does the right thing (but the q==0 test is required, as it's used as divisor in the % operation).

We can reduce duplication, by moving p % q into the test:

uintmax_t gcd(uintmax_t p, uintmax_t q)
{
    if (q == 0) {
        return p;
    }

    uintmax_t r;
    while ((r = p % q) != 0) {
        p = q;
        q = r;
    }

    return q;
}
\$\endgroup\$
7
  • \$\begingroup\$ Never heard of do {...} while(0) in the context of macros. Learnt something new/old again. \$\endgroup\$
    – AlexV
    Commented Jun 26, 2019 at 11:02
  • \$\begingroup\$ Could you add an example of how to improve static const char *cmc_test_color[] = {"\x1b[32m", "\x1b[31m", "\x1b[35m"}; with termcap? \$\endgroup\$ Commented Jun 26, 2019 at 14:34
  • \$\begingroup\$ That's not something I ever do from C (I'm generally happy with plain output). If you're having trouble using termcap/terminfo, then Stack Overflow may be a better place to get help. \$\endgroup\$ Commented Jun 26, 2019 at 14:43
  • \$\begingroup\$ typedef struct timer_s timer_t collides with a reserved identifier (all typedef names ending in _t are reserved for future Standard Library use). I was going to call that one, but I didn't because I didn't find the text in the Standard. Could you quote the specific section, please? \$\endgroup\$ Commented Jun 26, 2019 at 15:50
  • 1
    \$\begingroup\$ I was using CPPReference, which cites its sources as C99 section 7.26 and C11 section K.3.1.2. \$\endgroup\$ Commented Jun 26, 2019 at 15:55
3
\$\begingroup\$

- __func__

__func__ exists for a reason; use it instead of const char *unit_name = #UNAME;:

cmc_test_log(__func__, current_test, true, false);
...
printf("| Unit Test Report : %-30s|\n", __func__);

- printf("%s", NULL);

Strictly speaking, that is Undefined Behaviour. glibc has a trick, and prints (null) instead, but that trick is very unreliable, because if gcc decides to optimize printf into puts, then UB is invoked (read more: Adding newline character to printf() changes code behaviour on Stack Overflow).

So this code is probably invoking UB, given that all arguments are known at compile time, and the format string ends in \n (current_test was defined here: const char *current_test = NULL; (Why const and NULL???)).

#ifdef CMC_TEST_COLOR
        printf("UNIT_TEST %s %sABORTED\x1b[0m AT %s\n",
                        unit_name,
                        cmc_test_color[2],
                        current_test);
#else

- Differentiate typedefed identifiers from variable names

The easiest thing is to use _s for structs (Or just don't typedef at all) (not _t; it is reserved by POSIX). You did it once, but forgot to do it in one of the typedefs (cmc_test_info).

Solution:

struct cmc_test_info_s {
    uintmax_t total;
    uintmax_t passed;
    uintmax_t failed;
    bool aborted;
    bool verbose;
};
typedef struct cmc_test_info_s cmc_test_info_s;

- Function-like macros

Macros that behave like function calls should be named with lowercase to simulate functions, and help differentiate them of other macros that don't behave like functions:

#define timer_calc(timer)   do                              \
{                                                           \
    struct timer_s  *t_ = timer;                            \
    double          diff;                                   \
                                                            \
    diff        = t_->stop - t_->start;                     \
    t_->result  = diff * 1000.0 / CLOCKS_PER_SEC;           \
} while (0)

#define CMC_CREATE_UNIT(UNAME, verbose, BODY)               \
void UNAME(void)                                            \
{                                                           \
...
                                                            \
}

Variables local to a macro should use names that are unlikely to be used in the calling function to avoid shadowing a variable (Imagine what would happen if the calling function called the macro this way: timer_calc(t_);). The usual convention is to add a trailing underscore to names local to a macro.


- macros that depend on having a local variable with a magic name (source: Linux Kernel Coding Style)

#define FOO(val) bar(index, val)

might look like a good thing, but it’s confusing as hell when one reads the code and it’s prone to breakage from seemingly innocent changes.

Easy solution:

#define cmc_test_fail(tinfo, current_test)  do              \
{                                                           \
    struct cmc_test_info_s  tinfo_  = tinfo;                \
                                                            \
    tinfo_.failed++;                                        \
    if (tinfo_.verbose)                                     \
        cmc_test_log(__func__, current_test, false, false); \
                                                            \
} while (0)
\$\endgroup\$
2
  • \$\begingroup\$ For "Why const and NULL" -- why not? It's not char* const, which would be a bit odd but would be a reasonable way to give NULL a better, context-specific name. It's const char*. \$\endgroup\$
    – anon
    Commented Jun 26, 2019 at 18:04
  • \$\begingroup\$ @NicHartley True, I interpreted it wrongly as const char *const. But the question was in the sense that NULL is not a valid current_test value, and it would be better to give it some valid value, or leave it uninitialized, or ask the user to input a string as a variable. And because of that, I mixed some ideas :) \$\endgroup\$ Commented Jun 26, 2019 at 18:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.