Attention is currently required from: Jakub Czapiga, Jan Dabros. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56601 )
Change subject: tests: Add lib/cbfs-verification-test test case ......................................................................
Patch Set 8:
(5 comments)
Patchset:
PS8: Please reply to / resolve the old outstanding comments at some point so it's clearer to see what you're still planning to change.
File tests/include/tests/lib/cbfs_util.h:
https://review.coreboot.org/c/coreboot/+/56601/comment/b0f86c62_46ef2be3 PS8, Line 24: 90 Rather than having to check these by hand, I believe
sizeof((u8[]){TEST_DATA_1})
would also work to determine it programmatically.
https://review.coreboot.org/c/coreboot/+/56601/comment/b969aae7_64e09f97 PS8, Line 74: TEST_REGION(cbfs_cache, TEST_CBFS_CACHE_SIZE); This defines a global variable, so I think it should go in the .c file, otherwise you have weirdness if you link two compilation units that both included this header (even though that seems unlikely in this test framework, but probably not impossible).
Maybe create a tests/mock/cbfs_util.c to go with this that defines this and all the corresponding struct cbfs_test_file globals for this? (I'd maybe call it cbfs_mock or cbfs_file_mock instead in that case...)
File tests/lib/cbfs-verification-test.c:
https://review.coreboot.org/c/coreboot/+/56601/comment/761e8234_4d0eb46e PS2, Line 4: #define __noreturn
`fail()` in CMocka calls either `exit()`, `abort()` or `siglongjmp()`. […]
Resolved?
File tests/lib/cbfs-verification-test.c:
https://review.coreboot.org/c/coreboot/+/56601/comment/aedd859b_b7f5988b PS8, Line 244: return cmocka_run_group_tests(cbfs_verification_tests, NULL, NULL); One problem with this copying approach I noticed is that when you run the test, it's hard to figure out which output belongs to which variant... they all just say "cbfs_verification_tests: Running 4 test(s).". I believe you could use cmocka_run_group_tests_name() here instead to make them distinguishable.
(Of course, you need to somehow get the distinguishing name you want to print first. And if we're going to do this copying a lot, that may be a recurring problem that would be better solved a single time for all tests. Maybe you could design a fancy macro like `cmocka_run_test_variations(tests, setup, teardown, ...)` that you would call here as `cmocka_run_test_variations(cbfs_verification_tests, NULL, NULL, CONFIG_CBFS_VERIFICATION, VB2_SUPPORT_SHA512);` and it would translate that into `cmocka_run_group_tests_name("cbfs_verification_tests(CONFIG_CBFS_VERIFICATION=1, VB2_SUPPORT_SHA512=0)", cbfs_verification_tests, NULL, NULL);`, with the numbers matching the configuration for this particular instance of the test.)