Re: [kvm-unit-tests PATCH v1 2/6] s390x: basic self test

Radim Krčmář
2017-05-17T13:12:29Z

2017-05-16 21:16+0200, David Hildenbrand:
> On 16.05.2017 15:35, Radim Krčmář wrote:
> > 2017-05-12 12:58+0200, David Hildenbrand:
> >> Test if the general infrastructure is working. The test will fail until
> >> we have proper sclp console output.
> >>
> >> Signed-off-by: David Hildenbrand <david@redhat.com>
> >> ---
> >> diff --git a/s390x/selftest.c b/s390x/selftest.c
> >> @@ -0,0 +1,31 @@
> >> +int main(int argc, char**argv)
> >> +{
> >> + report_prefix_push("selftest");
> >> +
> >> + if (argc != 3)
> >> + report_abort("Wrong number of arguments");
> >> +
> >> + if (strcmp(argv[0], "s390x/selftest.elf") != 0)
> >> + report_abort("wrong program name");
> >
> > This is going to fail when executed as a standalone test (argv[0] would
> > be a temp file name). No point in checking, IMO.
>
> That name is determined during compile time, so it shouldn't change, or
> am i wrong? But if you prefer, I can drop it.

You are right, only x86 uses the actual kernel file name, sorry.

>>> + if (strcmp(argv[1], "test") != 0)
>>> + report_abort("wrong parameter value");
>>> + if (strcmp(argv[2], "123") != 0)
>>> + report_abort("wrong parameter value");
>>> +
>>> + report("test true", true, 0);
>> ^
>> It seems you'll be doing v2 -- please remove this zero in it.
>
> Thanks, if that would throw an error during compile time like printf, it
> would be perfect.
>
> Yes, I'll resend!

Thinking more about it, the test would look better if we used report()
instead of if()+report_abort() and a final report, something like:

report("argv[0] == PROGNAME", !strcmp(argv[0], "s390x/selftest.elf"));
report("argv[1] == test", !strcmp(argv[1], "test"));
report("argv[2] == 123", !strcmp(argv[2], "123"));

This is also a nitpick and I'll gladly accept the original version. :)