[PATCH v2] Tests for pthread_getcpuclockid method. Ticket:3891

Joel Sherrill joel at rtems.org
Mon Apr 13 16:56:16 UTC 2020


I am pretty sure I can report that it won't execute on just visually
reviewing it.

First, it assumes at least a semi-functional pthread_getcpuclockid() and
there is not one in the tree.

You should be testing things yourself. There are multiple simulators which
have BSPs and are easy to run. We can discuss which one in another thread.

Other comments interspersed.

On Thu, Apr 9, 2020 at 3:30 PM Eshan dhawan <eshandhawan51 at gmail.com> wrote:

> ---
>  testsuites/psxtests/Makefile.am               |  10 ++
>  testsuites/psxtests/configure.ac              |   1 +
>  testsuites/psxtests/psxgetcpuclockid01/init.c | 120 ++++++++++++++++++
>  .../psxgetcpuclockid01/psxgetcpuclockid01.doc |  19 +++
>  .../psxgetcpuclockid01/psxgetcpuclockid01.scn |   4 +
>  5 files changed, 154 insertions(+)
>  create mode 100644 testsuites/psxtests/psxgetcpuclockid01/init.c
>  create mode 100644
> testsuites/psxtests/psxgetcpuclockid01/psxgetcpuclockid01.doc
>  create mode 100644
> testsuites/psxtests/psxgetcpuclockid01/psxgetcpuclockid01.scn
>
> diff --git a/testsuites/psxtests/Makefile.am
> b/testsuites/psxtests/Makefile.am
> index 1f9e4233ec..ffadff16e7 100755
> --- a/testsuites/psxtests/Makefile.am
> +++ b/testsuites/psxtests/Makefile.am
> @@ -462,6 +462,16 @@ psxgetattrnp01_CPPFLAGS = $(AM_CPPFLAGS)
> $(TEST_FLAGS_psxgetattrnp01) \
>         $(support_includes) -I$(top_srcdir)/include
>  endif
>
> +if TEST_psxgetcpuclockid01
> +psx_tests += psxgetcpuclockid01
> +psx_screens += psxgetcpuclockid01/psxgetcpuclockid01.scn
> +psx_docs += psxgetcpuclockid01/psxgetcpuclockid01.doc
> +psxgetcpuclockid01_SOURCES = psxgetcpuclockid01/init.c
> +psxgetcpuclockid01_CPPFLAGS = $(AM_CPPFLAGS)
> $(TEST_FLAGS_psxgetcpuclockid01) \
> +       $(support_includes)
> +endif
> +
> +
>  if TEST_psxgetrusage01
>  psx_tests += psxgetrusage01
>  psx_screens += psxgetrusage01/psxgetrusage01.scn
> diff --git a/testsuites/psxtests/configure.ac b/testsuites/psxtests/
> configure.ac
> index 139787cccb..c509086abc 100644
> --- a/testsuites/psxtests/configure.ac
> +++ b/testsuites/psxtests/configure.ac
> @@ -91,6 +91,7 @@ RTEMS_TEST_CHECK([psxfile01])
>  RTEMS_TEST_CHECK([psxfile02])
>  RTEMS_TEST_CHECK([psxfilelock01])
>  RTEMS_TEST_CHECK([psxgetattrnp01])
> +RTEMS_TEST_CHECK([psxgetcpuclockid01])
>  RTEMS_TEST_CHECK([psxgetrusage01])
>  RTEMS_TEST_CHECK([psxglobalcon01])
>  RTEMS_TEST_CHECK([psxglobalcon02])
> diff --git a/testsuites/psxtests/psxgetcpuclockid01/init.c
> b/testsuites/psxtests/psxgetcpuclockid01/init.c
> new file mode 100644
> index 0000000000..8e1093e44b
> --- /dev/null
> +++ b/testsuites/psxtests/psxgetcpuclockid01/init.c
> @@ -0,0 +1,120 @@
> +/*
> + *  @file
> + *  @brief Test suite for getcpuclockid methods
> + */
> +
> +/*
> + * SPDX-License-Identifier: BSD-2-Clause
> + *
> + * Copyright (C) 2020 Eshan Dhawan
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + *    notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + *    notice, this list of conditions and the following disclaimer in the
> + *    documentation and/or other materials provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> "AS IS"
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
> THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
> PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS
> BE
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
> BUSINESS
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
> THE
> + * POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +
> +#ifdef HAVE_CONFIG_H
> +#include "config.h"
> +#endif
> +
> +#include <errno.h>
> +#include <pthread.h>
> +#include <stdint.h>
> +#include <rtems/test.h>
> +#include <sys/utsname.h>
> +#include <unistd.h>
> +#include <errno.h>
> +#include <sched.h>
>
> I am sure at least some of these are not needed. I suspect almost
everything below rtems/test.h is not needed.

errno.h is twice.


> +#include <rtems/score/todimpl.h>
> +#include <rtems/score/threadimpl.h>
>

Why are these needed?


> +#include "tmacros.h"
> +
> +const char rtems_test_name[] = "PSXGETCPUCLOCKID 1";
> +
> +/* Forward declaration to avoid warnings */
> +rtems_task Init(rtems_task_argument ignored);
> +void *test_pthread(void *);
> +/* test_task function begins */
> +void *test_pthread(void *arg)
> +{
> +  int r;
> +  clockid_t cid;
> +
> +  r = pthread_getcpuclockid( pthread_self(), &cid );
> +  if(r)
> +  {
> +    printf( "pthread_getcpuclockid : 0x%d", r );
> +  }
> +  rtems_test_assert( r == 0 );
> +
> +  pthread_exit(0);
> +  return NULL;
> +}
> +
> +/* init test function begins */
> +rtems_task Init(rtems_task_argument ignored)
> +{
>

For POSIX tests, use a POSIX Initialization thread unless specifically
verifying that a Classic API task has particular behavior when using POSIX
methods.



> +  TEST_BEGIN();
> +
> +  int r;
> +  clockid_t cid;
> +  pthread_t thread_id;
> +
> +  r= pthread_create( &thread_id, NULL, test_pthread, NULL );
> +  if( r )
> +  {
> +    printf( "pthread_create : 0x%d", r );
> +  }
> +  rtems_test_assert( r == 0 );
>


First, "0x%d" will print a 0x saying it is hex and then print a decimal
number.

Next, I think there are macro constants if you need to print RTEMS types. I
can't find anything except pritime.h and primode.h. Perhaps someone else
can step in here for help.

Some style issues:

+ if( should be "if ("

+ Braces should follow the denser K&R style:
   if (condition) {
   } else {
   }


> +
> + r = pthread_join( thread_id, NULL );
> +  if( r )
> +  {
> +    printf( "pthread_join : 0x%d", r );
> +  }
> +  rtems_test_assert( r == 0 );
>

You did not set any pthread attributes so you can't be certain that the
thread you created is joinable.

> +
> +  r = pthread_getcpuclockid( thread_id, &cid );
> +  if( r != ESRCH )
> +  {
> +    printf( "pthread_getcpuclockid : 0x%d", r );
> +  }
> +  rtems_test_assert( r == ESRCH );
>

I would start with a simple functional test on a single pthread that gets
cpu clock id (should succeed).

Then get its cpu clock and save that time.

Use some get time API in a loop and spin (no blocking) for say 1/2 second
of wall time passage.

Then see if the CPU time used went up 1/2 second as expected.

Be aware that the increase in CPU time used will not be exactly 1/2 second.
It will be at least 1/2 second but there is a reasonable upper bounds above
that.

> +
> +  TEST_END();
> +  rtems_test_exit(0);
> +}
> +
> +#define CONFIGURE_APPLICATION_NEEDS_CLOCK_DRIVER
> +#define CONFIGURE_APPLICATION_NEEDS_SIMPLE_CONSOLE_DRIVER
> +
> +#define CONFIGURE_MAXIMUM_POSIX_THREADS     2
> +#define CONFIGURE_MAXIMUM_TASKS 1
>

I see a need for 1 pthread in the code as written. Should stay 2 pthreads
because you need to switch to pthread initialization thread.

> +
> +#define CONFIGURE_INITIAL_EXTENSIONS RTEMS_TEST_INITIAL_EXTENSION
> +
> +#define CONFIGURE_RTEMS_INIT_TASKS_TABLE
>

Switch here to pthread initialization threads. And change Init task to
POSIX_Init thread.


> +
> +#define CONFIGURE_INIT
> +
> +#include <rtems/confdefs.h>
> +
> diff --git a/testsuites/psxtests/psxgetcpuclockid01/psxgetcpuclockid01.doc
> b/testsuites/psxtests/psxgetcpuclockid01/psxgetcpuclockid01.doc
> new file mode 100644
> index 0000000000..145f009337
> --- /dev/null
> +++ b/testsuites/psxtests/psxgetcpuclockid01/psxgetcpuclockid01.doc
> @@ -0,0 +1,19 @@
> +#  COPYRIGHT (c) 2020
> +#  On-Line Applications Research Corporation (OAR).
> +#
> +# SPDX-License-Identifier: BSD-2-Clause
> +#
> +
> +This file describes the directives and concepts tested by this test set.
> +
> +test set name:  psxgetcpuclockid 1
> +
> +Directives:
> +  Pthread_getcpuclockid()
> +
> +Concepts:
> +
> ++ This test exercises the getcpuclockid methods.
> +
> ++ For pthread_getcpuclockid it created a pthread using pthread_create and
> then finds
> +the clockid_t of that thread and then ends the thread.
> +After the thread is ended it again tried to find the clockid_t of the
> thread to check
> +for ESRCH exception.
> +
> diff --git a/testsuites/psxtests/psxgetcpuclockid01/psxgetcpuclockid01.scn
> b/testsuites/psxtests/psxgetcpuclockid01/psxgetcpuclockid01.scn
> new file mode 100644
> index 0000000000..937a908fd5
> --- /dev/null
> +++ b/testsuites/psxtests/psxgetcpuclockid01/psxgetcpuclockid01.scn
> @@ -0,0 +1,4 @@
> +*** BEGIN OF TEST PSXGETCLOCKID 1 ***
> +
> +*** END OF TEST PSXGETCPUCLOCKID 1 ***
> +
> --
> 2.17.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20200413/c7eb8b0b/attachment-0001.html>


More information about the devel mailing list