[PATCH] Test for clock_nanosleep() with CLOCK_MONOTONIC option.
Gedare Bloom
gedare at rtems.org
Tue Apr 21 17:13:55 UTC 2020
On Tue, Apr 21, 2020 at 4:33 AM Utkarsh Rai <utkarsh.rai60 at gmail.com> 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 +
> testsuites/psxtests/psxclocknanosleep/init.c | 182 ++++++++++++++++++
> .../psxclocknanosleep/psxclocknanosleep.doc | 15 ++
> .../psxclocknanosleep/psxclocknanosleep.scn | 14 ++
> 5 files changed, 222 insertions(+)
> create mode 100644 testsuites/psxtests/psxclocknanosleep/init.c
> create mode 100644 testsuites/psxtests/psxclocknanosleep/psxclocknanosleep.doc
> create mode 100644 testsuites/psxtests/psxclocknanosleep/psxclocknanosleep.scn
>
> diff --git a/testsuites/psxtests/Makefile.am b/testsuites/psxtests/Makefile.am
> index 1f9e4233ec..ad6e41b400 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_psxclocknanosleep
please append 01 to the name.
> +psx_tests += psxclocknanosleep
> +psx_screens += psxclocknanosleep/psxclocknanosleep.scn
> +psx_docs += psxclockrealtime01/psxclocknanosleep.doc
realtime01
> +psxclocknanosleep_SOURCES = psxclocknanosleep/init.c
> +psxclocknanosleep_CPPFLAGS = $(AM_CPPFLAGS) \
> + $(TEST_FLAGS_psxclocknanosleep) $(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..52b5e1624b 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([psxclocknanosleep])
> RTEMS_TEST_CHECK([psxclockrealtime01])
> RTEMS_TEST_CHECK([psxconcurrency01])
> RTEMS_TEST_CHECK([psxcond01])
> diff --git a/testsuites/psxtests/psxclocknanosleep/init.c b/testsuites/psxtests/psxclocknanosleep/init.c
> new file mode 100644
> index 0000000000..15e3ff396a
> --- /dev/null
> +++ b/testsuites/psxtests/psxclocknanosleep/init.c
> @@ -0,0 +1,182 @@
> +/*
> + * 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.
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include "config.h"
> +#endif
> +
> +#include <errno.h>
> +#include <limits.h>
> +#include <rtems.h>
> +#include <time.h>
> +#include <tmacros.h>
> +
> +static void assert_eno(const char *hint, int eno, int expected_eno)
> +{
> + const char *warn;
> +
> + if (eno != expected_eno) {
> + warn = "WARNING: ";
> + } else {
> + warn = "";
> + }
minor nit, you can save some SLOC and conditional branches by
initializing one state, and switching to other. Putting the most
expected state as default/initial is good practice.
So maybe
const char *warn = "";
if ( eno != expected_eno ) {
warn = "WARNING: ";
}
Also note the spacing inside the conditional expression.
> +
> + 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;
> + }
ditto
> +
> + assert_eno(hint, eno, expected_eno);
spacing inside parens (But for function/macro, not before ( or after ).
> +}
> +
> +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;
I don't know if it is a rule, but I would put all type declarations
(and global decls) before functions. It helps readability.
> +
> +static void run(test_mode mode, const char* test_name)
> +{
> + struct timespec delay_time;
> + struct timespec configure_time;
> + int rv;
> + int expected_eno;
> +
> + switch (mode) {
> + case MODE_TIMEOUT_FINITE:
I think we indent the case statement. Check for examples in current
code base, and update rules if it is not present in
https://docs.rtems.org/branches/master/eng/coding-conventions.html
> + 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;
Nice to add a blank line after break. Again not sure if it is a rule,
or should be one or not.
> + 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 += INT64_MAX;
> +
I'd remove this blank line, it interrupts the readability flow here.
> + 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;
> + }
> +
> + printf("*******%s*********\n", test_name);
I think it should be printed at the start of the test, not after
already performing some logic checks?
> +
> + 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 );
> + }
alignment problem, too many spaces in this block.
> +}
> +
> +static void timeout_finite(void)
> +{
> + run ( MODE_TIMEOUT_FINITE, "timeout finite" );
no space between function/macro name and opening parens. (This is
because function-like macros break with a space.) Repeated below
> +}
> +
> +static void timeout_huge_nsec(void)
> +{
> + run( MODE_TIMEOUT_HUGE_NSEC, "timeout huge nsec" );
> +}
> +
> +static void timeout_negative_nsec(void)
> +{
> + run( MODE_TIMEOUT_NEGATIVE_NSEC, "timeout negative nsec" );
> +}
> +
> +static void timeout_negative_sec_nsec(void)
> +{
> + run( MODE_TIMEOUT_NEGATIVE_SEC_NSEC, "timeout negative sec nsec" );
> +}
> +
> +static void timeout_negative_sec(void)
> +{
> + run( MODE_TIMEOUT_NEGATIVE_SEC, "timeout negative sec" );
> +}
> +
> +static void timeout_clock_modify(void)
> +{
> + run( MODE_TIMEOUT_CLOCK_MODIFY, "timeout clock modify" );
> +}
> +
> +const char rtems_test_name[] = "PSXCLOCKNANOSLEEP";
01
> +
> +static rtems_task Init(rtems_task_argument ignored)
> +{
> + TEST_BEGIN();
> +
> + timeout_finite();
> + timeout_huge_nsec();
> + timeout_negative_sec_nsec();
> + timeout_negative_nsec();
> + timeout_negative_sec();
> + timeout_clock_modify();
This could be done simpler, but it seems OK. The functions are
superfluous when you could just do
run( MODE_TIMEOUT_FINITE, "timeout finite")
etcetera, directly here.
> +
> + 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/psxclocknanosleep/psxclocknanosleep.doc b/testsuites/psxtests/psxclocknanosleep/psxclocknanosleep.doc
> new file mode 100644
> index 0000000000..a87874c627
> --- /dev/null
> +++ b/testsuites/psxtests/psxclocknanosleep/psxclocknanosleep.doc
> @@ -0,0 +1,15 @@
> +/*
> +This file describes the directives and concepts tested by this test set.
> +
> +test set name: psxclocknanosleep
01
> +
> +directives:
> +
> + - clock_nanosleep()
> +
> +concepts:
> +
> + - Test some invalid and extreme timeout values.
> + - Ensure that the CLOCK_MONOTONIC based delay is not effected by changes to
s/effected/affected
Effect is a noun, Affect is a verb.
> + CLOCK_REALTIME
> +*/
> \ No newline at end of file
> diff --git a/testsuites/psxtests/psxclocknanosleep/psxclocknanosleep.scn b/testsuites/psxtests/psxclocknanosleep/psxclocknanosleep.scn
> new file mode 100644
> index 0000000000..af803c8b14
> --- /dev/null
> +++ b/testsuites/psxtests/psxclocknanosleep/psxclocknanosleep.scn
> @@ -0,0 +1,14 @@
> +*** BEGIN OF TEST PSXCLOCKNANOSLEEP ***
> +*******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 PSXCLOCKNANOSLEEP ***
> --
> 2.17.1
>
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
More information about the devel
mailing list