[PATCH v2] Test for clock_nanosleep() with CLOCK_MONOTONIC option.

Sebastian Huber sebastian.huber at embedded-brains.de
Wed Apr 22 18:11:16 UTC 2020


On 22/04/2020 17:53, Utkarsh Rai wrote:

> Rationale for a new test-
>
>> Although most of the test cases for this test have been taken from clockrealtime,
> adding it to the current test with CLOCK_MONOTONIC may break the existing cases.
>
>> This test has a new case which tests for no change of delay with monotonic clock when
> the realtime clock has been modified, this is easier to add in a new test.
> ---
>   testsuites/psxtests/Makefile.am               |  10 ++
>   testsuites/psxtests/configure.ac              |   1 +
>   .../psxtests/psxclocknanosleep01/init.c       | 155 ++++++++++++++++++
>   .../psxclocknanosleep01.doc                   |  15 ++
>   .../psxclocknanosleep01.scn                   |  14 ++
>   5 files changed, 195 insertions(+)
>   create mode 100644 testsuites/psxtests/psxclocknanosleep01/init.c
>   create mode 100644 testsuites/psxtests/psxclocknanosleep01/psxclocknanosleep01.doc
>   create mode 100644 testsuites/psxtests/psxclocknanosleep01/psxclocknanosleep01.scn
>
> diff --git a/testsuites/psxtests/Makefile.am b/testsuites/psxtests/Makefile.am
> index 1f9e4233ec..ba79804be7 100755
> --- a/testsuites/psxtests/Makefile.am
> +++ b/testsuites/psxtests/Makefile.am
> @@ -312,6 +312,16 @@ psxclock01_CPPFLAGS = $(AM_CPPFLAGS) $(TEST_FLAGS_psxclock01) \
>   	$(support_includes) -I$(top_srcdir)/include
>   endif
>   
> +
> +if TEST_psxclocknanosleep01
> +psx_tests += psxclocknanosleep01
> +psx_screens += psxclocknanosleep01/psxclocknanosleep01.scn
> +psx_docs += psxclocknanosleep01/psxclocknanosleep01.doc
> +psxclocknanosleep01_SOURCES = psxclocknanosleep01/init.c
> +psxclocknanosleep01_CPPFLAGS = $(AM_CPPFLAGS) \
> +	$(TEST_FLAGS_psxclocknanosleep01) $(support_includes)
> +endif
> +
>   if TEST_psxclockrealtime01
>   psx_tests += psxclockrealtime01
>   psx_screens += psxclockrealtime01/psxclockrealtime01.scn
> diff --git a/testsuites/psxtests/configure.ac b/testsuites/psxtests/configure.ac
> index 139787cccb..9bfe8e2c0b 100644
> --- a/testsuites/psxtests/configure.ac
> +++ b/testsuites/psxtests/configure.ac
> @@ -75,6 +75,7 @@ RTEMS_TEST_CHECK([psxcleanup01])
>   RTEMS_TEST_CHECK([psxcleanup02])
>   RTEMS_TEST_CHECK([psxclock])
>   RTEMS_TEST_CHECK([psxclock01])
> +RTEMS_TEST_CHECK([psxclocknanosleep01])
>   RTEMS_TEST_CHECK([psxclockrealtime01])
>   RTEMS_TEST_CHECK([psxconcurrency01])
>   RTEMS_TEST_CHECK([psxcond01])
> diff --git a/testsuites/psxtests/psxclocknanosleep01/init.c b/testsuites/psxtests/psxclocknanosleep01/init.c
> new file mode 100644
> index 0000000000..c1f10b9eb9
> --- /dev/null
> +++ b/testsuites/psxtests/psxclocknanosleep01/init.c
> @@ -0,0 +1,155 @@
> +/*
> + *  Copyright (c) 2020 Utkarsh Rai.
> + *
> + *  The license and distribution terms for this file may be
> + *  found in the file LICENSE in this distribution or at
> + *  http://www.rtems.org/license/LICENSE.
> + */

Please use this template for new files (BSD-2-Clause license):

https://docs.rtems.org/branches/master/eng/coding-file-hdr.html#c-c-assembler-source-file-template

