Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40539 )
Change subject: tests: Add device/i2c-test test case ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/coreboot/+/40539/3/tests/device/i2c-test.c File tests/device/i2c-test.c:
https://review.coreboot.org/c/coreboot/+/40539/3/tests/device/i2c-test.c@48 PS3, Line 48: int __wrap_platform_i2c_transfer(unsigned int bus, struct i2c_msg *segments, Can you put this mock into a separate C file? In general, we would like mocks to be available to multiple unit tests without having to copy them, so it would be good to show the reader how to do it and if there are any issues they need to be aware of.
https://review.coreboot.org/c/coreboot/+/40539/3/tests/device/i2c-test.c@85 PS3, Line 85: mock_check_params The name of this function is a little confusing. It isn't checking the parameters, it is setting expectations for the parameters (which will get checked later). Consider changing to mock_expect_params_i2c_transfer() or something like that?
BTW, "There are only two hard things in Computer Science: cache invalidation and naming things." -- Phil Karlton
https://review.coreboot.org/c/coreboot/+/40539/3/tests/device/i2c-test.c@96 PS3, Line 96: } I would also add an expectation that the 'count' parameter is 1 or more.