On 04.11.19 15:24, Claudio Imbrenda wrote:
On Mon, 4 Nov 2019 14:47:54 +0100
David Hildenbrand <david@xxxxxxxxxx> wrote:
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.
the fact that the other test are not as extensive as they should be
doesn't mean this test should cover less.
All I'm saying is that you might test too much and I am not sure if that
is really needed everywhere in this patch. But I'll leave that up to you.
--
Thanks,
David / dhildenb
