Attention is currently required from: Nico Huber, Angel Pons, Sergii Dmytruk.

View Change

2 comments:

• Patchset:

  • Patch Set #7:

    Looks better, thanks for your work! I will have another look later, sorry for delays!

• File tests/chip_wp.c:

  • Patch Set #7, 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?

To view, visit change 59075. To unsubscribe, or for help writing mail filters, visit settings.