Attention is currently required from: Julius Werner, Jan Dabros. Jakub Czapiga 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:
(6 comments)
File tests/include/tests/lib/cbfs_util.h:
https://review.coreboot.org/c/coreboot/+/56601/comment/9a83fbbe_46a0db9e PS8, Line 24: 90
Rather than having to check these by hand, I believe […]
It does work, and indeed, this is safer than hardcoded length.
https://review.coreboot.org/c/coreboot/+/56601/comment/f14992db_e2715804 PS8, Line 74: TEST_REGION(cbfs_cache, TEST_CBFS_CACHE_SIZE); It is almost the same as for test data files. Test data file should be included only once per test (executable). However, test data is constant, and cbfs_cache is going to be written to, so I see, why you want to move cbfs_cache to its own compilation unit.
all the corresponding struct cbfs_test_file globals for this
Sorry, but I do not understand, what you mean. Do you want me to move `TEST_REGION(cbfs_cache, TEST_CBFS_CACHE_SIZE);` to the another file and also define test files in this mock? Defining test files in case of CBFS test does not make sense in my opinion. But if you meant to define arrays with test data for comparison with data returned by cbfs_map() etc, then it does make sense.
File tests/lib/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/56601/comment/5a914694_b74c5583 PS2, Line 199: CONFIG_NO_CBFS_MCACHE=0 \
Well I think file verification and metadata verification are certainly different enough things that […]
Done
File tests/lib/cbfs-verification-test.c:
https://review.coreboot.org/c/coreboot/+/56601/comment/fa939a29_acad2d1d PS2, Line 181: assert_memory_equal(mapping, &test_data, TEST_DATA_SIZE);
That's not how this works -- CBFS cache is used by the specific rdev's mmap() implementation, not by […]
Done
https://review.coreboot.org/c/coreboot/+/56601/comment/e6b0f8a0_42d24831 PS2, Line 243: /* No cache, so no validation required */
Not really sure what these comments mean. […]
Done
File tests/lib/cbfs-verification-test.c:
https://review.coreboot.org/c/coreboot/+/56601/comment/24d875e9_0c8b2145 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 […]
How about adding TEST_NAME define and using it instead? And we could redefine cmocka_run_group_tests to use TEST_NAME (or default #group_tests if TEST_NAME is empty), and that would solve this problem, because each test in Makefile has to have a different name.