ThrowTheSwitch / Unity

Simple Unit Testing for C
ThrowTheSwitch.org
MIT License
4k stars 967 forks source link

Issue with Discovery of Included Mocks In Test Runner Generator #689

Open zpif opened 1 year ago

zpif commented 1 year ago

The find_includes function in auto/generate_test_runner.rb scans the source code to find #include directives in the input file, enumerating the "local", "system" and "linkonly" include files. For a reason that I cannot fathom, it then wraps all system include files in "<" and ">". So, if the source code contains,

#include "MockFoo.h"
#include <MockBar.h>

then the list of "local" include files will contain MockFoo.h, and the list of "system" include files will contain <MockBar.h>.

The code then passes both the "local" and the "system" include files to the find_mocks function. For each include file, it tests if the file name begins with the mock prefix, e.g., "Mock" and ends with ".h". But, because of the <>, this regular expression never matches for any of the "system" includes.

Therefore, for mocks included with <>, the corresponding Mock Init, Verify and Destroy functions will never be called.

Removing the .map { |inc| "<#{inc}>" } from line 199 fixes this issue for me with no perceived side effects.

mvandervoord commented 1 year ago

This behavior exists because some embedded compilers enforce the usage of "" for files belonging to your project and <> for system files. GCC is sloppy in this regard (and personally I don't mind the sloppiness... it makes life easier), but there are compilers which are not. If you issue an include for "stdint.h" it's going to look in your project for your own personalized stdint. If you include <stuff.h> it's going to search for a header named stuff.h in the library include paths.

Are you using <> because the file being mocked is a system header? Even if that's the case, by the time we create a mock of it, it's no longer a system file. Typically tests are written to use "" for all the mocked files, as they're part of the project and not system files.

Make sense?

zpif commented 1 year ago

I understand. However, with that logic, why even pass the "system" include files to the find_mocks function when there is no possibility of them ever matching, and when there is a policy that <> shall not be used to include mocks?

It is confusing because it currently sort of works to use <> to include other mocks: the generated code compiles and runs, for the most part as expected. However, in our case, it currently results in false positives because the _Verify() functions are not called, and therefore unfulfilled expectations are ignored.

mvandervoord commented 1 year ago

That's an excellent question. It does seem we have a mismatch in features here. Also, as we're talking this out, I'm realizing that IF the compiler actually does have a problem with it, it'll complain on its own without the test runner generator needing to enforce it... so the script should just support whichever the user wants to do and assume they know what they're doing.

Thanks for bringing this up, and for being willing to talk it out. :)

AndersonMartins1 commented 6 months ago

It appears that the issue lies in the behavior of the find_includes function in auto/generate_test_runner.rb, which wraps all system include files in < and >. This causes a problem because the regular expression used to identify mock files (Mock*.h) doesn't match files included with <>.

The suggested solution is to remove the wrapping of system include files, which would allow the regular expression to match both local and system include files correctly. However, there's a concern about the necessity of passing system include files to the find_mocks function if they will never match the mock file pattern.

One member suggests that the usage of <> for system files is enforced by some compilers, while others argue that by the time mocks are created, they are no longer system files and should be included with "" instead.

The conclusion drawn from the discussion is that the script should support whichever include style the user prefers and assume they know what they're doing. Additionally, there seems to be agreement that passing system include files to find_mocks when they cannot match the mock file pattern is a mismatch in features that should be addressed.

Overall, the solution involves updating the script to support both "" and <> include styles and revisiting the logic for passing system include files to find_mocks.