> +
> +#ifdef HAVE_CONFIG_H
> +#include "config.h"
> +#endif
> +
> +#include <errno.h>
> +#include <limits.h>
> +#include <rtems.h>
> +#include <time.h>
> +#include <tmacros.h>
> +
> +typedef enum {
> +  MODE_TIMEOUT_FINITE,
> +  MODE_TIMEOUT_NEGATIVE_SEC,
> +  MODE_TIMEOUT_NEGATIVE_NSEC,
> +  MODE_TIMEOUT_NEGATIVE_SEC_NSEC,
> +  MODE_TIMEOUT_HUGE_NSEC,
> +  MODE_TIMEOUT_CLOCK_MODIFY
> +} test_mode;
> +
> +static void assert_eno(const char *hint, int eno, int expected_eno)
> +{
> +  const char *warn = "";
> +
> +  if ( eno != expected_eno ) {
> +    warn = "WARNING: ";
> +  }
> +
> +  printf(
> +    "%s%s: actual '%s', expected '%s'\n",
> +    warn,
> +    hint,
> +    strerror(eno),
> +    strerror(expected_eno)
> +  );
> +
> +  rtems_test_assert( eno == expected_eno );
> +}
> +
> +
> +static void assert_rv(const char *hint, int rv, int expected_eno)
> +{
> +  int eno;
> +
> +  if ( rv != 0 ) {
> +    eno = EINVAL;
> +  } else {
> +    eno = 0;
> +  }
> +
> +  assert_eno( hint, eno, expected_eno );
> +}

I am not in favour of adding new test support code like this. The new 
test framework has for example:

https://docs.rtems.org/branches/master/eng/test-framework.html#posix-error-numbers

