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

Tomasz Flendrich t.flendrich at gmail.com
Mon Jun 29 02:07:08 UTC 2015


Mark, everybody,

>> 1. Helper functions in Unity
That's a really great idea, something similar to what Damir Tomić came up
with, but you took it to the next level. It looks very useful, but I have
one concern:
Usually software is divided into the interface and the internals. In C, we
can't really force anyone not to use the internals. I am a little bit
afraid of using them, because usually they aren't said to be backward
compatible. NTP is a very long-term project and if a new version of Unity
comes out, we will want to use it. Do you really think it's okay if we use
stuff from unity_internals.h?

We could just convert the line number from integer to string and use it
with regular TEST_ASSERT_XXX_MESSAGE. I would really feel safer doing this,
but maybe there's no point in doing that at all.



>> 2. Which compare function to use?
>(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.

That's a very good idea - I'll use the HEX version whenever the byte
boundaries may be important.
By the way, there's always UNITY_TEST_ASSERT_EQUAL_HEX64, which is more
than 32 bits.




Thank you for your opinions! Of course they are useful! We really value
them.

Sorry for the previous mail on the mailing list, I think that I finally
figured it all out how to use it properly.

2015-06-27 14:34 GMT+02:00 Mark Vander Voord <mvandervoord at gmail.com>:

> 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