Re: [kvm-unit-tests PATCH v2 5/5] s390x: SCLP unit test

[David Hildenbrand <david@redhat.com>]

On 04.11.19 12:49, Claudio Imbrenda wrote:
> On Mon, 4 Nov 2019 12:31:32 +0100
> David Hildenbrand <david@redhat.com> wrote:
> 
>> On 04.11.19 12:29, Claudio Imbrenda wrote:
>>> On Mon, 4 Nov 2019 11:58:20 +0100
>>> David Hildenbrand <david@redhat.com> wrote:
>>>
>>> [...]
>>>    
>>>> Can we just please rename all "cx" into something like "len"? Or is
>>>> there a real need to have "cx" in there?
>>>
>>> if cx is such a nuisance to you, sure, I can rename it to i
>>
>> better than random characters :)
> 
> will be in v3
> 
>>>    
>>>> Also, I still dislike "test_one_sccb". Can't we just just do
>>>> something like
>>>>
>>>> expect_pgm_int();
>>>> rc = test_one_sccb(...)
>>>> report("whatever pgm", rc == WHATEVER);
>>>> report("whatever rc", lc->pgm_int_code == WHATEVER);
>>>>
>>>> In the callers to make these tests readable and cleanup
>>>> test_one_sccb(). I don't care if that produces more LOC as long as
>>>> I can actually read and understand the test cases.
>>>
>>> if you think that makes it more readable, ok I guess...
>>>
>>> consider that the output will be unreadable, though
>>>    
>>
>> I think his will turn out more readable.
> 
> two output lines per SCLP call? I  don't think so

To clarify, we don't always need two checks. E.g., I would like to see 
instead of

+static void test_sccb_too_short(void)
+{
+	int cx;
+
+	for (cx = 0; cx < 8; cx++)
+		if (!test_one_run(valid_code, pagebuf, cx, 8, PGM_BIT_SPEC, 0))
+			break;
+
+	report("SCCB too short", cx == 8);
+}

Something like

static void test_sccb_too_short(void)
{
	int i;

	for (i = 0; i < 8; i++) {
		expect_pgm_int();
		test_one_sccb(...); // or however that will be called
		check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
	}
}

If possible.

-- 

Thanks,

David / dhildenb