> +
> +static void run(test_mode mode, const char* test_name)
> +{
> + struct timespec  delay_time;
> + struct timespec  configure_time;
> + int     rv;
> + int     expected_eno;
> +
> + printf("*******%s*********\n", test_name);
The new test framework prints stuff like this in a format easy to parse.
> +
> + switch (mode) {
> +   case MODE_TIMEOUT_FINITE:
> +     rv = clock_gettime( CLOCK_MONOTONIC, &delay_time );
> +     rtems_test_assert( rv == 0 );
> +
> +     delay_time.tv_sec += 1;
> +     delay_time.tv_nsec += 1;
> +     expected_eno = ETIMEDOUT;
> +     break;

I think this switch case just make the code harder to read. What is the 
benefit? In the new test framework one of the test cases could look like:

T_TEST_CASE(POSIXClockGettimeTimeoutFinite)
{
     struct timespec delay_time;
     int eno;

     eno = clock_gettime( CLOCK_MONOTONIC, &delay_time );
     T_eno_success( eno );

     delay_time.tv_sec += 1;
     delay_time.tv_nsec += 1; // This can result in an invalid time, 
e.g. if 1.999999999 before the addition

     eno = clock_nanosleep( CLOCK_MONOTONIC, TIMER_ABSTIME, &delay_time, 
NULL );
     T_eno( eno, ETIMEDOUT );
}

> +
> +   case MODE_TIMEOUT_HUGE_NSEC:
> +     delay_time.tv_sec = 1;
> +     delay_time.tv_nsec = LONG_MAX;
> +     expected_eno = EINVAL;
> +     break;
> +
> +   case MODE_TIMEOUT_NEGATIVE_NSEC:
> +     delay_time.tv_sec = 1;
> +     delay_time.tv_nsec = -1;
> +     expected_eno = EINVAL;
> +     break;
> +
> +   case MODE_TIMEOUT_NEGATIVE_SEC_NSEC:
> +     delay_time.tv_sec = -1;
> +     delay_time.tv_nsec = -1;
> +     expected_eno = EINVAL;
> +     break;
> +
> +   case MODE_TIMEOUT_NEGATIVE_SEC:
> +     delay_time.tv_sec = -1;
> +     delay_time.tv_nsec = 1;
> +     expected_eno = ETIMEDOUT;
> +     break;
> +
> +   case MODE_TIMEOUT_CLOCK_MODIFY:
> +     rv = clock_gettime( CLOCK_REALTIME, &configure_time );
> +     rtems_test_assert( rv == 0 );
> +
> +     configure_time.tv_sec += 3600;
> +     rv = clock_settime( CLOCK_REALTIME, &configure_time );
> +     rtems_test_assert( rv == 0 );
> +
> +     rv = clock_gettime( CLOCK_MONOTONIC, &delay_time );
> +     rtems_test_assert( rv == 0 );
> +
> +     delay_time.tv_sec += 1;
> +     delay_time.tv_nsec += 1;
> +     expected_eno = ETIMEDOUT;
> +     break;
> +
> +   default:
> +     rtems_test_assert(0);
> +     break;
> + }
> +
> + rv = clock_nanosleep( CLOCK_MONOTONIC, TIMER_ABSTIME, &delay_time, NULL );
> + if ( expected_eno == ETIMEDOUT ) {
> +   assert_rv( "clock_nanosleep(clock monotonic)", rv, 0 );
> + } else {
> +   assert_rv( "clock_nanosleep(clock monotonic)", rv, expected_eno );
> + }
> +}
> +
> +const char rtems_test_name[] = "PSXCLOCKNANOSLEEP01";
> +
> +static rtems_task Init(rtems_task_argument ignored)
> +{
> +  TEST_BEGIN();
> +
> +  run( MODE_TIMEOUT_FINITE, "timeot finite" );
> +  run( MODE_TIMEOUT_HUGE_NSEC, "timeout huge nsec" );
> +  run( MODE_TIMEOUT_NEGATIVE_SEC_NSEC, "timeout negative sec nsec" );
> +  run( MODE_TIMEOUT_NEGATIVE_NSEC, "timeout negative nsec" );
> +  run( MODE_TIMEOUT_NEGATIVE_SEC, "timeout negative sec" );
> +  run( MODE_TIMEOUT_CLOCK_MODIFY, "timeout clock modify" );
> +
> +  TEST_END();
> +  rtems_test_exit(0);
> +}
> +
> +#define CONFIGURE_APPLICATION_NEEDS_CLOCK_DRIVER
> +
> +#define CONFIGURE_RTEMS_INIT_TASKS_TABLE
> +#define CONFIGURE_MAXIMUM_TASKS  1
> +#define CONFIGURE_INIT
> +#include <rtems/confdefs.h>
> \ No newline at end of file
> diff --git a/testsuites/psxtests/psxclocknanosleep01/psxclocknanosleep01.doc b/testsuites/psxtests/psxclocknanosleep01/psxclocknanosleep01.doc
> new file mode 100644
> index 0000000000..190748ac5a
> --- /dev/null
> +++ b/testsuites/psxtests/psxclocknanosleep01/psxclocknanosleep01.doc
> @@ -0,0 +1,15 @@
> +/*
> +This file describes the directives and concepts tested by this test set.
> +
> +test set name: psxclocknanosleep01
> +
> +directives:
> +
> +  - clock_nanosleep()
> +
> +concepts:
> +
> +  - Test some invalid and extreme timeout values.
> +  - Ensure that the CLOCK_MONOTONIC based delay is not affected by changes to
> +    CLOCK_REALTIME
> +*/
> \ No newline at end of file
> diff --git a/testsuites/psxtests/psxclocknanosleep01/psxclocknanosleep01.scn b/testsuites/psxtests/psxclocknanosleep01/psxclocknanosleep01.scn
> new file mode 100644
> index 0000000000..567daea80d
> --- /dev/null
> +++ b/testsuites/psxtests/psxclocknanosleep01/psxclocknanosleep01.scn
> @@ -0,0 +1,14 @@
> +*** BEGIN OF TEST PSXCLOCKNANOSLEEP01 ***
> +*******timeout finite*********
> +clock_nanosleep(clock monotonic): actual 'Success', expected 'Success'
> +*******timeout huge nsec*********
> +clock_nanosleep(clock monotonic): actual 'Invalid argument', expected 'Invalid argument'
> +*******timeout negative sec nsec*********
> +clock_nanosleep(clock monotonic): actual 'Invalid argument', expected 'Invalid argument'
> +*******timeout negative nsec*********
> +clock_nanosleep(clock monotonic): actual 'Invalid argument', expected 'Invalid argument'
> +*******timeout negative sec*********
> +clock_nanosleep(clock monotonic): actual 'Success', expected 'Success'
> +*******timeout clock modify*********
> +clock_nanosleep(clock monotonic): actual 'Success', expected 'Success'
> +*** END OF TEST PSXCLOCKNANOSLEEP01 ***


More information about the devel mailing list