Attention is currently required from: Aarya, Edward O'Callaghan, Nikolai Artemiev, Simon Buhrow, Thomas Heijligen.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67535?usp=email )
Change subject: tests: Unit tests for erase function selection algo ......................................................................
Patch Set 14:
(3 comments)
File tests/erase_func_algo.c:
https://review.coreboot.org/c/flashrom/+/67535/comment/d9f30ef6_29ab7dad : PS3, Line 238: void **state
I will check cmocka docs how to do it, keep this unresolved for now.
I found something in cmocka docs, around this page https://api.cmocka.org/group__cmocka__exec.html#gaaccacc105038e49462888a3ede...
They way to set this state is to implement setup function, but also it needs to be passed to cmocka_unit_test_X creation function which we have in tests.c. But that means setup needs to be visible in tests.c, which means introducing headers for setup function? Isn't it leaking internal test details to tests' main function?
Another thing is that I wanted to move g_state to its separate file at some point later (probably not in the first version of the test) so that it can grow large. I don't mind adding twice more meaningful test cases to the list, but this better be in the separate file.
With all that, do you think we can keep it as is for now?
https://review.coreboot.org/c/flashrom/+/67535/comment/069093f7_cbb4f4fe : PS3, Line 276: void **state
ditto.
other comment has the question
File tests/erase_func_algo.c:
https://review.coreboot.org/c/flashrom/+/67535/comment/551874f9_bb4375ee : PS5, Line 183: )
We can reduce global state by passing the test case as an argument here, i.e. […]
Done
-
Anastasia Klimchuk (Code Review)