Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow the combination of use_test_preprocessor and CMock's treat_inlines #728

Conversation

M-Bab
Copy link

@M-Bab M-Bab commented Jan 12, 2023

I expect this combination to be pretty common especially in embedded unit testing. Often the preprocessor is needed because the vendor headers alone are too complex and tricky to be parsed correctly without preprocessing (NXP, STM, ...). Not to mention the own developed header files.

On the other hand you want to mock the evil inline functions that try to directly manipulate non existing registers. Even if you fake-map the registers into the host runner RAM you still have no control if the inline function was called or not. You gain a more fine grained control over your tests if the inline functions are mocked.

Unfortunately the combination of preprocessor and treat_inlines creates a copy of the original header in the include path which is stripped of all macros (=preprocessed). When the following GCC build of unit test itself is in anyway dependent on these macros the build will fail. This MR allows the specific mechanism of removing the static/inline keywords for the header copy to operate on the original file instead of the preprocessed file. While other operations of CMock run on the preprocessed file so there is no change in behaviour there.

I am not particular happy about this pull request concerning the code quality itself. It was my first touch with ruby/rake ever and I am just glad I achieved the level "works for us". Probably the tests need to be fixed. Probably there is a better or complete different way to resolve this. I just wanted to provide this to give an idea how it works. But I am also ready and willing to improve it, if you have some suggestions.

For more information compare the issue #706

This PR is dependent on this PR: ThrowTheSwitch/CMock#421

This allows to CMock to handle inlines in the original header
without writing a ruined header to the include path.

If preprocessing is disabled the mock and orig path are the same.

Updated submodule vendor/cmock to the matching branch.
@M-Bab
Copy link
Author

M-Bab commented Feb 14, 2023

@mvandervoord any idea how to go on with this? Hints to fix the tests or should we go for an entirely different approach? Currently we use gem specific_install to install our fork of ceedling but I would be very happy, if we can switch back to the original ceedling.

@mvandervoord
Copy link
Member

I'd like to get it rolled into the next release (see PR for 0.32.0 in progress). I haven't dug into why the self-tests were failing yet, but the conflicts with main suggest it's going to be far more challenging to merge to the pre-release branch. It might take some doing. ;)

@mvandervoord
Copy link
Member

Am I understanding correctly that your preprocessor is removing inline functions when it preprocesses?

@M-Bab
Copy link
Author

M-Bab commented Feb 14, 2023

Am I understanding correctly that your preprocessor is removing inline functions when it preprocesses?

It is a bit tricky: If you enable treat_inlines the module will create duplicates of the original header with the static keyword removed. But if use_test_preprocessor is also enabled the preprocessor will strip all macros (which is his actual job) - so the treat_inlines feature will create header duplicates which are missing all the macros. As a result, building the test fails because the macros are missing.

The target of my 2 MRs is, that the special feature of treat_inlines gets served the original header files (and not the preprocessed ones). This is legit because the header duplicates are still preprocessed as usual when the test is build.

@mvandervoord
Copy link
Member

I see. Thanks for the clarification. I'm understanding now. :)

…allow-parallel-use-preprocessor-and-include-inlines
@mkarlesky
Copy link
Member

I am happy to report that the latest prerelease of Ceedling 1.0.0 (formerly 0.32) fixes the problem documented in this PR. Ceedling's much improved preprocessing and CMock's :treat_inlines now work together as they should. In fact, this has been true for quite a while. The revamp of Ceedling's preprocessing had simply fixed the problem, but a couple small bugs were getting in the way.

As this PR is for version 0.31.1 that will soon lose active support and the latest version of Ceedling fixes the problem, I am going to close this PR. Thank you so much for helping us understand the problem!

@mkarlesky mkarlesky closed this Jul 16, 2024
@M-Bab
Copy link
Author

M-Bab commented Jul 18, 2024

Thats fantastic news! Thanks to you @mkarlesky and your team 😄

I am looking forward testing the 1.0.0 candidate in the next couple of weeks and am glad that our workaround is not necessary anymore. Since this MR was closed you most likely also want to close the corresponding MR in CMock: ThrowTheSwitch/CMock#421

@M-Bab
Copy link
Author

M-Bab commented Oct 8, 2024

Unfortunately the issue continues to exist according to the description with the latest ceedling version. Compare comment #868 (comment)

@mkarlesky
Copy link
Member

@M-Bab I'm sorry this is so complex and not yet meeting your expectations.

  1. Can you please provide a sample header file and what you are hoping for the result of mocking it to be?
  2. Can you also explain where the combination of :use_test_preprocessor: :tests with CMock’s various features (no test preprocessor enabled for mocking at all) has a gap for you? I was under the impression CMock's features themselves were sufficient for your circumstances but test preprocessing was clobbering the content of your mockable header files before CMock processed them. I had thought the option to use the preprocessor on test files alone without touching mockable header files achieved your goal.

@M-Bab
Copy link
Author

M-Bab commented Oct 15, 2024

Thanks for your response. I immediately tried it out because for a brief moment :use_test_preprocessor: :tests sounded exactly like what we need. I needed quite a while to understand it is not. We do want to use full preprocessing for mocks and tests (I assume most users want this).

What we do not want is that the header copy of the original created by the treat_inlines feature is preprocessed. This copy (also placed in build/test/mocks) should definitely only strip the inline keywords. It is the first header in the search path and if it is filtered through the preprocessor all the macros are missing.

Lets make a more specific example with one of our most simple projects: Here we got the original config file and 2 example header files that are not properly mocked with :use_test_preprocessor: :tests. They are both from the NXP HAL.

Examples-not-working.zip

What fails here and why?

  • fsl_common.h: The mock extractor gets easily overwhelmed by the complex macros and generates even invalid C-code. If you look at the mock for EnableIRQ somewhere in between a diag_error can be found. And this is originally a within the #if (defined(__ICCARM__)) part.
  • fsl_adc.h: A rather classic trap for an automatic mock generator: adc_hardware_trigger_mask_mode_t exists only if certain defines are given which is not the case here. The mock generator still extracts static inline void ADC_SetHardwareTriggerMaskMode(ADC_Type *base, adc_hardware_trigger_mask_mode_t mode) (not knowing it is disabled by macro-ifs). The resulting build will fail because the enum is unknown.

Now a comparison with feature of my branches and test_processor and treat_inlines enabled: The copy headers in build/test/mocks are stripped from the inline keyword but otherwise unchanged. So all the macros are kept and the inline functions can be properly found and linked to the macros. From these copies syntactically correct mock-files are generated. The following zip-file contains the generated headers (stripped from inline) and the mocks.

Example-working.zip

I am still convinced that the current behavior of vanilla ceedling with treat_inlines and preprocessor is a bug. Just follow the data stream from the treat_inline Feature: If it was passed through the preprocessor before the result will be wrong. Please don't hesitate to ask if anything is unclear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants