On Tue, Apr 21, 2020 at 4:33 AM Utkarsh Rai <utkarsh.ra...@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@rtems.org > http://lists.rtems.org/mailman/listinfo/devel _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel