ThrowTheSwitch / Unity

Simple Unit Testing for C
ThrowTheSwitch.org
MIT License
4.07k stars 976 forks source link

RUN_TEST macro in unity_internals.h has problems #323

Closed jwalkerbg closed 5 years ago

jwalkerbg commented 6 years ago

Hi,

I tried to use UnityDefaultTestRun instead of the the macro RUN_TEST from test_runner.c and fell in two issues.

1) RUN_TEST has one argument only while the generated calls ro RUN_TEST have two arguments. 2) The UnityDefaultTestRun(func, #func, LINE) -- LINE generates number that is the line where RUN_TEST is called instead of where the test function is located in the test file.

https://github.com/ThrowTheSwitch/Unity/blob/d9cd6988f3766689a10c490dd30676311ebc94f1/src/unity_internals.h#L615

Regards Ivan

mvandervoord commented 6 years ago

Ivan:

Why are you trying to call UnityDefaultTestRun directly? There's no reason to ever do that, I don't believe. It's intended that you would always call one of the RUN_TEST macros (depending on the feature set).

Mark

jwalkerbg commented 6 years ago

Hi,

I use Unity as part of ceedling (0.28.2).

I saw that UnityDefaultTestRun does the same that RUN_TEST, generated by generate_test_runner.rb does. Besides that, I use XC8 and PIC16 in MPLAB X simulator of Microchip and sometimes there is no enough flash. In fact I use RUN_TEST macro in the generated test runner. Using UnityDefaultTestRun crates smaller executable files (hex).

In my installation, I made small additions in generate_test_runner.rb and one of them was to add new option, :use_unity_DefaultTestRun (in project.yml), that makes generate_test_runner.rb to not generate RUN_TEST macro. This way, RUN_TEST macro from unity_internals.h is used that calls UnityDefaultTestRun. So I do use RUN_TEST.

I did not see any differences in behavior when RUN_TEST for the test runner is used and when UnityDefaultTestRun is used. By the way, RUN_TEST is present in unity_internals.h, and I was curious to see how to use it.

Ivan

mvandervoord commented 6 years ago

Sorry, I'm not following what the issue is then. It sounds like everything is working as intended?

You have the choice to use the RUN_TEST fallback mechanism in the header file, which is a valid option. As you pointed out, it gives you less information, because it's the fallback mechanism. It's to be used for systems that can't handle a full test runner.

Or is this not what you are saying?

jwalkerbg commented 6 years ago

I think that RUN_TEST macro in unity_internals.h must receive two parameters. Instead, it receives only one - the name of the test. This way, UnityDefaultTestRun's argument const int FuncLineNum receives the value of __LINE__ at the moment of the RUN_TEST call which is wrong. It should receive the actual line, where the test is in the test file.

I edited RUN TEST macro this way:

from: #define RUN_TEST(func) UnityDefaultTestRun(func, #func, __LINE__) to #define RUN_TEST(func, line_num) UnityDefaultTestRun(func, #func, (line_num))

and it works well now.

Ivan

mvandervoord commented 6 years ago

I think I see, now. The title of this issue suggests that there is something wrong with the RUN_TEST macro in unity_internals, but that's not true. You just don't like the quick version. You'd like to have an option to wrap the default test run routine in Unity and pass in a line number.

This is a really good idea. It provides a nice middle ground for people who are looking to have the more advanced features of Ceedling provided by the runner generator, but still have significant memory constraints.

I don't agree with the way you've gone about it on your system... hacking out the quick version that some people already appreciate isn't really the way to go, but it would be easy for me to add an option to the runner generator which doesn't generate a full RUN_TEST macro... it just thinly wraps the default handler.

It should be noted that this solution (as well as the one you are doing right now) are incompatible with running mocks, unless you add the mock handlers on your own.

jwalkerbg commented 6 years ago

Yes, I understood now that I did may be the right thing in the wrong place. unity_internals.h should not be touched. Instead,

#define RUN_TEST(func, line_num) UnityDefaultTestRun(func, #func, (line_num))

could be generated by generate_test_runner.rb.

Ivan

mvandervoord commented 6 years ago

Agreed. Overall, though, this is a great idea. I'm glad that you came up with it!

If you feel up to updating gererate_test_runner to have this option, that would be awesome. Otherwise, I'll do it when I return from vacation.

Thanks for talking this out!

jwalkerbg commented 6 years ago

I feel that I don't know some things that are important in ceedling internals and I am not sure that I could do the work .

I tried and have partial success. I managed to achieve working solution in Microchip's MPLAB X simulator. However, when I tried with a small test module that I created by 'ceedling new np', the things are OK when RUN_TEST macro is generated. When UnityDefaultTestRun call is generated, then ceedling says that there is no output generated to stdout.

Ivan

mvandervoord commented 5 years ago

The RUN_TEST macro at this point supports one or two arguments, depending on the features enabled.