Re: [kvm-unit-tests PATCH v3 03/16] s390x: css: simplify skipping tests on no device

[Pierre Morel <pmorel@linux.ibm.com>]

On 4/6/21 2:44 PM, Cornelia Huck wrote:
> On Tue,  6 Apr 2021 09:40:40 +0200
> Pierre Morel <pmorel@linux.ibm.com> wrote:
> 
>> We will have to test if a device is present for every tests
>> in the future.
>> Let's provide separate the first tests from the test loop and
>> skip the remaining tests if no device is present.
> 
> What about the following patch description:
> 
> "We keep adding tests that act upon a concrete device, and we have to
> test that a device is present for all of those.
> 
> Instead, just skip all of the tests requiring a device if we were not
> able to set it up in the first place. The enumeration test will already
> have failed in that case."

ok yes better.

> 
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>>   s390x/css.c | 36 ++++++++++++++----------------------
>>   1 file changed, 14 insertions(+), 22 deletions(-)
>>
> 
> (...)
> 
>> @@ -336,8 +316,6 @@ static struct {
>>   	void (*func)(void);
>>   } tests[] = {
>>   	/* The css_init test is needed to initialize the CSS Characteristics */
> 
> If you remove the css_init test from this list, the above comment does
> not make sense anymore :)


grrr I thought I did remove this.
will do.

> 
>> -	{ "initialize CSS (chsc)", css_init },
>> -	{ "enumerate (stsch)", test_enumerate },
>>   	{ "enable (msch)", test_enable },
>>   	{ "sense (ssch/tsch)", test_sense },
>>   	{ "measurement block (schm)", test_schm },
>> @@ -352,11 +330,25 @@ int main(int argc, char *argv[])
>>   
>>   	report_prefix_push("Channel Subsystem");
>>   	enable_io_isc(0x80 >> IO_SCH_ISC);
>> +
>> +	report_prefix_push("initialize CSS (chsc)");
>> +	css_init();
>> +	report_prefix_pop();
>> +
>> +	report_prefix_push("enumerate (stsch)");
>> +	test_enumerate();
>> +	report_prefix_pop();
> 
> Could we maybe have two lists of tests: one that don't require a
> device, and one that does?

Yes, I can do that.

> 
>> +
>> +	if (!test_device_sid)
>> +		goto end;
> 
> In any case, I think we should log an explicit message that we skip the
> remaining tests because of no device being available.

OK, I will re-arrange the test order.

Thanks,
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen