Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40538 )
Change subject: tests: Add lib/string-test test case ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/c/coreboot/+/40538/1/tests/include/mocks/assert.... File tests/include/mocks/assert.h:
https://review.coreboot.org/c/coreboot/+/40538/1/tests/include/mocks/assert.... PS1, Line 33: hlt(); \ I don't think you really need to mock out this header, I think it should be enough to mock out hlt().
https://review.coreboot.org/c/coreboot/+/40538/1/tests/include/mocks/assert.... PS1, Line 64: * called. Can you provide an example? I think dead_code() should be independent of unit tests (there is no need to write a test to "check" whether a dead_code() works, because it is already a build time check) but shouldn't cause problems for it either. When a dead_code() triggers that usually means that you're building with an invalid combination of Kconfig options. In that case I think it is correct that that fails to build even for unit tests -- we should run our tests with valid configurations.
https://review.coreboot.org/c/coreboot/+/40538/1/tests/lib/string-test.c File tests/lib/string-test.c:
https://review.coreboot.org/c/coreboot/+/40538/1/tests/lib/string-test.c@30 PS1, Line 30: #ifdef __RAMSTAGE__ The "which stage are we running in" concept is central to coreboot, so I think you should be adding that to the Makefile framework. I would suggest adding a new attribute $(test)-stage to decide that. It can default to ramstage when not provided (I think that makes most sense, it's generally our most feature-rich stage). Then it should automatically define the right macro like __RAMSTAGE__ to make sure the <rules.h> stuff works right.