[ntp-GSoC] Unity: agreeing to some convention, code review

Mark Vander Voord mvandervoord at gmail.com
Sat Jun 27 12:34:29 UTC 2015


I'd like to throw in a couple opinions on Tomasz's post.


> 1. Helper functions in Unity
>

I completely agree with his thoughts here. It's better to have tests fail
with as much information as possible. It's really going to save time down
the road when someone accidentally breaks something.

If you have some helpers that are commonly used (just often or even in
multiple test files), then you might consider taking it even a step further
and doing something like this:

void helper_function(int expectedYear, int year, int line)
{
    const char* msg = "information about year and expectedYear";
    UNITY_TEST_ASSERT_EQUAL_INT(expectedYear, year - 1, line, msg);
}
#define HELPER_FUNCTION(expected, actual)
helper_function(expected,year,__LINE__)

Notice (1) the macro that would be the thing actually getting called and
(2) that we are now using the assertions that are prefixed with UNITY_.
These allow us to automatically insert the line number of the TEST that
actually failed instead of the line of the helper. You can differentiate
the lines of the helper by giving good individual messages on each
assertion within the helper.

As a matter of convention, I'd recommend always having the expected values
first.


> 2. Which compare function to use?
>
> Again, I think your intuition is good here. While it's best to use the
macro that matches your actual data, there are instances where you won't be
sure. A couple additional thoughts on this:

(1) Often, even when characters are greater than 8-bits, you only care
about 8-bits worth of that data (or less). In these cases, it is still best
to using a UINT8 comparison. For example, TI DSP's use 16-bit characters,
but the compiler guarantees that the upper byte is always going to be
zero... so there's really no point in treating it like a 16-bit character.
If you know your data is never going to be greater than 255, then you can
safely use the more precise assertion.

(2) When the size isn't a sure thing, going with a larger macro is a really
good idea. In these cases, going with the TEST_ASSERT_EQUAL_UINT is
awesome. Another to consider is TEST_ASSERT_EQUAL_HEX because it is easy to
see where the byte boundaries are when something fails... but this would
only be possible if you knew it was going to be 32bits or less.


>
> 3. Should we test a helper function input?
>

Those assertions are all worth putting into a helper if they are conditions
that can happen and you want to protect against on the "actual" side of the
assertion. I wouldn't go very far with adding things for the "expected"
side... I'd say protecting against things that might crash the test is a
worthwhile practice, but anything else is not likely to be worth the time
investment. A mistake in the test is almost always going to lead to a
failure, so it should get caught anyway.

 ***

Thanks for allowing me to throw in my opinions. I hope they are useful.

Mark


More information about the GSoC mailing list