ThrowTheSwitch / Unity

Simple Unit Testing for C
ThrowTheSwitch.org
MIT License
4.1k stars 980 forks source link

Invalid file name reported #744

Closed bremenpl closed 3 months ago

bremenpl commented 3 months ago

Hello there!

I am currently testing unity (current main). I am using the Makefile example provided from here: https://www.throwtheswitch.org/build/make

Everything works but one thing. The file name reporting on the result print. Consider these 3 minimal files:

test_module.c

#include "unity.h"
#include "module.h"

void setUp(void) {
    // Set up code, if needed
}

void tearDown(void) {
    // Tear down code, if needed
}

int main(void) {
    UNITY_BEGIN();

    RUN_TEST(module_run_tests);

    return UNITY_END();
}

module.h

#ifndef __MODULE_H__
#define __MODULE__

#ifdef __cplusplus
 extern "C" {
#endif

void module_run_tests(void);

#ifdef __cplusplus
 }
#endif

#endif

module.c

#include "unity.h"
#include "module.h"

static void test_dummy(void) {
    TEST_ASSERT_EQUAL(NULL, NULL);
}

void module_run_tests(void) {
    RUN_TEST(test_dummy);
}

When tests are ran I get:

$ ./build/test_module.out 
test/test_module.c:9:test_dummy:PASS
test/test_module.c:9:test_dummy:PASS

-----------------------
2 Tests 0 Failures 0 Ignored 
OK

2 Issues here:

  1. Why is the test called twice?
  2. Why does the file report the incorrect file but correct line?
test/test_module.c:9

It should be test/module.c:9. In general, whatever tests I run in separate files, it would always report the file name in which the main function is located. Also, interestingly, when I do this (comment out):

void module_run_tests(void) {
    //RUN_TEST(test_dummy);
}

I get after run:

test/test_module.c:15:module_run_tests:PASS

Which is the line in which RUN_TEST(module_run_tests); is called...

I would appreciate all help in understanding what is the case in here... Thank you!

Letme commented 3 months ago

Why does module.c have unity.h as include as well as it has RUN_TEST(test_dummy); reference. Why dont you just expose test_dummy in module.h and go with that?

  1. It is called twice, because you call it from test file and then again in source file. That is also why filename is from source file as that was the last one to be called and where PASS is printed.
bremenpl commented 3 months ago

Hi @Letme , thank you for the answer.

Why does module.c have unity.h as include as well as it has RUN_TEST(test_dummy); reference. Why dont you just expose test_dummy in module.h and go with that?

I would like to have unit tests separated in different files, for different modules. Otherwise I would need to have a really long list of tests in just one file with the main function right?

It is called twice, because you call it from test file and then again in source file. That is also why filename is from source file as that was the last one to be called and where PASS is printed.

Indeed, silly mistake here from my end here. But the core problem persists. Even when changed to this:

int main(void) {
    UNITY_BEGIN();

    module_run_tests();

    return UNITY_END();
}

I now get:

test/test_module.c:9:test_dummy:PASS

DONE

While I think i should get test/module.c:9:test_dummy:PASS. Please note that the line number is displayed correctly from the module.c, not test_module.c (it would need to be 15, not 9, if it was test_module.c).

Letme commented 3 months ago

Let's separate test code from normal code, because you now sound confusing. I still do not see a reason why src/module.c should include unity.h, but based on what you are saying of separating unit tests I assume you meant you want multiple test/test_module2.c ? So to make it easier the files in test/ test files in src. If you are saying you want a helper function, then just create a helper_module.c, which can have unity.h as reference, but only use ASSERTs in it (not RUN_TEST). Then you can have helper_module.h to share functions between multiple test_module*.c. Is that what you wanted?

bremenpl commented 3 months ago

but based on what you are saying of separating unit tests I assume you meant you want multiple test/test_module2.c

The test_module.c file is maybe misleading. It could be as well be named main.c. I did not want to have multiple files with main functions in it (multiple executables). I wanted to have a single executable with all required tests, for different modules.

So to make it easier the files in test/ test files in src. If you are saying you want a helper function, then just create a helper_module.c, which can have unity.h as reference, but only use ASSERTs in it (not RUN_TEST). Then you can have helper_module.h to share functions between multiple test_module*.c. Is that what you wanted?

Yes its just that the crude example I gace consists of no src code yet. But following this convention, module.c and module.h would be in the test dir.

Maybe my approach is flawed, but I wanted to simply acomplish separate test files for each src module. To clarify things, assume I have main.c, test_somemodule.c and test_somemodule.h. How to have the main function in main.c, and at the same time have test cases in test_somemodule.c, in a way that test_somemodule.c is displayed i

Letme commented 3 months ago

Its exactly my example.

Your main.c is my src/module.c . Since you want to expose main function (which is bad anyway, but I will go with your example), you want src/module.h (would be correctly named inc/module.h to not confuse future readers) which has

#ifndef MODULE_H
#define MODULE_H

