On 6/27/24 20:41, Shuah Khan wrote:
> On 5/6/24 05:13, Laura Nao wrote:
>> Modify the script output to conform to the KTAP format standard. The
>
> What is script here?
>
I was referring to the watchdog-test.c file addressed in this patch. I
understand this could be confusing, I will rephrase the commit message
to avoid ambiguity.
>> number of tests executed is determined by the script arguments, and
>> options such as -c, -f, -h, -i, and -p do not impact the total test
>> count.
>>
>> No functional change is intended.
>
> There are functional changes - keep_alive() coupled with changes
> tailored by a script that isn't in the kernel code which isn't
> ideal.
>
> Why not inlcude the script in this patch series to make it part
> of the kernel?
>
Right, I'll remove the 'no functional change is intended' sentence from
the commit message.
Apart from the patches already in this series, no other script is
required to run the test in a CI environment.
>>
>> Signed-off-by: Laura Nao <laura.nao@xxxxxxxxxxxxx>
>> ---
>> .../selftests/watchdog/watchdog-test.c | 154 ++++++++++--------
>> 1 file changed, 89 insertions(+), 65 deletions(-)
>>
>> diff --git a/tools/testing/selftests/watchdog/watchdog-test.c
>> b/tools/testing/selftests/watchdog/watchdog-test.c
>> index 786cc5a26206..90f32de9e194 100644
>> --- a/tools/testing/selftests/watchdog/watchdog-test.c
>> +++ b/tools/testing/selftests/watchdog/watchdog-test.c
>> @@ -22,6 +22,7 @@
>> #include <sys/ioctl.h>
>> #include <linux/types.h>
>> #include <linux/watchdog.h>
>> +#include "../kselftest.h"
>> #define DEFAULT_PING_RATE 1
>> #define DEFAULT_PING_COUNT 5
>> @@ -29,6 +30,7 @@
>> int fd;
>> const char v = 'V';
>> static const char sopts[] = "bdehp:c:st:Tn:NLf:i";
>> +static const char topts[] = "bdeLn:Nst:T";
>> static const struct option lopts[] = {
>> {"bootstatus", no_argument, NULL, 'b'},
>> {"disable", no_argument, NULL, 'd'},
>> @@ -52,7 +54,7 @@ static const struct option lopts[] = {
>> * the PC Watchdog card to reset its internal timer so it doesn't
>> trigger
>> * a computer reset.
>> */
>> -static void keep_alive(void)
>> +static int keep_alive(void)
>> {
>> int dummy;
>> int ret;
>> @@ -60,6 +62,8 @@ static void keep_alive(void)
>> ret = ioctl(fd, WDIOC_KEEPALIVE, &dummy);
>> if (!ret)
>> printf(".");
>> +
>> + return ret;
>> }
>
> Are these changes driven by the script that isn't in the kernel code?
> I don't want to see changes to keep_alive() bevator.
>
These changes are not driven by any external script; the aim of this
patch is just to conform the output to KTAP for easier parsing of the
results in CI environments.
Returning ret from keep_alive() allows to track the result for
the last WDIOC_KEEPALIVE ioctl and report it to the user through
ksft_test_result, analogously to other ioctls tested in this
same file.
Thanks,
Laura
