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.
In fact, I have found bugs in some implementations of SCLP exactly
because I did not test only the boundary conditions.
