Ceedling 1.0.1 does not resolve #include directives before mocking a header

[mikewzr]

I just tried out Ceedling 1.0.1 for my ESP IDF project and noticed some new linker errors, which weren't present with Ceedling 1.0.0.

👟 Building Test Executables
----------------------------
Linking test_ESP_CAN_default.out...
🧨 EXCEPTION: 'Default Test Linker' (gcc) terminated with exit code [1] and output >>
/usr/bin/ld: my_can.o: in function `init_gpios':
my_can.c:459:(.text+0x358): undefined reference to `esp_rom_gpio_connect_out_signal'
my_can.c:460:(.text+0x362): undefined reference to `esp_rom_gpio_pad_select_gpio'
my_can.c:467:(.text+0x38e): undefined reference to `esp_rom_gpio_connect_in_signal'
my_can.c:468:(.text+0x398): undefined reference to `esp_rom_gpio_pad_select_gpio'
collect2: error: ld returned 1 exit status
🌱 Ceedling could not complete operations because of errors

These functions where mocked before with Ceedling 1.0.0. I was investigating the issue a little bit by comparing the prepocessed header files and found out the following:

The preprocessed file gpio.h looks like this, after executing Ceedling 1.0.0:

// CEEDLING NOTICE: This generated file only to be consumed by CMock

#ifndef _GPIO_H_ // Ceedling-generated include guard
#define _GPIO_H_

#include "gpio_etm.h"

void esp_rom_gpio_pad_select_gpio(uint32_t iopad_num);

void esp_rom_gpio_pad_pullup_only(uint32_t iopad_num);

void esp_rom_gpio_pad_unhold(uint32_t gpio_num);
void esp_rom_gpio_pad_set_drv(uint32_t iopad_num, uint32_t drv);
void esp_rom_gpio_connect_in_signal(uint32_t gpio_num, uint32_t signal_idx, bool inv);
void esp_rom_gpio_connect_out_signal(uint32_t gpio_num, uint32_t signal_idx, bool out_inv, bool oen_inv);
typedef intr_handle_t gpio_isr_handle_t;

typedef void (*gpio_isr_t)(void *arg);

typedef struct {
    uint64_t pin_bit_mask;
    gpio_mode_t mode;
    gpio_pullup_t pull_up_en;
    gpio_pulldown_t pull_down_en;
    gpio_int_type_t intr_type;

} gpio_config_t;
esp_err_t gpio_config(const gpio_config_t *pGPIOConfig);
esp_err_t gpio_reset_pin(gpio_num_t gpio_num);
esp_err_t gpio_set_intr_type(gpio_num_t gpio_num, gpio_int_type_t intr_type);
esp_err_t gpio_intr_enable(gpio_num_t gpio_num);
esp_err_t gpio_intr_disable(gpio_num_t gpio_num);
esp_err_t gpio_set_level(gpio_num_t gpio_num, uint32_t level);
int gpio_get_level(gpio_num_t gpio_num);
esp_err_t gpio_set_direction(gpio_num_t gpio_num, gpio_mode_t mode);
esp_err_t gpio_set_pull_mode(gpio_num_t gpio_num, gpio_pull_mode_t pull);
esp_err_t gpio_wakeup_enable(gpio_num_t gpio_num, gpio_int_type_t intr_type);
esp_err_t gpio_wakeup_disable(gpio_num_t gpio_num);
esp_err_t gpio_isr_register(void (*fn)(void *), void *arg, int intr_alloc_flags, gpio_isr_handle_t *handle);
esp_err_t gpio_pullup_en(gpio_num_t gpio_num);
esp_err_t gpio_pullup_dis(gpio_num_t gpio_num);
esp_err_t gpio_pulldown_en(gpio_num_t gpio_num);
esp_err_t gpio_pulldown_dis(gpio_num_t gpio_num);
esp_err_t gpio_install_isr_service(int intr_alloc_flags);

void gpio_uninstall_isr_service(void);
esp_err_t gpio_isr_handler_add(gpio_num_t gpio_num, gpio_isr_t isr_handler, void *args);
esp_err_t gpio_isr_handler_remove(gpio_num_t gpio_num);
esp_err_t gpio_set_drive_capability(gpio_num_t gpio_num, gpio_drive_cap_t strength);
esp_err_t gpio_get_drive_capability(gpio_num_t gpio_num, gpio_drive_cap_t *strength);
esp_err_t gpio_hold_en(gpio_num_t gpio_num);
esp_err_t gpio_hold_dis(gpio_num_t gpio_num);
void gpio_deep_sleep_hold_en(void);

void gpio_deep_sleep_hold_dis(void);

void gpio_iomux_in(uint32_t gpio_num, uint32_t signal_idx);
void gpio_iomux_out(uint8_t gpio_num, int func, bool out_en_inv);
esp_err_t gpio_sleep_sel_en(gpio_num_t gpio_num);
esp_err_t gpio_sleep_sel_dis(gpio_num_t gpio_num);
esp_err_t gpio_sleep_set_direction(gpio_num_t gpio_num, gpio_mode_t mode);
esp_err_t gpio_sleep_set_pull_mode(gpio_num_t gpio_num, gpio_pull_mode_t pull);
esp_err_t gpio_dump_io_configuration(FILE *out_stream, uint64_t io_bit_mask);

#endif // _GPIO_H_

The same file (with the same project.yml) looks like this, when preprocessed by Ceedling 1.0.1:

// CEEDLING NOTICE: This generated file only to be consumed by CMock

#ifndef _GPIO_H_ // Ceedling-generated include guard
#define _GPIO_H_

#include "sdkconfig.h"
#include "esp_err.h"
#include "esp_intr_alloc.h"
#include "soc_caps.h"
#include "gpio_types.h"
#include "esp_rom_gpio.h"
#include "gpio_etm.h"

typedef intr_handle_t gpio_isr_handle_t;

typedef void (*gpio_isr_t)(void *arg);

typedef struct {
    uint64_t pin_bit_mask;
    gpio_mode_t mode;
    gpio_pullup_t pull_up_en;
    gpio_pulldown_t pull_down_en;
    gpio_int_type_t intr_type;

} gpio_config_t;
esp_err_t gpio_config(const gpio_config_t *pGPIOConfig);
esp_err_t gpio_reset_pin(gpio_num_t gpio_num);
esp_err_t gpio_set_intr_type(gpio_num_t gpio_num, gpio_int_type_t intr_type);
esp_err_t gpio_intr_enable(gpio_num_t gpio_num);
esp_err_t gpio_intr_disable(gpio_num_t gpio_num);
esp_err_t gpio_set_level(gpio_num_t gpio_num, uint32_t level);
int gpio_get_level(gpio_num_t gpio_num);
esp_err_t gpio_set_direction(gpio_num_t gpio_num, gpio_mode_t mode);
esp_err_t gpio_set_pull_mode(gpio_num_t gpio_num, gpio_pull_mode_t pull);
esp_err_t gpio_wakeup_enable(gpio_num_t gpio_num, gpio_int_type_t intr_type);
esp_err_t gpio_wakeup_disable(gpio_num_t gpio_num);
esp_err_t gpio_isr_register(void (*fn)(void *), void *arg, int intr_alloc_flags, gpio_isr_handle_t *handle);
esp_err_t gpio_pullup_en(gpio_num_t gpio_num);
esp_err_t gpio_pullup_dis(gpio_num_t gpio_num);
esp_err_t gpio_pulldown_en(gpio_num_t gpio_num);
esp_err_t gpio_pulldown_dis(gpio_num_t gpio_num);
esp_err_t gpio_install_isr_service(int intr_alloc_flags);

void gpio_uninstall_isr_service(void);
esp_err_t gpio_isr_handler_add(gpio_num_t gpio_num, gpio_isr_t isr_handler, void *args);
esp_err_t gpio_isr_handler_remove(gpio_num_t gpio_num);
esp_err_t gpio_set_drive_capability(gpio_num_t gpio_num, gpio_drive_cap_t strength);
esp_err_t gpio_get_drive_capability(gpio_num_t gpio_num, gpio_drive_cap_t *strength);
esp_err_t gpio_hold_en(gpio_num_t gpio_num);
esp_err_t gpio_hold_dis(gpio_num_t gpio_num);
void gpio_deep_sleep_hold_en(void);

void gpio_deep_sleep_hold_dis(void);

void gpio_iomux_in(uint32_t gpio_num, uint32_t signal_idx);
void gpio_iomux_out(uint8_t gpio_num, int func, bool out_en_inv);
esp_err_t gpio_sleep_sel_en(gpio_num_t gpio_num);
esp_err_t gpio_sleep_sel_dis(gpio_num_t gpio_num);
esp_err_t gpio_sleep_set_direction(gpio_num_t gpio_num, gpio_mode_t mode);
esp_err_t gpio_sleep_set_pull_mode(gpio_num_t gpio_num, gpio_pull_mode_t pull);
esp_err_t gpio_dump_io_configuration(FILE *out_stream, uint64_t io_bit_mask);

#endif // _GPIO_H_

It seems like Ceedling 1.0.1 does not process the include statements in the header-to-be-mocked anymore. The file I'm trying to mock is part of the ESP IDF and can be viewed here: https://github.com/espressif/esp-idf/blob/v5.3.1/components/esp_driver_gpio/include/driver/gpio.h.

[mvandervoord]

Hi @mikewzr -- sorry about the trouble and THANK YOU for the detailed report. You've provided a lot of good information for us to work with here!

I actually believe this is working the way it's intended, and you were just lucky enough that the problems in 1.0.0 were working to your advantage. I'll make an attempt to capture the design decisions here. I'd love to hear your thoughts if you have any.

In version 1.0.0, the preprocessing was accidentally pulling too much information from "deeper" or "nested" headers into headers directly included by the tests or mocks. This leads to problems at times with double definitions of the same information (for example, both a.h and b.h might include config.h. If config.h defines macros or variables or inlines... these become conflicts because they now exist in both places).

This bug also had the side effect that, in some cases, meant that functions defined in nested headers were also being mocked (whereas usually only the functions directly in the specified headers are mocked). At worst, this can lead to the double-inclusion problem above for generated mocks. Often it just generates a lot of mocks which aren't actually going to be used.

It sound like, in your situation, it wasn't leading to either of the problems above, and you desired to have those functions mocked anyway. So it all seemed purposeful.

The intended behavior is that a test is written with an explicit list of all the files which need mocking. This means that if you need mocks from a nested header file, you should be required to include that header file. In your situation, your test includes mock_gpio.h. If my_can.c directly makes calls to functions defined in esp_rom_gpio.h instead of working through the public api in gpio.h, then it also makes sense that it should have to include mock_esp_rom_pgio.h

Including all the headers required for your test fixes the problem and it returns the control of what is included to you, the test writer.

So that's the theory.

As with anything, there are tradeoffs abound. At its best, this behavior means more work for the test writer (which we don't love). At its worst, it is somehow challenging to create the perfect circumstances to include all the nested headers with their required defines, etc. It can get tedious hunting down what is needed.

This is why we often recommend building a single header file (esp_api.h for example) which contains the public API to the esp functions AS USED BY YOUR APPLICATION). It takes a little work to build up (though honestly most of it can be done as-you-go), but it has some huge advantages:

• It avoids all the preprocessing nightmares and time-consuming configuration details
• It serves as clear documentation for anyone working on this project of which functions / types / etc are being used in this project
• It makes your work more portable. If you upgrade to a new version of the library (espressif's in your example), and if the API changes, you have the choice of abstracting away just the changed components if needed.

Ok. I think I've rambled long enough now, though I'm happy to chat further about any of these things if need be. Best of luck!