#ifdef TEST
int main(void);
#endif /* TEST */
#endif /* MODULE_H */

Then in your test_somemodule.c you have include of module.h (please use correct compiler include paths here, otherwise you will go nuts with paths) to get that main call from module.c. Now test_somemodule.c if I take your example will be:

#include "unity.h"
#include "module.h"
#include "helper_test_module.h" /* This is here so we can share the test_dummy along multiple test_somemodule.c test files */

void setUp(void) {
    // Set up code, if needed
}

void tearDown(void) {
    // Tear down code, if needed
}

int main(void) {
    UNITY_BEGIN();

    RUN_TEST(test_dummy);

    return UNITY_END();
}

helper_test_module.h (where you said will have help modules in test dir - I just named it that way)

#ifndef HELPER_TEST_MODULE_H
#define HELPER_TEST_MODULE_H

/** My ultra flexible test case */
void test_dummy(void);
#endif

helper_test_module.c

void test_dummy(void) {
    TEST_ASSERT_EQUAL(NULL, NULL);
}

I can assume output here will be test/test_module.c:15:test_dummy:PASS

Or you would expect something else or could be that output might be helper_test_module.c:2:test_dummy:PASS?

bremenpl commented 3 months ago

Ok, I think I mislead you due to my poor example. Lets start over if you still have patience... I prepared another example showing what I am trying to acomplish, although this one does not compile for some not obvious reason... Here is the tree of src and test dirs:

$ tree test
test
├── test_gpio.c
├── test_gpio.h
└── test_main.c
$ tree src
src
├── gpio.c
└── gpio.h

So:

The files:

test/test_main.c

#include "unity.h"
#include "test_gpio.h"

void setUp(void) {
    // Set up code, if needed
}

void tearDown(void) {
    // Tear down code, if needed
}

int main(void) {
    UNITY_BEGIN();

    test_gpio_runTests();

    return UNITY_END();
}

test/gpio.h

#ifndef __TEST_GPIO_H__
#define __TEST_GPIO_H__

#ifdef __cplusplus
 extern "C" {
#endif

void test_gpio_runTests(void);

#ifdef __cplusplus
 }
#endif

#endif

test/test_gpio.c

#include "gpio.h"
#include "test_gpio.h"
#include "unity.h"

static void test_gpio_getOne(void)
{
    TEST_ASSERT_EQUAL(gpio_getOne(), 1);
}

void test_gpio_runTests(void)
{
    RUN_TEST(test_gpio_getOne);
}

src/gpio.h

#ifndef __GPIO_H__
#define __GPIO_H__

#ifdef __cplusplus
 extern "C" {
#endif

int gpio_getOne(void);

#ifdef __cplusplus
 }
#endif

#endif

src/gpio.c

#include "gpio.h"

int gpio_getOne(void)
{
    return 1;
}

Is the general approach correct and not confusing? I would appreciate your feedback!

Letme commented 3 months ago

It's less confusing than what you had before, but the question in my mind is:

The first question is mostly because I am wondering how are you mocking. It is much easier to mock files within one binary, than to have mocks and all the other stuff grouped together to figure out if you are calling mocked functions or real implementations. That test_main.c grouping just seems plain wrong, as you want separation when you write tests, to avoid pollution from other sources.

bremenpl commented 3 months ago

why don't you just have test_gpio.c with UNITYBEGIN/END and just create a binary per test file that you run?

My idea was to have a single executable to run the tests for all modules. An example of a module could be gpio.c/h.

If test_gpio.c is like helper, then it does not need RUN_TEST - you might move those to test_main.c, but if it is not, then why does it not have UNITY_BEGIN/END there as well?

If I had more modules, for example another one could be spi.c/h, then with this approach I would add test_spi.c and test_spi.h to place spi specific tests there.

That test_main.c grouping just seems plain wrong, as you want separation when you write tests, to avoid pollution from other sources.

This I understand I think. I thought that I would just reset all modules via setUp and tearDown for each test, but maybe indeed this is bad isolation.

In that case, from your perspective, how should this architecture look like exactly? Should I have a separate binary for each module, i.e., separate main function for test_gpio.c and test_spi.c? If yes, should I use a single Makefle to generate multiple binaries?

bremenpl commented 3 months ago

I am now looking at this example: https://github.com/ThrowTheSwitch/Unity/tree/master/examples/example_1

And in here it indeed seems as if there is a dedicated executable generated for each module to be tested. It is a bit difficult to read because of all the externs https://github.com/ThrowTheSwitch/Unity/blob/master/examples/example_1/test/test_runners/TestProductionCode_Runner.c . But the RUN_TEST define tells me that maybe this one is a bit outdated, since this macro already exists in the unity source code?

Would you advice any state of the art example?

The issue I see here is that it can be somewhat troublesome to maintain via Makefile, since every time a new test is added, it would need to be modified...

Letme commented 3 months ago

