I heard a great development process idea from Tom Forsyth where he explained that he litters his code with assert(true) to make sure that he hits all the key code paths during early testing.
This got me thinking about asserts and code coverage.
Code coverage is an evil that kills the sole of testers. It is treated as an absolute and they are forced to try to hit 100%. A much more efficient approach is to have humans spend real thinking power about where and what needs to have code coverage checked. Sure this is not black box testing, but a clever mix of black box, API, edge case, and coverage testing is a much better approach.
I know limited testing can always be argued against, but in the real world we need to ship a product and we need to know it is as good as it needs to be for the real use. It is easy to add more code and more bugs to solve cases that are not possible to reach. Only a careful human programmer and user review will know what needs testing and what can be dropped.
External code coverage tools are great and they can be automated, but this leads to the 100% problem and also adds a new phase in the development process. One that often comes after the code and not during the development.
This leads us back to Tom’s experience and common sense approach to iteratively test while developing the code.
So what might an improved and super simple approach look like.
I thought that assert() could be replaced with a more complex macro that would allow a report to say what asserts in the code have been missed. It can also be __VA_ARGS__ expanded to have coverage of logic in the same line. Assert makes sure the boolean is always true, and later booleans can be ‘at least once’ true. Something like this:
if (m_isInitialised)
{
m_assert(m_data != nullptr);
...
}
....
m_assert(index < m_size, size == 0, size == << 16);
Then in a testing program, using DEBUG and TESTING set, an end of main function can print out the results:
storage.h:line 134 never reached to test "m_data != nullptr"
storage.h:line 356 coverage not hit to check "size == 0"C++ Does Not Do Code Inspection
So how can C++ know we want record the line of code that is never hit?
There are many approaches that don’t work because of the way C++ is designed.
A simple approach could have been to get static initialization to do some of the work for us.
However, the rules of C++ mean we cant have global initialisation in the middle of functions where the actual assert might be put using replacement macro for assert();
void test(void)
{
int i{0};
class singleton
{
public:
static const singleton* instance()
{
static testing123 instance;
return &instance;
}
singleton()
{
printf("Pre-main code has been run...\n");
}
//Error C2246: illegal static data member in locally defined class
inline static const singleton* m_instance = instance();
};
i++;
};
This means we have to consider a solution using the global scope to run the pre-main code. We also have an issue with how to get to know where the asserts are in the code. The only solution now is to use __COUNTER__ which will increment for each occurrence even if the code is not actually run. The assert solution needs to work in either a .h or a .cpp and this can be an issue with the included files already incrementing __COUNTER__. The solution to this is get a recording of the __COUNTER__ before and after the file contents – excluding any other includes.
It can be seen that we started with using a singleton class. However this excellent post on Static Registration Macros showed that a simple struct with a constructor at global scope will also get the code run before main().
However there is the issue of Static Initialization Order Fiasco which means the order of when files are included and initialised is a problem. There is also an even deeper issue with early initialisation in that static items exist in more than one place and the linker needs to decide which code is to be used. If the solution uses the preprocessor. At least on MSVC a static function (with some asserts) is run from a different compile to the static initialisations. Thus the __COUNTER__ do not match up.
This could be fixed with putting a COUNTER_BASE in ever file like this post suggested.
enum { COUNTER_BASE = __COUNTER__ };
#define LOCAL_COUNTER (__COUNTER__ - COUNTER_BASE)
However this needs a unique name for the enum. Which requires the assert to know this unique name as well. The nicest solution for this is to have the unique name provided by the user. Still not as nice as we would like.
#define ASSERT_FILE assertTest_h
_assertStart();
void function(void)
{
_assert(true);
};
_assertEnd();
#undef ASSERT_FILE
Another solution is to work out the range and then patch up the values once some asserts are triggered in running code. This is a bit nicer looking but has the issue it may be confusing when reading the report. It also has the unique name required twice and for it to match.
_assertFile(assertTest_h);
void function(void)
{
_assert(true);
};
_assertFile(assertTest_h);
The next step required putting this all together. Not as easy as it seems. The order of static initialisation was tricky as there are the three parts, start and end that are not in a function and the assert part that is in normal local code. So to get the right values to the right place in the right order was tricky. The easiest solution was to have each item be callable at any time. This did mean the COUNTER_BASE trick had to be used so the correct counter could be used regardless of the order.
There is also the issue that it would be nice to put an _assert() into code that itself is running pre-main initialisation. So having a simpler approach helped in this case.
Also to solve the include files classing when used in several compile units the start/end had to be wrapped in an anonymous namespace. Even with this each variable/name used had to have the filename concatenated onto it to stop those clashes.
MSVC Configuration Manager
Clone the Debug configuration and then add _TESTING to the Preprocessor Definitions entry (under Configuration Properties -> C/C++ -> Preprocessor).
The final code looks like this:
#if !defined(_TESTING)
#define ASSERT_GET_FIRST_ARG(_1, ...) _1
#define _assert(...) assert(EXPAND(ASSERT_GET_FIRST_ARG(__VA_ARGS__)))
#define ASSERT_START()
#define ASSERT_END()
#define ASSERT_REPORT(X)
#endif .// !defined(_TESTING)
#if defined(_TESTING)
#define ASSERT_GET_FIRST_ARG(_1, ...) _1
#define ASSERT_GET_NTH_ARG(_1, _2, _3, _4, _5, N, ...) N
#define ASSERT_COUNT_ARGS(...) EXPAND(ASSERT_GET_NTH_ARG (__VA_ARGS__, 5, 4, 3, 2, 1))
#define ASSERT_1(N, M, C, X) EXPAND(ASSERT_COVERAGE(N, M, C, X);)
#define ASSERT_2(N, M, C, X, ...) EXPAND(ASSERT_COVERAGE(N, M, C, X); ASSERT_1(N+1, M, C, __VA_ARGS__))
#define ASSERT_3(N, M, C, X, ...) EXPAND(ASSERT_COVERAGE(N, M, C, X); ASSERT_2(N+1, M, C, __VA_ARGS__))
#define ASSERT_4(N, M, C, X, ...) EXPAND(ASSERT_COVERAGE(N, M, C, X); ASSERT_3(N+1, M, C, __VA_ARGS__))
#define ASSERT_5(N, M, C, X, ...) EXPAND(ASSERT_COVERAGE(N, M, C, X); ASSERT_4(N+1, M, C, __VA_ARGS__))
#define ASSERT_START() \
namespace \
{ \
static_assert(true, STRINGIFY(ASSERT_FILENAME)" must be string literal"); \
enum { CONCATENATE(ASSERT_FILENAME, _counterBase) = __COUNTER__ }; \
struct CONCATENATE(ASSERT_FILENAME, _start) \
{ \
CONCATENATE(ASSERT_FILENAME, _start) () \
{ \
constexpr const u32 hash = elfHash(__FILE__); \
assertCoverage::addFileStart(hash, STRINGIFY(ASSERT_FILENAME)); \
} \
} CONCATENATE(ASSERT_FILENAME, _startInstance); \
} \
#define _assert(...) \
static_assert(true, STRINGIFY(ASSERT_FILENAME)" must be string literal"); \
EXPAND \
( \
ASSERT_GET_NTH_ARG(__VA_ARGS__, ASSERT_5, ASSERT_4, ASSERT_3, ASSERT_2, ASSERT_1) \
( \
0, \
ASSERT_COUNT_ARGS(__VA_ARGS__), \
(__COUNTER__ - CONCATENATE(ASSERT_FILENAME, _counterBase)), \
__VA_ARGS__ \
) \
) \
assert(ASSERT_GET_FIRST_ARG(__VA_ARGS__)); \
#define ASSERT_COVERAGE(N, M, C, ASSERT_BOOL) \
{ \
constexpr const u32 hash = elfHash(__FILE__); \
char* coverageString = (char*)STRINGIFY(ASSERT_BOOL); \
bool coverageHit = (bool)(ASSERT_BOOL); \
assertCoverage::addCoverage(hash, C, __LINE__, N, coverageString, coverageHit); \
} \
#define ASSERT_END() \
namespace \
{ \
struct CONCATENATE(ASSERT_FILENAME, _end) \
{ \
CONCATENATE(ASSERT_FILENAME, _end) () \
{ \
constexpr const u32 hash = elfHash(__FILE__); \
const u32 counter = __COUNTER__ - CONCATENATE(ASSERT_FILENAME, _counterBase); \
assertCoverage::addFileEnd(hash, counter); \
} \
} CONCATENATE(ASSERT_FILENAME, _endInstance); \
} \
#define ASSERT_REPORT(X) assertCoverage::report(CONCATENATE(assertCoverage::format::, X))
class assertCoverage
{
public: // defines
enum class format { verbose, minimal, summary };
private: // defines
static inline constexpr u32 _filenameCapacity{64}; // Longest filename string
static inline constexpr u32 _filesCapacity{64}; // Max number of files in project
static inline constexpr u32 _assertsCapacity{64}; // Max number of asserts in one file
static inline constexpr u32 _coveragesCapacity{5}; // assert + up to 4 coverage booleans
static inline constexpr u32 _coveragesStringCapacity{64}; // Longest coverage string
public: // interface
static void addFileStart(u32 hash, const char* filename)
{
u32 filesIndex = _getFilesIndex(hash);
file& file = _files[filesIndex];
strcpy_s(file._filename, _filenameCapacity, filename);
}
static void addFileEnd(u32 hash, u32 counter)
{
u32 filesIndex = _getFilesIndex(hash);
file& file = _files[filesIndex];
file._assertsSize = counter - 1;
}
static void addCoverage(u32 hash, u32 counter, u32 lineNumber, u32 coveragesIndex, char* coverageString, bool coverageHit)
{
u32 filesIndex = _getFilesIndex(hash);
file &file = _files[filesIndex];
u32 assertsIndex = counter - 1;
assert(assertsIndex < _assertsCapacity);
file::assert &assert = file._asserts[assertsIndex];
assert._line = lineNumber;
assert(coveragesIndex < _coveragesCapacity);
assert._coveragesSize = _max(assert._coveragesSize, coveragesIndex + 1);
file::assert::coverage &coverage = assert._coverages[coveragesIndex];
strcpy_s(coverage._coverageString, _coveragesStringCapacity, coverageString);
if (coverageHit) coverage._coverageHit++;
}
static void report(format format)
{
if (format == format::verbose)
{
printf("=============================\n");
for (u32 fileIndex = 0; fileIndex < _filesSize; fileIndex++)
{
file& file = _files[fileIndex];
printf("%x: %s\n", file._hash, file._filename);
for (u32 assertIndex = 0; assertIndex < file._assertsSize; assertIndex++)
{
file::assert& assert = file._asserts[assertIndex];
if (assert._line == 0)
{
printf("#%d: Line unknown as assert not reached:\n", assertIndex);
continue;
}
printf("#%d: Line %d:\n", assertIndex, assert._line);
for (u32 coverageIndex = 0; coverageIndex < assert._coveragesSize; coverageIndex++)
{
file::assert::coverage& coverage = assert._coverages[coverageIndex];
printf(" %d: %s\n", coverage._coverageHit, coverage._coverageString);
}
}
}
}
if ((format == format::minimal) || (format == format::verbose))
{
printf("=============================\n");
for (u32 fileIndex = 0; fileIndex < _filesSize; fileIndex++)
{
file& file = _files[fileIndex];
for (u32 assertIndex = 0; assertIndex < file._assertsSize; assertIndex++)
{
file::assert& assert = file._asserts[assertIndex];
if (assert._line == 0)
{
if (assertIndex == 0)
{
printf("%s:?? (first assert) assert not reached.\n", file._filename);
continue;
}
if (assertIndex == file._assertsSize - 1)
{
printf("%s:?? (last assert) assert not reached.\n", file._filename);
continue;
}
u32 previousLine = _files[fileIndex]._asserts[assertIndex - 1]._line;
u32 nextLine = _files[fileIndex]._asserts[assertIndex + 1]._line;
printf("%s:?? (between line %d and %d) assert not reached.\n", file._filename, previousLine, nextLine);
continue;
}
for (u32 coverageIndex = 0; coverageIndex < assert._coveragesSize; coverageIndex++)
{
file::assert::coverage& coverage = assert._coverages[coverageIndex];
if (coverage._coverageHit == 0)
{
printf("%s:%d coverage not hit. %s\n", file._filename, assert._line, coverage._coverageString);
}
}
}
}
}
u32 covered{0};
u32 total{0};
for (u32 fileIndex = 0; fileIndex < _filesSize; fileIndex++)
{
file& file = _files[fileIndex];
for (u32 assertIndex = 0; assertIndex < file._assertsSize; assertIndex++)
{
file::assert& assert = file._asserts[assertIndex];
if (assert._line == 0)
{
total++;
continue;
}
for (u32 coverageIndex = 0; coverageIndex < assert._coveragesSize; coverageIndex++)
{
file::assert::coverage& coverage = assert._coverages[coverageIndex];
total++;
if (coverage._coverageHit > 0) covered++;
}
}
}
printf("Assert Coverage: %d of %d. [%s]\n", covered, total, (covered == total) ? "DONE" : "TODO");
}
private: // functions
assertCoverage() = default;
static const assertCoverage* _getInstance()
{
static assertCoverage instance;
return &instance;
}
static u32 _getFilesIndex(u32 hash)
{
u32 filesIndex{0};
while (filesIndex < _filesSize)
{
if (_files[filesIndex]._hash == hash) break;
filesIndex++;
}
assert(filesIndex < _filesCapacity);
_files[filesIndex]._hash = hash;
_filesSize = _max(_filesSize, filesIndex + 1);
return filesIndex;
}
private: // members
inline static const assertCoverage* _instance = _getInstance();
inline static u32 _filesSize{0};
inline static struct file
{
u32 _hash;
char _filename[_filenameCapacity];
u32 _assertsSize;
struct assert
{
u32 _line;
u32 _coveragesSize;
struct coverage
{
u32 _coverageHit;
char _coverageString[_coveragesStringCapacity];
} _coverages[_coveragesCapacity];
} _asserts[_assertsCapacity];
} _files[_filesCapacity];
};
#endif // defined(_TESTING)