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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04.11.19 13:06, Claudio Imbrenda wrote:
On Mon, 4 Nov 2019 12:55:48 +0100
David Hildenbrand <david@xxxxxxxxxx> wrote:

On 04.11.19 12:49, Claudio Imbrenda wrote:
On Mon, 4 Nov 2019 12:31:32 +0100
David Hildenbrand <david@xxxxxxxxxx> wrote:
On 04.11.19 12:29, Claudio Imbrenda wrote:
On Mon, 4 Nov 2019 11:58:20 +0100
David Hildenbrand <david@xxxxxxxxxx> 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.


so, thousands of output lines for the whole test, ok


A couple of things to note

a) You perform 8 checks, so report the result of 8 checks
b) We really don't care about the number of lines in a log file as long as we can roughly identify what went wrong (e.g., push/pop a prefix here) c) We really *don't* need full coverage here. The same applies to other tests. Simply testing against the boundary conditions is good enough.


expect_pgm_int();
test_one_sccb(..., 0, ...);
check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);

expect_pgm_int();
test_one_sccb(..., 7, ...);
check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);

Just as we handle it in other tests.

--

Thanks,

David / dhildenb





[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux