tests: Rework mocking facility

Using the linker's --wrap feature has the downside that it only covers
references across object files: If foo.c defines a() and b(), with b
calling a, --wrap=a does nothing to that call.

Instead, use objcopy to mark a weak and global so it can be overridden
by another implementation, but only for files originating in src/.

That way mocks - implemented in tests/ - become the source of truth.

TEST=Had such an issue with get_log_level() in a follow-up commit, and
the mock now takes over. Also, all existing unit tests still pass.

Change-Id: I99c6d6e44ecfc73366bf464d9c51c7da3f8db388
Signed-off-by: Patrick Georgi <pgeorgi@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/55360
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Jakub Czapiga <jacz@semihalf.com>
This commit is contained in:
Patrick Georgi 2021-06-09 18:37:42 +02:00
parent 36c90179f0
commit ce55ca2fca
6 changed files with 24 additions and 20 deletions

View File

@ -274,8 +274,8 @@ without changing the source code.
coreboot unit test infrastructure supports overriding of functions at link time. coreboot unit test infrastructure supports overriding of functions at link time.
This is as simple as adding a `name_of_function` to be mocked into This is as simple as adding a `name_of_function` to be mocked into
<test_name>-mocks variable in Makefile.inc. The result is that every time the <test_name>-mocks variable in Makefile.inc. The result is that the test's
function is called, `wrap_name_of_function` will be called instead. implementation of that function is called instead of coreboot's.
```eval_rst ```eval_rst
.. admonition:: i2c-test example .. admonition:: i2c-test example
@ -284,13 +284,13 @@ function is called, `wrap_name_of_function` will be called instead.
i2c-test-mocks += platform_i2c_transfer i2c-test-mocks += platform_i2c_transfer
Now, dev can write own implementation of `platform_i2c_transfer` and define it Now, dev can write own implementation of `platform_i2c_transfer`. This
as `wrap_platform_i2c_transfer`. This implementation instead of accessing real implementation instead of accessing real i2c bus, will write/read from
i2c bus, will write/read from fake structs. fake structs.
.. code-block:: c .. code-block:: c
int __wrap_platform_i2c_transfer(unsigned int bus, struct i2c_msg *segments, int platform_i2c_transfer(unsigned int bus, struct i2c_msg *segments,
int count) int count)
{ {
} }
@ -324,13 +324,13 @@ in which mock should have particular value, or be inside a described range.
I2C_M_RECV_LEN, I2C_M_NOSTART}; I2C_M_RECV_LEN, I2C_M_NOSTART};
/* Flags should always be only within supported range */ /* Flags should always be only within supported range */
expect_in_set_count(__wrap_platform_i2c_transfer, segments->flags, expect_in_set_count(platform_i2c_transfer, segments->flags,
expected_flags, -1); expected_flags, -1);
expect_not_value_count(__wrap_platform_i2c_transfer, segments->buf, expect_not_value_count(platform_i2c_transfer, segments->buf,
NULL, -1); NULL, -1);
expect_in_range_count(__wrap_platform_i2c_transfer, count, 1, INT_MAX, expect_in_range_count(platform_i2c_transfer, count, 1, INT_MAX,
-1); -1);
} }

View File