You should have a binary for each module yes - well, I have multiple binaries for each module, but like you mentioned this will become quite a big makefile (and hard to maintain). There is no need for Makefile rule per module, so that you will have to modify the makefile each time, but it will require quite some wildcarding to complete this (and some rules).

Here is a small snippet, which might help you with Makefile magic:

C_FILES = $(sort $(wildcard src/*.c)) # gets all C source files and sorts them - you might have modules here
OBJS = $(patsubst src/%.c, obj/%.o, $(C_FILES))  # this should contain all source object files
TEST_C_FILES = $(sort $(wildcard test/test_*.c)) ] gets all C test files and sorts them
TEST_OBJ_FILES = $(patsubst src/%.c, obj/%.o, $(TEST_C_FILES)) # this converts C files to Object files (standard %.o:%.c rule needed)
TEST_OUT_FILES = $(patsubst obj/%.o, build/%.out, $(TEST_OBJ_FILES)) # this converts Object files to OUT/Elf/EXE files (suit yourself)

$(TEST_OUT_FILES): build/%.out : obj/%.o $(OBJS)
    $(HIDE_CMD)$(MKDIR) $(dir $@)
    $(HIDE_CMD)$(CC) $< $(OBJS) -o $@ $(LDFLAGS)

Of course, you will end up soon with more problems once you integrate CMock and other things, where you will need to figure out which files you want in certain binaries based on include paths, so my next proposal is to use Ceedling (https://github.com/ThrowTheSwitch/Ceedling). This will simplify everything and eliminate Makefile pollution (you can just call Ceedling from makefile if you use Makefile for production build). Keep in mind version 1.0.0 was promised to soon be released, so might be good to try pre-releases (for local play they are fully functional).

Forgot to add: there is possibility to use Meson. This is ninja based a bit more modern build system which nicely integrates with Unity and CMock (I still didn't play much with it, but my colleagues made it work).

bremenpl commented 3 months ago

Thank you for the tips and better explanation of the case.

I wanted to start off with Ceedling actually. The issue is that I am working with an existing code that requires unit tests to be added long time after it was started. It is built via self maintained Makefile with a lot of dependencies. So I figured that in order to not break anything, my best chance would be to add a test target to that makefile to create a separate binary for the unit tests. I did that and was happy about it, but now once I learned I actually need to handle multiple binaries, this became problematic again.

Also I did not look that far as to CMock. I do not rule out that it will be needed at some point to properly mock parts of the code, but this would probably take me even longer to setup.

Letme commented 3 months ago

Here is one project I can share (its not perfect, but it is working) https://github.com/melexis/mlx90632-library/blob/master/Makefile#L112-L115

For example rake in this line should be replaced by Ceedling and the ceedling should be used from gem, not as a submodule. But it will probably help how you can not keep everything in Makefile, but you can spin it out. Maybe it helps, but I can tell you from personal experience that Makefile path, along with just CMock requires quite few rules to make Makefiles fairly readable. And if today someone wants me to write this same thing, I would try Meson route, if Ruby is really something you can't stomach. Although, you will not see much ruby code if Ceedling works as expected (except for yml parsing, which they improved in upcoming 1.0.0)

bremenpl commented 3 months ago

Understood, thank you for all help!

bremenpl commented 3 months ago

@Letme , one more question though...

Assuming that I would not need to mock anything and would follow the patch of only single executable for the whole test suite- is there anything else here I am missing that would not work (apart from the incorrect file being printed)?

Letme commented 3 months ago

I cannot imagine running unit tests without mocking nowadays. Maybe it works for integration tests or qualification tests, but as soon as you want to have some control over inputs (changes of inputs at various points), then you will need to mock your inputs that will provide control.

That said, there are few things you need to consider with single executable.

I could have forgotten some things, so don't take this as a complete answer.

bremenpl commented 3 months ago

Thank you for the answer @Letme ,

So luckily for me this is not a real embedded application. Its a generic linuc program written in c that runs on linux (various arch platforms). I decided to Unity because it is a pure c framework, unlike other tools based on cpp.

I also managed to create a separate Makefile that I use only for test purposes. With it, I now produce separate executables like you suggested, so this should hooefully make things easier.

Regarding mocking, since this is no embedded app, I dont think I have much to mock. I could try to mock filesystem or networking operations, but since its linux I can use the files in tempfs or roll out a tcp server. I might be mistaken of course and maybe at some point cmock will become required...

Letme commented 3 months ago

In your use case you indeed could work with single executable, but you could also just split out since you have all the comfort of the PC.

I am not sure how good Cmock will be with UNIX sockets and stuff, but not having to spin up TCP server, can help a lot when writing protocol tests. Then you basically can have everything in your test program, instead of writing program for the spinned up TCP server (which just adds to runtime, while you could do it all in memory).

bremenpl commented 3 months ago

Well Im glad I divided the tests in separate executables, since at least now the log is going to report the right file.

I will check the cmock options once the need arises, as otherwise I wont move forward....

Thank you a lot for help.