[PATCH v2] Tests for pthread_getcpuclockid method. Ticket:3891
Eshan Dhawan
eshandhawan51 at gmail.com
Mon Apr 13 17:32:11 UTC 2020
On Mon, Apr 13, 2020 at 10:26 PM Joel Sherrill <joel at rtems.org> wrote:
> 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.
>
I have tested it against this patch
https://lists.rtems.org/pipermail/devel/2020-March/058176.html
and It works fine
>
> 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.
>
Tested it on sparc/erc32
> 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.
>
I will correct that
>
>
>> +#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.
>
Ok
>
>
>> + 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.
>
> Ok
> 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 {
> }
>
>
Ok
> +
>> + 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.
>
>
OK
> +
>> +#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/a578c972/attachment-0001.html>
More information about the devel
mailing list