Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First (far from done) implementation for printf style formatting for … #500

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

alariois
Copy link
Contributor

@alariois alariois commented May 5, 2020

I'd like to add printf style formating to all TEST_*_MESSAGE macros as long as UNITY_INCLUDE_PRINT_FORMATTED is defined.
I've seen this discussed in some issues and thought to give it a go.

I managed to get a first proof of concept working but as I dont know (yet) the inner working of CMock and Ceedling I'd like some feedback on whether I'm breaking some dependencies. All other suggestions are also welcome of course!

Supported at the moment:

TEST_FAIL_MESSAGE
TEST_ASSERT_MESSAGE
TEST_IGNORE_MESSAGE
TEST_MESSAGE
TEST_PASS_MESSAGE
TEST_ASSERT_TRUE_MESSAGE
TEST_ASSERT_UNLESS_MESSAGE
TEST_ASSERT_FALSE_MESSAGE
TEST_ASSERT_NULL_MESSAGE
TEST_ASSERT_NOT_NULL_MESSAGE
TEST_ASSERT_EQUAL_INT[X]_MESSAGE
TEST_ASSERT_EQUAL_UINT[X]_MESSAGE
TEST_ASSERT_EQUAL_HEX[X]_MESSAGE

For formating and printing I'm using UnityPrintFVA

The biggest possibly breaking change with CMock and Ceedling is that I had to change the ordering of some function parameters. (UnityFail, UnityIgnore, UnityMessage, UnityAssertEqualNumber)
As long as these tools don't call direclty those functions, everything should be ok.

I used a quite hackish (but short) way to define some functions to support variable argument lists and fallbacks without it.

#ifdef UNITY_INCLUDE_PRINT_FORMATTED
#define VA_LIST_IF_ENABLED , va_list va
#define VA_ARGS_IF_ENABLED , ...
#else
#define VA_LIST_IF_ENABLED
#define VA_ARGS_IF_ENABLED
#endif

With this trick its possible to write

void UnityFail(const UNITY_LINE_TYPE line, const char* msg VA_ARGS_IF_ENABLED)

This expands to

void UnityFail(const UNITY_LINE_TYPE line, const char* msg)

if UNITY_INCLUDE_PRINT_FORMATTED is not defined and otherwise to

void UnityFail(const UNITY_LINE_TYPE line, const char* msg, ...)

Also worth noting is that the _MESSAGE macros are defined like

#define TEST_ASSERT_EQUAL_INT_MESSAGE(expected, actual, ...) 

rather than

#define TEST_ASSERT_EQUAL_INT_MESSAGE(expected, actual, format, ...) 

This is to support c99, that needs at least one parameter in VA_ARGS list.
With this it's possible to write

TEST_ASSERT_EQUAL_INT_MESSAGE(1, 2, "I cannot believe this failed"); 

Without any extra parameters

TEST_PRINTF is now alias to TEST_MESSAGE
And now TEST_PRINTF can also be used without any extra parameters like so:

TEST_MESSAGE("Hello, World!");
TEST_MESSAGE("Hello %s! pi = %f", "World", 3.141592);

The ugliest thing about implementing this feature is that all the _MESSAGE macros have to be duplicated. Yikes! Its not possible to use the VA_ARGS_IF_ENABLED trick with macros sadly.

Again ... all suggestions are welcome! :)

@mvandervoord
Copy link
Member

Heya @alariois :

Instead of going through the full list of assertions and creating an alternate version, maybe instead it makes sense to create a single macro which assembles data with sprintf and then it can be dropped into the _MESSAGE portion of any of the existing macros?

@alariois
Copy link
Contributor Author

alariois commented May 5, 2020

Hey @mvandervoord
Yes, I have thought of that also. Actually I have used this method in my own projects, but it's a bit clunky to use IMHO. Also I don't like that with this solution the sprintf function runs and wastes CPU cycles every time even if the assertion succeeds.

Do I understand correctly that you proposed something like this:

TEST_ASSERT_TRUE_MESSAGE(false, FORMAT("Hi %s", "there"));

or calling the FORMAT macro one level lower like so:

TEST_ASSERT_TRUE_MESSAGE(false, "Hi %s", "there");

and in unity.h file

#define TEST_ASSERT_TRUE_MESSAGE(condition, ...) ((condition), __LINE__, FORMAT(__VA_ARGS__))

The latter is cleaner to use but then all the _MESSAGE macros still have to have an alternative version. On top of that the sprintf is still running needlessly every time.

@alariois
Copy link
Contributor Author

alariois commented May 7, 2020

Hi @mvandervoord
I'm almost done with this PR. I think I've covered all the cases now. Have to do some more testing to be sure.
I'd appreciate if you could take a look and test it out :) (no hurry of course)

Maybe there should also be a compiler flag for whether to use UnityPrintFVA or vsprintf for formatting. What do you think?

I know it's a LOT of new macros but I was able to keep the assertion functions in unity.c basically unchanged.

@alariois
Copy link
Contributor Author

Resolves #491

Example outputs:

// Pointer comparison
TEST_ASSERT_NOT_EQUAL_PTR(ptr1, ptr1); // FAIL: Expected 0x0040C400 to be not equal to 0x0040C400
TEST_ASSERT_NOT_EQUAL_PTR(NULL, ptr1); // PASS
TEST_ASSERT_NOT_EQUAL_PTR(ptr1, NULL); // PASS
TEST_ASSERT_NOT_EQUAL_PTR(ptr1, ptr2); // PASS
TEST_ASSERT_NOT_EQUAL_PTR(NULL, NULL); // FAIL: Expected 0x00000000 to be not equal to 0x00000000

// String comparison
TEST_ASSERT_NOT_EQUAL_STRING("Foo", "Foo"); // FAIL: Expected 'Foo' to be not equal to 'Foo'
TEST_ASSERT_NOT_EQUAL_STRING(NULL, "Foo"); // PASS
TEST_ASSERT_NOT_EQUAL_STRING("Foo", NULL); // PASS
TEST_ASSERT_NOT_EQUAL_STRING(NULL, NULL); // FAIL: Expected NULL to be not equal to NULL

// String with length comparison
TEST_ASSERT_NOT_EQUAL_STRING_LEN("FooX", "FooY", 3); // FAIL: Expected 'Foo' to be not equal to 'Foo'
TEST_ASSERT_NOT_EQUAL_STRING_LEN(NULL, "Foo", 3); // PASS
TEST_ASSERT_NOT_EQUAL_STRING_LEN("Foo", NULL, 3); // PASS
TEST_ASSERT_NOT_EQUAL_STRING_LEN(NULL, NULL, 3); // FAIL: Expected NULL to be not equal to NULL
TEST_ASSERT_NOT_EQUAL_STRING_LEN("Foo", "Foo", 0); // FAIL: Expected '' to be not equal to ''
TEST_ASSERT_NOT_EQUAL_STRING_LEN(NULL, NULL, 0); // FAIL: Expected NULL to be not equal to NULL
TEST_ASSERT_NOT_EQUAL_STRING_LEN("Foo", NULL, 0); // PASS
TEST_ASSERT_NOT_EQUAL_STRING_LEN(NULL, "Foo", 0); // PASS

// Memory comparison
TEST_ASSERT_NOT_EQUAL_MEMORY("Foo", "Foo", 3); // FAIL: Expected 3 bytes of memory not to be equal
TEST_ASSERT_NOT_EQUAL_MEMORY(NULL, "Foo", 3); // PASS
EST_ASSERT_NOT_EQUAL_MEMORY("Foo", NULL, 3); // PASS
TEST_ASSERT_NOT_EQUAL_MEMORY(NULL, NULL, 3); // FAIL: Expected NULL to be not equal to NULL
TEST_ASSERT_NOT_EQUAL_MEMORY("Foo", "Foo", 0); // FAIL: You Asked Me To Compare Nothing, Which Was Pointless.

To keep from making 3 totally new assertion functions for these cases I modified UnityAssertEqualString, UnityAssertEqualStringLen, UnityAssertEqualMemory functions by adding a new const UNITY_UINT not parameter to them. If this is 0 then these functions act as before and if it is 1 then the results are inversed.

I guess I should add new unity test cases for these new assertion macros.
Also I think ideally the test cases should test _MESSAGE macros compiling with and without UNITY_INCLUDE_PRINT_FORMATTED defined to test VA_ARGS stuff

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants