Attention is currently required from: Peter Marheine. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56911 )
Change subject: tests: add init_shutdown test for realtek_mst_i2c_spi ......................................................................
Patch Set 1:
(8 comments)
Patchset:
PS1: Thanks for writing a test!
File tests/init_shutdown.c:
https://review.coreboot.org/c/flashrom/+/56911/comment/8df145ce_db617b87 PS1, Line 302: #include <fcntl.h> : #include <linux/i2c-dev.h> Tests should be as lightweight and independent as possible, they only thing to check is CONFIG_REALTEK_MST_I2C_SPI == 1, and that's inside test method. Detect exactly what is the minimum set of symbols you need from these libraries (is it O_RDWR and I2C_SLAVE?) and define them in io_mock.h (there are some examples of that already). Remove these includes, remove check for IS_LINUX. Mock implementations (open,ioctl,read,write) should compile as is, you only check CONFIG_REALTEK_MST_I2C_SPI == 1 inside the test method. Follow the example of all other tests.
https://review.coreboot.org/c/flashrom/+/56911/comment/4f925a5e_348e9737 PS1, Line 364: io_mock_register(&realtek_mst_io); You need to unregister at the end, `io_mock_register(NULL);` (you can look at the other tests)
File tests/tests.c:
https://review.coreboot.org/c/flashrom/+/56911/comment/0bdaa2c5_1fdc3cc9 PS1, Line 93: if (current_io->open64) : return current_io->open64(current_io->state, pathname, flags); : if (current_io->open) : return current_io->open(current_io->state, pathname, flags); Why do you put both open and open64 into one wrap? There is a dedicated wrap for open, so `current_io->open` should go there.
https://review.coreboot.org/c/flashrom/+/56911/comment/7c4aa9ad_0703901f PS1, Line 120: return -1; Default behaviour for our wraps is "do nothing, pretend everything is successful". You need to return a value which emulates success. Is it sz for this write operation?
https://review.coreboot.org/c/flashrom/+/56911/comment/27a21a61_69f228be PS1, Line 127: return current_io->read(current_io->state, fd, buf, sz); We are following Linux kernel coding style https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docu... which says that indentations are 8 chars.
https://review.coreboot.org/c/flashrom/+/56911/comment/01ad558a_8a5fc193 PS1, Line 128: return -1; Same here, you need to return value which means success.
https://review.coreboot.org/c/flashrom/+/56911/comment/cb85340b_14ff6424 PS1, Line 345: cmocka_unit_test Same thing, indentations should be 8 chars