@ -17,6 +17,7 @@ coverage_dir = coverage_reports
CMOCKA_LIB := $(cmockaobj)/src/libcmocka.so CMOCKA_LIB := $(cmockaobj)/src/libcmocka.so
CMAKE:= cmake CMAKE:= cmake
OBJCOPY?= objcopy
TEST_DEFAULT_CONFIG = $(top)/configs/config.emulation_qemu_x86_i440fx TEST_DEFAULT_CONFIG = $(top)/configs/config.emulation_qemu_x86_i440fx
TEST_DOTCONFIG = $(testobj)/.config TEST_DOTCONFIG = $(testobj)/.config
@ -114,18 +115,21 @@ $$($(1)-config-file): $(TEST_KCONFIG_AUTOHEADER)
$($(1)-objs): TEST_CFLAGS += -I$$(dir $$($(1)-config-file)) \ $($(1)-objs): TEST_CFLAGS += -I$$(dir $$($(1)-config-file)) \
-D__$$(shell echo $$($(1)-stage) | tr '[:lower:]' '[:upper:]')__ -D__$$(shell echo $$($(1)-stage) | tr '[:lower:]' '[:upper:]')__
$($(1)-srcobjs): OBJCOPY_FLAGS += $$(foreach mock,$$($(1)-mocks),--globalize-symbol=$$(mock) --weaken-symbol=$$(mock))
$($(1)-objs): $(testobj)/$(1)/%.o: $$$$*.c $$($(1)-config-file) $($(1)-objs): $(testobj)/$(1)/%.o: $$$$*.c $$($(1)-config-file)
mkdir -p $$(dir $$@) mkdir -p $$(dir $$@)
$(HOSTCC) $(HOSTCFLAGS) $$(TEST_CFLAGS) $($(1)-cflags) -MMD \ $(HOSTCC) $(HOSTCFLAGS) $$(TEST_CFLAGS) $($(1)-cflags) -MMD \
-MT $$@ -c $$< -o $$@ -MT $$@ -c $$< -o $$@.orig
$(OBJCOPY) $$@.orig $$(OBJCOPY_FLAGS) $$@
$($(1)-bin): TEST_LDFLAGS+= $$(foreach mock,$$($(1)-mocks),-Wl,--wrap=$$(mock))
$($(1)-bin): $($(1)-objs) $(CMOCKA_LIB) $($(1)-bin): $($(1)-objs) $(CMOCKA_LIB)
$(HOSTCC) $$^ $($(1)-cflags) $$(TEST_LDFLAGS) -o $$@ $(HOSTCC) $$^ $($(1)-cflags) $$(TEST_LDFLAGS) -o $$@
endef endef
$(foreach test, $(alltests), \ $(foreach test, $(alltests), \
$(eval $(test)-srcobjs:=$(addprefix $(testobj)/$(test)/, \
$(patsubst %.c,%.o,$(filter src/%,$($(test)-srcs))))) \
$(eval $(test)-objs:=$(addprefix $(testobj)/$(test)/, \ $(eval $(test)-objs:=$(addprefix $(testobj)/$(test)/, \
$(patsubst %.c,%.o,$($(test)-srcs))))) $(patsubst %.c,%.o,$($(test)-srcs)))))
$(foreach test, $(alltests), \ $(foreach test, $(alltests), \

View File

@ -30,7 +30,7 @@ i2c_ex_devs_t i2c_ex_devs[] = {
} }, } },
}; };
int __wrap_platform_i2c_transfer(unsigned int bus, struct i2c_msg *segments, int platform_i2c_transfer(unsigned int bus, struct i2c_msg *segments,
int count) int count)
{ {
int i; int i;
@ -77,13 +77,13 @@ static void mock_expect_params_platform_i2c_transfer(void)
I2C_M_RECV_LEN, I2C_M_NOSTART}; I2C_M_RECV_LEN, I2C_M_NOSTART};
/* Flags should always be only within supported range */ /* Flags should always be only within supported range */
expect_in_set_count(__wrap_platform_i2c_transfer, segments->flags, expect_in_set_count(platform_i2c_transfer, segments->flags,
expected_flags, -1); expected_flags, -1);
expect_not_value_count(__wrap_platform_i2c_transfer, segments->buf, expect_not_value_count(platform_i2c_transfer, segments->buf,
NULL, -1); NULL, -1);
expect_in_range_count(__wrap_platform_i2c_transfer, count, 1, INT_MAX, expect_in_range_count(platform_i2c_transfer, count, 1, INT_MAX,
-1); -1);
} }

View File

@ -264,7 +264,7 @@ void cbmem_run_init_hooks(int is_recovery)
} }
extern uintptr_t _cbmem_top_ptr; extern uintptr_t _cbmem_top_ptr;
void *__wrap_cbmem_top_chipset(void) void *cbmem_top_chipset(void)
{ {
return (void *)_cbmem_top_ptr; return (void *)_cbmem_top_ptr;
} }

View File

@ -36,7 +36,7 @@ void cbmem_run_init_hooks(int is_recovery)
function_called(); function_called();
} }
void *__wrap_cbmem_top_chipset(void) void *cbmem_top_chipset(void)
{ {
return (void *)_cbmem_top_ptr; return (void *)_cbmem_top_ptr;
} }

View File

@ -42,14 +42,14 @@ static int teardown_spd_cache(void **state)
} }
int __wrap_fmap_locate_area_as_rdev(const char *name, struct region_device *area) int fmap_locate_area_as_rdev(const char *name, struct region_device *area)
{ {
return rdev_chain(area, &flash_rdev_rw, 0, flash_buffer_size); return rdev_chain(area, &flash_rdev_rw, 0, flash_buffer_size);
} }
/* This test verifies if load_spd_cache() correctly loads spd_cache pointer and size /* This test verifies if load_spd_cache() correctly loads spd_cache pointer and size
from provided region_device. Memory region device is returned by from provided region_device. Memory region device is returned by our
__wrap_fmap_locate_area_as_rdev() */ fmap_locate_area_as_rdev() override. */
static void test_load_spd_cache(void **state) static void test_load_spd_cache(void **state)
{ {
uint8_t *spd_cache; uint8_t *spd_cache;