[kvm-unit-tests,v1,2/6] s390x: basic self test
diff mbox

Message ID 20170512105830.10604-3-david@redhat.com
State New
Headers show

Commit Message

David Hildenbrand May 12, 2017, 10:58 a.m. UTC
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>
---
 s390x/Makefile      |  2 ++
 s390x/selftest.c    | 31 +++++++++++++++++++++++++++++++
 s390x/unittests.cfg |  5 +++++
 3 files changed, 38 insertions(+)
 create mode 100644 s390x/selftest.c

Comments

Radim Krčmář May 16, 2017, 1:35 p.m. UTC | #1
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.

> +	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.

(I'll prepare patch that check format of report in order to have
 automatic notifications in the future. :])

Thanks.
David Hildenbrand May 16, 2017, 7:16 p.m. UTC | #2
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.

> 
>> +	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!

> 
> (I'll prepare patch that check format of report in order to have
>  automatic notifications in the future. :])
> 
> Thanks.
>
Radim Krčmář May 17, 2017, 1:12 p.m. UTC | #3
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. :)
David Hildenbrand May 17, 2017, 4:50 p.m. UTC | #4
>>>> +	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. :)
> 

Thanks - I'll add that change and send a new version out tomorrow.

Patch
diff mbox

diff --git a/s390x/Makefile b/s390x/Makefile
index f9468bb..549c1c0 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -1,3 +1,5 @@ 
+tests = $(TEST_DIR)/selftest.elf
+
 all: test_cases
 
 test_cases: $(tests)
diff --git a/s390x/selftest.c b/s390x/selftest.c
new file mode 100644
index 0000000..827ef60
--- /dev/null
+++ b/s390x/selftest.c
@@ -0,0 +1,31 @@ 
+/*
+ * Copyright (c) 2017 Red Hat Inc
+ *
+ * Authors:
+ *  Thomas Huth <thuth@redhat.com>
+ *  David Hildenbrand <david@redhat.com>
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Library General Public License version 2.
+ */
+#include <libcflat.h>
+#include <util.h>
+
+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");
+	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);
+
+	return report_summary();
+}
diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
index b1e0b1e..92e01ab 100644
--- a/s390x/unittests.cfg
+++ b/s390x/unittests.cfg
@@ -17,3 +17,8 @@ 
 #			 # to check separated by a space but each check
 #			 # parameter needs to be of the form <path>=<value>
 ##############################################################################
+
+[selftest-setup]
+file = selftest.elf
+groups = selftest
+extra_params = -append 'test 123'