remove regress maxtime

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

remove regress maxtime

Alexander Bluhm
Hi,

Does anyone use the REGRESS_MAXTIME feature?  I would like to remove it.

- The timeout based on CPU seconds is pretty useless.
  Most hanging tests sleep and do not spin.
- A timeout cannot be distinguished from failure.
- I have never triggered it.
- Only 3 tests set it.
- It comes in my way for a new fail/pass reporting feature.

Also do _SKIP_FAIL cleanup.

ok?

bluhm

Index: regress/lib/libpthread/Makefile.inc
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/regress/lib/libpthread/Makefile.inc,v
retrieving revision 1.10
diff -u -p -r1.10 Makefile.inc
--- regress/lib/libpthread/Makefile.inc 19 Aug 2012 18:55:16 -0000 1.10
+++ regress/lib/libpthread/Makefile.inc 14 May 2019 05:56:28 -0000
@@ -10,5 +10,3 @@ CFLAGS+= -Wall # -Werror
 #DEBUG= -ggdb
 CFLAGS+= -DSRCDIR='"${.CURDIR}"'
 CFLAGS+= -I${.CURDIR}/../include
-
-REGRESS_MAXTIME?=30
Index: regress/lib/libpthread/pthread_specific/Makefile
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/regress/lib/libpthread/pthread_specific/Makefile,v
retrieving revision 1.2
diff -u -p -r1.2 Makefile
--- regress/lib/libpthread/pthread_specific/Makefile 22 Dec 2013 11:08:31 -0000 1.2
+++ regress/lib/libpthread/pthread_specific/Makefile 14 May 2019 05:56:28 -0000
@@ -2,6 +2,4 @@

 PROG= pthread_specific

-REGRESS_MAXTIME=60
-
 .include <bsd.regress.mk>
Index: regress/lib/libpthread/stdarg/Makefile
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/regress/lib/libpthread/stdarg/Makefile,v
retrieving revision 1.5
diff -u -p -r1.5 Makefile
--- regress/lib/libpthread/stdarg/Makefile 26 Dec 2013 16:22:55 -0000 1.5
+++ regress/lib/libpthread/stdarg/Makefile 14 May 2019 05:56:28 -0000
@@ -4,6 +4,4 @@ PROG= stdarg

 CFLAGS+= -I${.CURDIR}/../include

-REGRESS_MAXTIME=10
-
 .include <bsd.regress.mk>
Index: share/man/man5/bsd.regress.mk.5
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/share/man/man5/bsd.regress.mk.5,v
retrieving revision 1.18
diff -u -p -r1.18 bsd.regress.mk.5
--- share/man/man5/bsd.regress.mk.5 2 Dec 2018 11:46:31 -0000 1.18
+++ share/man/man5/bsd.regress.mk.5 14 May 2019 05:33:37 -0000
@@ -80,9 +80,6 @@ Points to the fully-qualified path of a
 results are appended.
 Defaults to
 .Pa /dev/null .
-.It Ev REGRESS_MAXTIME
-Maximum limit of CPU seconds to spend on the regression test.
-Exceeding this time will result in a failure being logged.
 .It Ev REGRESS_ROOT_TARGETS
 Targets for which root access is required to run the test.
 The
Index: share/mk/bsd.regress.mk
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/share/mk/bsd.regress.mk,v
retrieving revision 1.17
diff -u -p -r1.17 bsd.regress.mk
--- share/mk/bsd.regress.mk 3 Dec 2018 22:30:04 -0000 1.17
+++ share/mk/bsd.regress.mk 14 May 2019 05:43:43 -0000
@@ -39,14 +39,14 @@ REGRESS_SKIP_TARGETS=run-regress-${PROG}
 .  endif
 .endif

-.if defined(REGRESS_SLOW_TARGETS) && ${REGRESS_SKIP_SLOW} != no
+.if defined(REGRESS_SLOW_TARGETS) && ${REGRESS_SKIP_SLOW:L} != no
 REGRESS_SKIP_TARGETS+=${REGRESS_SLOW_TARGETS}
 .endif

-.if ${REGRESS_FAIL_EARLY} != no
-_SKIP_FAIL=
+.if ${REGRESS_FAIL_EARLY:L} != no
+_REGRESS_IGNORE_FAIL=
 .else
-_SKIP_FAIL=-
+_REGRESS_IGNORE_FAIL= -
 .endif

 .if defined(REGRESS_ROOT_TARGETS)
@@ -102,26 +102,13 @@ regress: .SILENT
  @echo -n "SKIP " ${_REGRESS_OUT}
  @echo SKIPPED
 .  else
