Name of user not set #1002873 has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40539 )
Change subject: tests: Add device/i2c-test test case ......................................................................
Patch Set 4:
(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,
OK, then I guess this one is OK for here. […]
This is important thing, thanks for bringing this up.
Yes, let's keep this flexible in the meaning that we shouldn't force anything on the dev. It's sometimes better to keep test really simple, than use "overcomplicated" mock.
That being said, I would say that sharing mocks should be a default, to avoid code duplication. I think that in future will come up with some implementation were mock will be shared. This will be valuable example. Resolving this comment for now.
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. […]
:)
Good point. I would like to avoid too long name "trains", but the only candidate which sounds good to me and is addressing your comment is mock_expect_params_platform_i2c_transfer().. Even though I don't like it, at least it is really telling what the function is doing.
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.
Done