Attention is currently required from: Nico Huber, Angel Pons, Sergii Dmytruk. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59075 )
Change subject: [RFC][WPTST] tests: test write protection ......................................................................
Patch Set 7:
(2 comments)
Patchset:
PS7: Looks better, thanks for your work! I will have another look later, sorry for delays!
File tests/chip_wp.c:
https://review.coreboot.org/c/flashrom/+/59075/comment/dd7edba5_91ef6a90 PS7, Line 177: /* Check initial mode. */ : assert_int_equal(0, flashrom_wp_get_mode(&cfg, &mode)); : assert_int_equal(WP_MODE_DISABLED, mode); : : /* Switch modes in dummyflasher. */ : assert_int_equal(0, flashrom_wp_set_mode(&cfg, WP_MODE_PERMANENT)); : assert_int_equal(0, flashrom_wp_write_chip_config(&flash, &cfg)); : assert_int_equal(0, flashrom_wp_set_mode(&cfg, WP_MODE_POWER_CYCLE)); : assert_int_equal(0, flashrom_wp_write_chip_config(&flash, &cfg)); : : /* Final mode should be "permanent". */ : assert_int_equal(0, flashrom_wp_read_chip_config(&flash, &cfg)); : assert_int_equal(0, flashrom_wp_get_mode(&cfg, &mode)); : assert_int_equal(WP_MODE_PERMANENT, mode); What I was trying to understand last time:
1) Does this sequence of operations (lines 175 to 190 in current patchset) represent an atomic test scenario, so that all these operations has to be performed, and exactly in this order?
2) Or, is this test method covering more than one test scenario? For example, test sets PERMANENT mode, and then immediately, without checking current mode, sets POWER_CYCLE. Why do you need to switch so many times?
If 1: then it's better to add a comment describing the scenario
If 2: then you need to split into separate tests
I don't know whether it is 1 or 2, please help me understand. Maybe the goal of the test is to switch modes 10 times? but then you would need to check mode after every switch?