-# XXX - we need a better method to see if a test fails due to timeout or just
-#       normal failure.
-.   if !defined(REGRESS_MAXTIME)
- ${_SKIP_FAIL}if cd ${.CURDIR} && ${MAKE} ${RT}; then \
+ ${_REGRESS_IGNORE_FAIL} if ${MAKE} -C ${.CURDIR} ${RT}; then \
     echo -n "SUCCESS " ${_REGRESS_OUT} ; \
  else \
     echo -n "FAIL " ${_REGRESS_OUT} ; \
     echo FAILED ; \
     false; \
  fi
-.   else
- ${_SKIP_FAIL}if cd ${.CURDIR} && \
-    (ulimit -t ${REGRESS_MAXTIME} ; ${MAKE} ${RT}); then \
-    echo -n "SUCCESS " ${_REGRESS_OUT} ; \
- else \
-    echo -n "FAIL (possible timeout) " ${_REGRESS_OUT} ; \
-    echo FAILED ; \
-    false; \
- fi
-.   endif
 .  endif
  @echo ${_REGRESS_NAME}/${RT:S/^run-regress-//} ${_REGRESS_OUT}
 .endfor

Reply | Threaded
Open this post in threaded view
|

Re: remove regress maxtime

Ingo Schwarze
Hi Alexander,

Alexander Bluhm wrote on Tue, May 14, 2019 at 02:16:19AM -0400:

> Does anyone use the REGRESS_MAXTIME feature?

Not me.

> I would like to remove it.
>
> - The timeout based on CPU seconds is pretty useless.
>   Most hanging tests sleep and do not spin.
> - A timeout cannot be distinguished from failure.
> - I have never triggered it.
> - Only 3 tests set it.
> - It comes in my way for a new fail/pass reporting feature.

I like the deletion.

Getting rid of features that work poorly, are fragile, are rarely used
and get in the way of maintenance and development is good.
Simplicity is good, and having one robust way of doing things rather
than some rarely-tested options is good.

Please delete the "BUGS AND LIMITATIONS" section in bsd.regress.mk(5),
too, which had a wrong name, anyway.

> Also do _SKIP_FAIL cleanup.

Sure, why not.

Yours,
  Ingo


> Index: regress/lib/libpthread/Makefile.inc
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/regress/lib/libpthread/Makefile.inc,v
> retrieving revision 1.10
> diff -u -p -r1.10 Makefile.inc
> --- regress/lib/libpthread/Makefile.inc 19 Aug 2012 18:55:16 -0000 1.10
> +++ regress/lib/libpthread/Makefile.inc 14 May 2019 05:56:28 -0000
> @@ -10,5 +10,3 @@ CFLAGS+= -Wall # -Werror
>  #DEBUG= -ggdb
>  CFLAGS+= -DSRCDIR='"${.CURDIR}"'
>  CFLAGS+= -I${.CURDIR}/../include
> -
> -REGRESS_MAXTIME?=30
> Index: regress/lib/libpthread/pthread_specific/Makefile
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/regress/lib/libpthread/pthread_specific/Makefile,v
> retrieving revision 1.2
> diff -u -p -r1.2 Makefile
> --- regress/lib/libpthread/pthread_specific/Makefile 22 Dec 2013 11:08:31 -0000 1.2
> +++ regress/lib/libpthread/pthread_specific/Makefile 14 May 2019 05:56:28 -0000
> @@ -2,6 +2,4 @@
>
>  PROG= pthread_specific
>
> -REGRESS_MAXTIME=60
> -
>  .include <bsd.regress.mk>
> Index: regress/lib/libpthread/stdarg/Makefile
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/regress/lib/libpthread/stdarg/Makefile,v
> retrieving revision 1.5
> diff -u -p -r1.5 Makefile
> --- regress/lib/libpthread/stdarg/Makefile 26 Dec 2013 16:22:55 -0000 1.5
> +++ regress/lib/libpthread/stdarg/Makefile 14 May 2019 05:56:28 -0000
> @@ -4,6 +4,4 @@ PROG= stdarg
>
>  CFLAGS+= -I${.CURDIR}/../include
>
> -REGRESS_MAXTIME=10
> -
>  .include <bsd.regress.mk>
> Index: share/man/man5/bsd.regress.mk.5
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/share/man/man5/bsd.regress.mk.5,v
> retrieving revision 1.18
> diff -u -p -r1.18 bsd.regress.mk.5
> --- share/man/man5/bsd.regress.mk.5 2 Dec 2018 11:46:31 -0000 1.18
> +++ share/man/man5/bsd.regress.mk.5 14 May 2019 05:33:37 -0000
> @@ -80,9 +80,6 @@ Points to the fully-qualified path of a
>  results are appended.
>  Defaults to
>  .Pa /dev/null .
> -.It Ev REGRESS_MAXTIME
> -Maximum limit of CPU seconds to spend on the regression test.
> -Exceeding this time will result in a failure being logged.
>  .It Ev REGRESS_ROOT_TARGETS
>  Targets for which root access is required to run the test.
>  The
> Index: share/mk/bsd.regress.mk
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/share/mk/bsd.regress.mk,v
> retrieving revision 1.17
> diff -u -p -r1.17 bsd.regress.mk
> --- share/mk/bsd.regress.mk 3 Dec 2018 22:30:04 -0000 1.17
> +++ share/mk/bsd.regress.mk 14 May 2019 05:43:43 -0000
> @@ -39,14 +39,14 @@ REGRESS_SKIP_TARGETS=run-regress-${PROG}
>  .  endif
>  .endif
>
> -.if defined(REGRESS_SLOW_TARGETS) && ${REGRESS_SKIP_SLOW} != no
> +.if defined(REGRESS_SLOW_TARGETS) && ${REGRESS_SKIP_SLOW:L} != no
>  REGRESS_SKIP_TARGETS+=${REGRESS_SLOW_TARGETS}
>  .endif
>
> -.if ${REGRESS_FAIL_EARLY} != no
> -_SKIP_FAIL=
> +.if ${REGRESS_FAIL_EARLY:L} != no
> +_REGRESS_IGNORE_FAIL=
>  .else
> -_SKIP_FAIL=-
> +_REGRESS_IGNORE_FAIL= -
>  .endif
>
>  .if defined(REGRESS_ROOT_TARGETS)
> @@ -102,26 +102,13 @@ regress: .SILENT
>   @echo -n "SKIP " ${_REGRESS_OUT}
>   @echo SKIPPED
>  .  else
> -# XXX - we need a better method to see if a test fails due to timeout or just
> -#       normal failure.
> -.   if !defined(REGRESS_MAXTIME)
> - ${_SKIP_FAIL}if cd ${.CURDIR} && ${MAKE} ${RT}; then \
> + ${_REGRESS_IGNORE_FAIL} if ${MAKE} -C ${.CURDIR} ${RT}; then \
>      echo -n "SUCCESS " ${_REGRESS_OUT} ; \
>   else \
>      echo -n "FAIL " ${_REGRESS_OUT} ; \
>      echo FAILED ; \
>      false; \
>   fi
> -.   else
> - ${_SKIP_FAIL}if cd ${.CURDIR} && \
> -    (ulimit -t ${REGRESS_MAXTIME} ; ${MAKE} ${RT}); then \
> -    echo -n "SUCCESS " ${_REGRESS_OUT} ; \
> - else \
> -    echo -n "FAIL (possible timeout) " ${_REGRESS_OUT} ; \
> -    echo FAILED ; \
> -    false; \
> - fi
> -.   endif
>  .  endif
>   @echo ${_REGRESS_NAME}/${RT:S/^run-regress-//} ${_REGRESS_OUT}
>  .endfor

Reply | Threaded
Open this post in threaded view
|

Re: remove regress maxtime

Otto Moerbeek
In reply to this post by Alexander Bluhm
On Tue, May 14, 2019 at 02:16:19AM -0400, Alexander Bluhm wrote:

> Hi,
>
> Does anyone use the REGRESS_MAXTIME feature?  I would like to remove it.
>
> - The timeout based on CPU seconds is pretty useless.
>   Most hanging tests sleep and do not spin.
> - A timeout cannot be distinguished from failure.
> - I have never triggered it.
> - Only 3 tests set it.
> - It comes in my way for a new fail/pass reporting feature.
>
> Also do _SKIP_FAIL cleanup.

yes please. If you modify the malloc_duel test to use lots of threads it
fails because each tread uses cpu and that gets aggregated so it hits
the limit quickly. If this is removed I can make the test better and we
can enable it to be run always.

        -Otto

>
> ok?
>
> bluhm
>
> Index: regress/lib/libpthread/Makefile.inc
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/regress/lib/libpthread/Makefile.inc,v
> retrieving revision 1.10
> diff -u -p -r1.10 Makefile.inc
> --- regress/lib/libpthread/Makefile.inc 19 Aug 2012 18:55:16 -0000 1.10
> +++ regress/lib/libpthread/Makefile.inc 14 May 2019 05:56:28 -0000
> @@ -10,5 +10,3 @@ CFLAGS+= -Wall # -Werror
>  #DEBUG= -ggdb
>  CFLAGS+= -DSRCDIR='"${.CURDIR}"'
>  CFLAGS+= -I${.CURDIR}/../include
> -
> -REGRESS_MAXTIME?=30
> Index: regress/lib/libpthread/pthread_specific/Makefile
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/regress/lib/libpthread/pthread_specific/Makefile,v
> retrieving revision 1.2
> diff -u -p -r1.2 Makefile
> --- regress/lib/libpthread/pthread_specific/Makefile 22 Dec 2013 11:08:31 -0000 1.2
> +++ regress/lib/libpthread/pthread_specific/Makefile 14 May 2019 05:56:28 -0000
> @@ -2,6 +2,4 @@
>
>  PROG= pthread_specific
>
> -REGRESS_MAXTIME=60
> -
>  .include <bsd.regress.mk>
> Index: regress/lib/libpthread/stdarg/Makefile
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/regress/lib/libpthread/stdarg/Makefile,v
> retrieving revision 1.5
> diff -u -p -r1.5 Makefile
> --- regress/lib/libpthread/stdarg/Makefile 26 Dec 2013 16:22:55 -0000 1.5
> +++ regress/lib/libpthread/stdarg/Makefile 14 May 2019 05:56:28 -0000
> @@ -4,6 +4,4 @@ PROG= stdarg
>
>  CFLAGS+= -I${.CURDIR}/../include
>
> -REGRESS_MAXTIME=10
> -
>  .include <bsd.regress.mk>
> Index: share/man/man5/bsd.regress.mk.5
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/share/man/man5/bsd.regress.mk.5,v
> retrieving revision 1.18
> diff -u -p -r1.18 bsd.regress.mk.5
> --- share/man/man5/bsd.regress.mk.5 2 Dec 2018 11:46:31 -0000 1.18
> +++ share/man/man5/bsd.regress.mk.5 14 May 2019 05:33:37 -0000
> @@ -80,9 +80,6 @@ Points to the fully-qualified path of a
>  results are appended.
>  Defaults to
>  .Pa /dev/null .
> -.It Ev REGRESS_MAXTIME
> -Maximum limit of CPU seconds to spend on the regression test.
> -Exceeding this time will result in a failure being logged.
>  .It Ev REGRESS_ROOT_TARGETS
>  Targets for which root access is required to run the test.
>  The
> Index: share/mk/bsd.regress.mk
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/share/mk/bsd.regress.mk,v
> retrieving revision 1.17
> diff -u -p -r1.17 bsd.regress.mk
> --- share/mk/bsd.regress.mk 3 Dec 2018 22:30:04 -0000 1.17
> +++ share/mk/bsd.regress.mk 14 May 2019 05:43:43 -0000
> @@ -39,14 +39,14 @@ REGRESS_SKIP_TARGETS=run-regress-${PROG}
>  .  endif
>  .endif
>
> -.if defined(REGRESS_SLOW_TARGETS) && ${REGRESS_SKIP_SLOW} != no
> +.if defined(REGRESS_SLOW_TARGETS) && ${REGRESS_SKIP_SLOW:L} != no
>  REGRESS_SKIP_TARGETS+=${REGRESS_SLOW_TARGETS}
>  .endif
>
> -.if ${REGRESS_FAIL_EARLY} != no
> -_SKIP_FAIL=
> +.if ${REGRESS_FAIL_EARLY:L} != no
> +_REGRESS_IGNORE_FAIL=
>  .else
> -_SKIP_FAIL=-
> +_REGRESS_IGNORE_FAIL= -
>  .endif
>
>  .if defined(REGRESS_ROOT_TARGETS)
> @@ -102,26 +102,13 @@ regress: .SILENT
>   @echo -n "SKIP " ${_REGRESS_OUT}
>   @echo SKIPPED
>  .  else
> -# XXX - we need a better method to see if a test fails due to timeout or just
> -#       normal failure.
> -.   if !defined(REGRESS_MAXTIME)
> - ${_SKIP_FAIL}if cd ${.CURDIR} && ${MAKE} ${RT}; then \
> + ${_REGRESS_IGNORE_FAIL} if ${MAKE} -C ${.CURDIR} ${RT}; then \
>      echo -n "SUCCESS " ${_REGRESS_OUT} ; \
>   else \
>      echo -n "FAIL " ${_REGRESS_OUT} ; \
>      echo FAILED ; \
>      false; \
>   fi
> -.   else
> - ${_SKIP_FAIL}if cd ${.CURDIR} && \
> -    (ulimit -t ${REGRESS_MAXTIME} ; ${MAKE} ${RT}); then \
> -    echo -n "SUCCESS " ${_REGRESS_OUT} ; \
> - else \
> -    echo -n "FAIL (possible timeout) " ${_REGRESS_OUT} ; \
> -    echo FAILED ; \
> -    false; \
> - fi
> -.   endif
>  .  endif
>   @echo ${_REGRESS_NAME}/${RT:S/^run-regress-//} ${_REGRESS_OUT}
>  .endfor
>