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

["Radim Krčmář" <rkrcmar@redhat.com>]

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. :)