From ce55ca2fcaab23010b2f7e310c921f65b037034d Mon Sep 17 00:00:00 2001 From: Patrick Georgi Date: Wed, 9 Jun 2021 18:37:42 +0200 Subject: [PATCH] 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 Reviewed-on: https://review.coreboot.org/c/coreboot/+/55360 Tested-by: build bot (Jenkins) Reviewed-by: Jakub Czapiga --- Documentation/tutorial/part3.md | 18 +++++++++--------- tests/Makefile.inc | 8 ++++++-- tests/device/i2c-test.c | 8 ++++---- tests/lib/coreboot_table-test.c | 2 +- tests/lib/imd_cbmem-test.c | 2 +- tests/lib/spd_cache-test.c | 6 +++--- 6 files changed, 24 insertions(+), 20 deletions(-) diff --git a/Documentation/tutorial/part3.md b/Documentation/tutorial/part3.md index 6d147ff9a9..7cdf7d9f21 100644 --- a/Documentation/tutorial/part3.md +++ b/Documentation/tutorial/part3.md @@ -274,8 +274,8 @@ without changing the source code. 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 --mocks variable in Makefile.inc. The result is that every time the -function is called, `wrap_name_of_function` will be called instead. +-mocks variable in Makefile.inc. The result is that the test's +implementation of that function is called instead of coreboot's. ```eval_rst .. 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 - Now, dev can write own implementation of `platform_i2c_transfer` and define it - as `wrap_platform_i2c_transfer`. This implementation instead of accessing real - i2c bus, will write/read from fake structs. + Now, dev can write own implementation of `platform_i2c_transfer`. This + implementation instead of accessing real i2c bus, will write/read from + fake structs. .. 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) { } @@ -324,13 +324,13 @@ in which mock should have particular value, or be inside a described range. I2C_M_RECV_LEN, I2C_M_NOSTART}; /* 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); - expect_not_value_count(__wrap_platform_i2c_transfer, segments->buf, + expect_not_value_count(platform_i2c_transfer, segments->buf, 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); } diff --git a/tests/Makefile.inc b/tests/Makefile.inc index 3473ef74d0..15ea670336 100644 --- a/tests/Makefile.inc +++ b/tests/Makefile.inc @@ -17,6 +17,7 @@ coverage_dir = coverage_reports CMOCKA_LIB := $(cmockaobj)/src/libcmocka.so CMAKE:= cmake +OBJCOPY?= objcopy TEST_DEFAULT_CONFIG = $(top)/configs/config.emulation_qemu_x86_i440fx TEST_DOTCONFIG = $(testobj)/.config @@ -114,18 +115,21 @@ $$($(1)-config-file): $(TEST_KCONFIG_AUTOHEADER) $($(1)-objs): TEST_CFLAGS += -I$$(dir $$($(1)-config-file)) \ -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) mkdir -p $$(dir $$@) $(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) $(HOSTCC) $$^ $($(1)-cflags) $$(TEST_LDFLAGS) -o $$@ endef $(foreach test, $(alltests), \ + $(eval $(test)-srcobjs:=$(addprefix $(testobj)/$(test)/, \ + $(patsubst %.c,%.o,$(filter src/%,$($(test)-srcs))))) \ $(eval $(test)-objs:=$(addprefix $(testobj)/$(test)/, \ $(patsubst %.c,%.o,$($(test)-srcs))))) $(foreach test, $(alltests), \ diff --git a/tests/device/i2c-test.c b/tests/device/i2c-test.c index b564e20435..2c9423052e 100644 --- a/tests/device/i2c-test.c +++ b/tests/device/i2c-test.c @@ -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 i; @@ -77,13 +77,13 @@ static void mock_expect_params_platform_i2c_transfer(void) I2C_M_RECV_LEN, I2C_M_NOSTART}; /* 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); - expect_not_value_count(__wrap_platform_i2c_transfer, segments->buf, + expect_not_value_count(platform_i2c_transfer, segments->buf, 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); } diff --git a/tests/lib/coreboot_table-test.c b/tests/lib/coreboot_table-test.c index cd82ee5c3a..ba3e63d84b 100644 --- a/tests/lib/coreboot_table-test.c +++ b/tests/lib/coreboot_table-test.c @@ -264,7 +264,7 @@ void cbmem_run_init_hooks(int is_recovery) } extern uintptr_t _cbmem_top_ptr; -void *__wrap_cbmem_top_chipset(void) +void *cbmem_top_chipset(void) { return (void *)_cbmem_top_ptr; } diff --git a/tests/lib/imd_cbmem-test.c b/tests/lib/imd_cbmem-test.c index ef90909a1a..f857579960 100644 --- a/tests/lib/imd_cbmem-test.c +++ b/tests/lib/imd_cbmem-test.c @@ -36,7 +36,7 @@ void cbmem_run_init_hooks(int is_recovery) function_called(); } -void *__wrap_cbmem_top_chipset(void) +void *cbmem_top_chipset(void) { return (void *)_cbmem_top_ptr; } diff --git a/tests/lib/spd_cache-test.c b/tests/lib/spd_cache-test.c index ef0f6ef5ac..4bc0a3dcfe 100644 --- a/tests/lib/spd_cache-test.c +++ b/tests/lib/spd_cache-test.c @@ -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); } /* This test verifies if load_spd_cache() correctly loads spd_cache pointer and size - from provided region_device. Memory region device is returned by - __wrap_fmap_locate_area_as_rdev() */ + from provided region_device. Memory region device is returned by our + fmap_locate_area_as_rdev() override. */ static void test_load_spd_cache(void **state) { uint8_t *spd_cache;