Attention is currently required from: Felix Singer, Thomas Heijligen, Evan Benn.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69620 )
Change subject: tests: Add selfcheck to unit tests ......................................................................
Patch Set 7:
(15 comments)
File tests/selfcheck.c:
https://review.coreboot.org/c/flashrom/+/69620/comment/d6d7adef_7cdced17 PS7, Line 40: programmer row programmer entry
https://review.coreboot.org/c/flashrom/+/69620/comment/ba1faf3e_3c1c2f79 PS7, Line 68: assert_table(chip->bustype != BUS_NONE, "chip bustype is BUS_NONE", i, : chip->name); make one line
https://review.coreboot.org/c/flashrom/+/69620/comment/01f43ced_4c329c3f PS7, Line 74: { You can add `(void)state; /* unused */` in the beginning here. And don't forget new line after :)
https://review.coreboot.org/c/flashrom/+/69620/comment/a4882238_416078f2 PS7, Line 87: /* Blocks with zero size are bugs in flashchips.c. */ : /* Blocks with zero size are bugs in flashchips.c. */ Same line repeats twice?
https://review.coreboot.org/c/flashrom/+/69620/comment/a4e9c405_ed9b7c65 PS7, Line 89: if (eraser.eraseblocks[i].count : && !eraser.eraseblocks[i].size) { make if condition on one line
https://review.coreboot.org/c/flashrom/+/69620/comment/82a7942d_3a554945 PS7, Line 91: fail_msg( : "Flash chip %s erase function " : "%zu region %zu has size 0", make one line (out of 91-93)
https://review.coreboot.org/c/flashrom/+/69620/comment/7b901ec3_fce91ab2 PS7, Line 97: if (!eraser.eraseblocks[i].count : && eraser.eraseblocks[i].size) { make if condition on one line
https://review.coreboot.org/c/flashrom/+/69620/comment/2db58e2e_deba01a6 PS7, Line 99: fail_msg( : "Flash chip %s erase function " : "%zu region %zu has count 0", make one line (out of 99-101)
https://review.coreboot.org/c/flashrom/+/69620/comment/2858275b_5457302b PS7, Line 135: remove empty new line
https://review.coreboot.org/c/flashrom/+/69620/comment/644ebc9e_bc2215fa PS7, Line 136: fail_msg( : "Flash chip %s erase function " : "%zu and %zu are identical.", make one line (out of 136-138)
https://review.coreboot.org/c/flashrom/+/69620/comment/0b4f067d_59c49825 PS7, Line 143: fail_msg( : "Flash chip %s erase function %zu is not " : "in order", make one line (out of 143-145)
https://review.coreboot.org/c/flashrom/+/69620/comment/0721dbe9_6cd0eb7c PS7, Line 170: fail_msg("Board enable for %s %s is misdefined.\n", b->vendor_name, : b->board_name); make one line
File tests/tests.h:
https://review.coreboot.org/c/flashrom/+/69620/comment/bdad05c0_58858fc3 PS7, Line 90: _tables The file name is `selfcheck.c` without `_tables` suffix
File tests/well_formed_tables.c:
https://review.coreboot.org/c/flashrom/+/69620/comment/a9e6ae8b_7edd2282 PS6, Line 24: do { \ : if (!(assertion)) \ : fail_msg(#assertion " is false for index:%zu name:%s", (index), \ : (name) ? (name) : "unknown"); \ : } while (0)
I agree that would be the preferred style but a macro is used so that the __LINE__ format used by fa […]
Having correct __LINE__ is important, helps a lot with debugging tests. So I am happy to keep as it is. Felix would you agree?
https://review.coreboot.org/c/flashrom/+/69620/comment/12ce4e84_8a724dc2 PS6, Line 83: SKIP_TEST
Done
not done :P
-
Anastasia Klimchuk (Code Review)