Skip to content

Conversation

jlindgren90
Copy link
Contributor

I'm submitting this pull request as a "Request For Comment" since it's a little bit of a design change to the test-runner-generator, so I'd like to hear any concerns or ideas as to how to do this better.

(Summary from the commit message follows:)

This standardizes the form of many of the functions in the auto-generated
test runners, using the UnityRunner_ prefix. The initial goal was to make
UnityRunner_Verify() accessible from within tests -- this is useful for
verifying the call chain halfway through a longer, integration-style test.

A new UnityRunner_RunTest function replaces the generated RUN_TEST macro.
It requires test functions to be of the form "void func(void)", so
parameterized tests are now handled by creating a tiny wrapper that passes
the desired arguments to the actual test.

Commonizing UnityRunner_RunTest significantly reduces the size of the
compiled binary. On amd64, the largest test runner in the test suite
(testsample_DefaultsThroughCommandLine_runner.o) was reduced from 3.3 kB
to 2.4 kB (stripped).

A new header, unity_runner_common.h, contains the implementation of
UnityRunner_RunTest. This header is included in each generated test runner,
so users shouldn't need to do anything with the new header directly.

The :framework option is no longer supported. I don't think that setting
this to anything besides "unity" would have worked anyway. The generated
test runners use functions in unity.h, so they need to include unity.h.
All other changes should not affect existing usage.

@mvandervoord
Copy link
Member

I'm going to guess you're not also a user of CMock or Ceedling?

The customization of the Runner is an important feature for pulling in mocking frameworks like CMock or FFF. I'm not saying this is a bad direction. I'd have to think about it a bit. It could just be that we add additional macros to support injecting mock calls in appropriate places.

I'll come back to this with a fresh brain (hopefully soon). :)

@jlindgren90
Copy link
Contributor Author

jlindgren90 commented Nov 29, 2018

We are using CMock but not Ceedling.

I may have not described how this change works very well. All of the UnityRunner functions except RunTest() are still generated by the Ruby script. UnityRunner_Init/Verify/Destroy are just renames of CMock_Init/Verify/Destroy and still do all the mock management they did before.

So all the mocks should still "just work", and all the customization options are still there, with the single exception of the :framework header override (I suspect that that one hasn't worked for a while).

@mvandervoord
Copy link
Member

Ah, I am more clear on the change now. :)

@jlindgren90
Copy link
Contributor Author

I will split out the fixes for the setup stubs / weak symbols into a separate pull request, since that's a smaller change, and then rebase this one on it once that's merged.

@jlindgren90
Copy link
Contributor Author

Rebased on top of #443.

Requiring a macro like UNITY_INCLUDE_SETUP_STUBS to be defined before
including unity.h is fragile and does not work if unity.h is included
(directly or indirectly) via a command-line directive.

This also resolves issue #438.
This standardizes the form of many of the functions in the auto-generated
test runners, using the UnityRunner_ prefix.  The initial goal was to make
UnityRunner_Verify() accessible from within tests -- this is useful for
verifying the call chain halfway through a longer, integration-style test.

A new UnityRunner_RunTest function replaces the generated RUN_TEST macro.
It requires test functions to be of the form "void func(void)", so
parameterized tests are now handled by creating a tiny wrapper that passes
the desired arguments to the actual test.

Commonizing UnityRunner_RunTest significantly reduces the size of the
compiled binary.  On amd64, the largest test runner in the test suite
(testsample_DefaultsThroughCommandLine_runner.o) was reduced from 3.3 kB
to 2.4 kB (stripped).

A new header, unity_runner_common.h, contains the implementation of
UnityRunner_RunTest.  This header is included in each generated test runner,
so users shouldn't need to do anything with the new header directly.

The :framework option is no longer supported.  I don't think that setting
this to anything besides "unity" would have worked anyway.  The generated
test runners use functions in unity.h, so they need to include unity.h.
All other changes should not affect existing usage.
@mvandervoord
Copy link
Member

For users not using any of the test runner generation, mocking, etc., using this proposed method, they would still need to create all these functions, correct?

UnityRunner_TearDown();
UnityRunner_Verify();
UnityRunner_Destroy();
UnityRunner_Init();
UnityRunner_SetUp();

Is there a simple way to make the default behavior be that they don't have to specify anything extra?

@jlindgren90
Copy link
Contributor Author

jlindgren90 commented Oct 21, 2019

Users not using the test runner generation don't need to do anything. The declarations for UnityRunner_SetUp() etc. in unity.h will be useless because the functions are not implemented anywhere, but that's fine because nothing in unity.c calls them; they are not required.

@mvandervoord
Copy link
Member

Hm. I was still misunderstanding. Sorry about that.

I now see you're not having RUN_TEST get defined as UnityRunner_RunTest by default? So a person who wishes to not use Ruby at all would still use the existing RUN_TEST macro (or provide their own), while a person using the test runner generator script would (unknowingly) be using UnityRunner_RunTest.

Correct?

@jlindgren90
Copy link
Contributor Author

Yes, that's correct. The generated runner no longer contains a (local) RUN_TEST macro, but the default one is still available in unity_internals.h and resolves to UnityDefaultTestRun() as before.

You could think of UnityRunner_RunTest() as an "enhanced" version of UnityDefaultTestRun() that integrates better with the generated test runner (and isn't used, nor useful, without it).

@mvandervoord
Copy link
Member

Awesome! Thanks for walking me through it

@jlindgren90
Copy link
Contributor Author

Withdrawn in favor of #455.

@jlindgren90 jlindgren90 deleted the runner-api branch October 22, 2019 19:48
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