[regression] Undefined references to setUp and tearDown

[jlindgren90]

Please revert commit 45020b0; it causes the following build errors on MinGW when setUp() and tearDown() are not overridden (i.e. when relying on the weak symbols):

ld.exe: unity.o: in function `UnityDefaultTestRun': unity.c:1748: undefined reference to `setUp'
ld.exe: unity.c:1753: undefined reference to `tearDown'
collect2.exe: error: ld returned 1 exit status

Note that under MinGW, weak symbols need to be defined in the same module that references them. I made a note of this in

Unity/src/unity.h

Line 33 in 2939c42

* implementations of the above functions as weak symbols. Note that on

but it appears to have been overlooked.

The correct fix for #417 would be to make sure that neither UNITY_WEAK_ATTRIBUTE nor UNITY_WEAK_PRAGMA are defined when using the TI C55x compiler. That's how you're supposed to tell Unity that the compiler doesn't support weak symbols, and in that case, UNITY_INCLUDE_SETUP_STUBS will do nothing.

[mvandervoord]

I rejected the PR, but not the need to fix this. I don't love a solution which requires more core files to be includes.

I get that it's almost hypocritical: I spend a lot of time teaching people to break their files into smaller parts and such, but a common complaint that we get with Unity is that it's not all in a single header file. I'd like to not make it MORE files.

I'm not sure I follow the logic in the 2nd statement, that it "does not work if unity.h is included
(directly or indirectly) via a command-line directive." That seems to be a place where it easily DOES work. If UNITY_INCLUDE_SETUP_STUBS is defined at the command line, then it's ALWAYS defined when needed, right? How is that a problem?

We've attempted "optional" setUp and tearDown a number of ways now, primarily with weak functions. Thus far it's been fairly problematic because of the inconsistencies. GCC's support of weak changes from version to version. Other compilers don't support it at all, or support it in a completely different way. What started as a convenience idea for those using gcc has ballooned into a complicated mess of options.

I'm considering dropping all the weak support and returning to a point where setUp and tearDown are always required again. For those using Ceedling and/or the test runner generator, we could have it automatically look to see if the functions exist in each test file, and generate the runner to have replacements if they don't. I think this would solve it in a compiler agnostic way.

Thoughts?

[Letme]

I agree with the mandatory setup-teardown definition. Maybe even an error message explaining you to add empty stubs for that (so that we check if they exist).

[mvandervoord]

Thanks for the comment, @Letme , I appreciate your input!

[jlindgren90]

Making setUp/tearDown mandatory is not an attractive option to me because it will break a number of tests across different projects. I don't know the exact number, but I expect it to be large enough that pushing a breaking change like that would not be a good idea.

The situation with suiteSetUp/suiteTearDown will be worse, because we started using them more recently, after the weak symbol support was added, and most of our tests do not have them present -- but the minority that do have them, need them. So either we also make suiteSetUp/suiteTearDown mandatory, and the majority of tests fail to compile; or we disable suiteSetUp/suiteTearDown by default, and the tests that need them still compile, but start failing in less obvious ways due to missing initializations.

The most likely outcome is that I'll end up maintaining the weak symbol support in a fork of Unity instead. I understand the desire to keep the number of #includes small, but surely removing features and fragmenting the user base due to the forks that result is also a concern?

[mvandervoord]

optional setUp, tearDown, suiteSetup, etc., would all be available still. We're just changing the method of supporting it: instead of relying on weak symbols which have been having issues, we'd allow the generators to do the right thing and provide them as needed. Does that make sense?

[jlindgren90]

I'm not sure I follow the logic in the 2nd statement, that it "does not work if unity.h is included
(directly or indirectly) via a command-line directive." That seems to be a place where it easily DOES work. If UNITY_INCLUDE_SETUP_STUBS is defined at the command line, then it's ALWAYS defined when needed, right? How is that a problem?

To answer just this particular point, the issue is not when UNITY_INCLUDE_SETUP_STUBS is defined at the command line, but when unity.h is included at the command line ("-iunity.h").

I don't have a problem with the idea of the test generator detecting when setUp() etc. are defined using some Ruby-based parsing, if it ends up being robust. I guess you avoid depending on some compiler-specific features at a cost of replacing those features yourself.

[mvandervoord]

:) Ah. I hadn't ever considered that someone might force-include a header with a command line. What's the advantage of doing it this way?

As for your second point: Yes. That's exactly what I'm suggesting. Since the C standard doesn't have weak, it's handled in many different ways. We either continue to wrestle with trying to support ALL the ways (which has been an effort that has lasted nearly 4 years and is still getting change requests like this one), or we ignore the compilers' methods and we do it ourselves. As the person who did the very first version of weak support, I definitely WANT it in there... but my original plan of using the compilers built-in functionality isn't paying off well. ;)

[mvandervoord]

@jlindgren90 & @Letme -- if I post a PR today or tomorrow with a proposed test-runner version of this feature, do you have time to give it a try?

[jlindgren90]

:) Ah. I hadn't ever considered that someone might force-include a header with a command line. What's the advantage of doing it this way?

Maybe no real advantage -- a coworker had done so temporarily while trying to debug a different problem, and the fact that doing so broke Unity, seemed less than ideal. Since adding UNITY_INCLUDE_SETUP_STUBS was originally my fault (#296), #443 was my attempt to clean up my own mess :)

[jlindgren90]

@jlindgren90 & @Letme -- if I post a PR today or tomorrow with a proposed test-runner version of this feature, do you have time to give it a try?

For sure!

[Letme]

Tomorrow evening I will have some time to test it, so just reference me in the PR.

[Xenoamor]

What was the resolution to this issue? I see UNITY_INCLUDE_SETUP_STUBS is no longer with us so I can't use that fix

[mvandervoord]

If you're using the test runner generator script or ceedling, the functions will be automatically generated for you if you don't include them yourself. If you're hand-building a runner, you can build it in whatever manner you wish, making this feature unnecessary. Finally, if you're using the built-in test runner in Unity, it requires setUp and tearDown be defined, which has always been the fallback.

[Xenoamor]

Cheers! I've removed the #define and added my own stubbed functions