infrastructure patch: improve test handling

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

infrastructure patch: improve test handling

Marc Espie-2
Documentation AND infra patch.

- add test to the list of things that can be rebuilt/cleaned
- recognize PORTS_PRIVSEP as a special case for automated testing,
specifically, set TEST_IS_INTERACTIVE=network  for testsuites that require
network access.



Index: bsd.port.mk.5
===================================================================
RCS file: /cvs/src/share/man/man5/bsd.port.mk.5,v
retrieving revision 1.511
diff -u -p -r1.511 bsd.port.mk.5
--- bsd.port.mk.5 31 May 2019 21:27:48 -0000 1.511
+++ bsd.port.mk.5 29 Jun 2019 12:39:47 -0000
@@ -169,7 +169,7 @@ Clean ports contents.
 By default, it will clean the work directory.
 It can be invoked as
 make clean='[depends build bulk work fake flavors dist install sub package
-packages plist]'.
+packages plist test]'.
 .Bl -tag -width packages
 .It Va work
 Clean work directory.
@@ -195,6 +195,8 @@ Uninstall package.
 Remove all copies of package file.
 .It Va plist
 Remove registered packing lists of all subpackages.
+.It Va test
+Clean test cookie.
 .It Va sub
 With
 .Va install
@@ -702,8 +704,11 @@ see
 .It Cm reprepare
 Force running the
 .Ar prepare
-target
-again.
+target again.
+.It Cm retest
+Force running the
+.Ar test
+target again.
 .It Cm show
 Invoked as make show=name, show the contents of ${name}.
 Invoked as make show="name1 name2 ...",
@@ -3015,9 +3020,18 @@ Empty by default.
 .It Ev TEST_IS_INTERACTIVE
 Set to
 .Sq Yes
-if port needs human interaction to run its tests, or set to
+if port needs human interaction to run its tests, set to
 .Sq X11
-if the tests need an active X11 display to work.
+if the tests need an active X11 display to work,
+set to
+.Sq network
+if the tests need access network
+(setting
+.Ev PORTS_PRIVSEP
+disables network access for the
+.Sq _pbuild
+user, so these tests become, in effect,
+interactive).
 .It Ev TEST_LOG
 Command used to log the results of regression tests to TEST_LOGFILE.
 Read-only.





Index: bsd.port.mk
===================================================================
RCS file: /cvs/ports/infrastructure/mk/bsd.port.mk,v
retrieving revision 1.1471
diff -u -p -r1.1471 bsd.port.mk
--- bsd.port.mk 16 Jun 2019 14:27:42 -0000 1.1471
+++ bsd.port.mk 29 Jun 2019 12:39:53 -0000
@@ -253,7 +253,7 @@ _clean += -f
 .endif
 
 _okay_words = depends work fake -f flavors dist install sub packages package \
- bulk force plist build all
+ bulk force plist build all test
 .for _w in ${_clean}
 .  if !${_okay_words:M${_w}}
 ERRORS += "Fatal: unknown clean command: ${_w}\n(not in ${_okay_words})"
@@ -928,8 +928,13 @@ TEST_LOGFILE ?= ${WRKDIR}/test.log
 TEST_LOG = | ${_PBUILD} tee ${TEST_LOGFILE}
 IS_INTERACTIVE ?= No
 TEST_IS_INTERACTIVE ?= No
+.if ${TEST_IS_INTERACTIVE:L} == "network" && ${PORTS_PRIVSEP:L} != "yes"
+_TEST_IS_INTERACTIVE = No
+.else
+_TEST_IS_INTERACTIVE = ${TEST_IS_INTERACTIVE}
+.endif
 
-.if ${TEST_IS_INTERACTIVE:L} == "x11"
+.if ${_TEST_IS_INTERACTIVE:L} == "x11"
 TEST_ENV += DISPLAY=${DISPLAY} XAUTHORITY=${XAUTHORITY}
 XAUTHORITY ?= ${HOME}/.Xauthority
 .endif
@@ -1422,9 +1427,9 @@ MISSING_FILES += ${_F}
 ################################################################
 TRY_BROKEN ?= No
 _IGNORE_TEST ?=
-.if ${TEST_IS_INTERACTIVE:L} != "no" && defined(BATCH)
+.if ${_TEST_IS_INTERACTIVE:L} != "no" && defined(BATCH)
 _IGNORE_TEST += "has interactive tests"
-.elif ${TEST_IS_INTERACTIVE:L} == "no" && defined(INTERACTIVE)
+.elif ${_TEST_IS_INTERACTIVE:L} == "no" && defined(INTERACTIVE)
 _IGNORE_TEST += "does not have interactive tests"
 .endif
 
@@ -2826,7 +2831,7 @@ ${_TEST_COOKIE}: ${_BUILD_COOKIE}
 .if ${NO_TEST:L} == "no"
  @${ECHO_MSG} "===>  Regression tests for ${FULLPKGNAME}${_MASTER}"
 # When interactive tests need X11
-.  if ${TEST_IS_INTERACTIVE:L} == "x11"
+.  if ${_TEST_IS_INTERACTIVE:L} == "x11"
 .    if !defined(DISPLAY) || !exists(${XAUTHORITY})
  @echo 1>&2 "The regression tests require a running instance of X."
  @echo 1>&2 "You will also need to set the environment variable DISPLAY"
@@ -3151,6 +3156,9 @@ _internal-clean:
 .    endfor
 .  endif
 .endif
+.if ${_clean:Mtest}
+ rm -f ${_TEST_COOKIE}
+.endif
 
 print-build-depends:
 .if !empty(_BUILD_DEP)
@@ -3473,6 +3481,10 @@ rebuild:
  @${_PBUILD} rm -f ${_BUILD_COOKIE}
  @${_MAKE} build
 
+retest:
+ @${_PBUILD} rm -f ${_TEST_COOKIE}
+ @${_MAKE} test
+
 regen:
  @${_PBUILD} rm -f ${_GEN_COOKIE}
  @${_MAKE} gen
@@ -3549,7 +3561,7 @@ _all_phony = ${_recursive_depends_target
  post-distpatch post-extract post-install \
  post-patch post-test pre-build pre-configure pre-extract pre-fake \
  pre-install pre-patch pre-test prepare \
- print-build-depends print-run-depends rebuild regen reprepare \
+ print-build-depends print-run-depends rebuild regen reprepare retest \
  test-depends test-depends-list run-depends-list \
     show-required-by subpackage uninstall _print-metadata \
  run-depends-args lib-depends-args all-lib-depends-args wantlib-args \

Reply | Threaded
Open this post in threaded view
|

Re: infrastructure patch: improve test handling

Klemens Nanni-2
On Sat, Jun 29, 2019 at 02:43:37PM +0200, Marc Espie wrote:
> - add test to the list of things that can be rebuilt/cleaned
> - recognize PORTS_PRIVSEP as a special case for automated testing,
> specifically, set TEST_IS_INTERACTIVE=network  for testsuites that require
> network access.
These make sense but I have yet to test them - I started looking into
more convenient clean targets just earlier this day :)

> @@ -3015,9 +3020,18 @@ Empty by default.
>  .It Ev TEST_IS_INTERACTIVE
>  Set to
>  .Sq Yes
> -if port needs human interaction to run its tests, or set to
> +if port needs human interaction to run its tests, set to
>  .Sq X11
> -if the tests need an active X11 display to work.
> +if the tests need an active X11 display to work,
> +set to
> +.Sq network
> +if the tests need access network
Either "network access" or "access to network", but I suggest the former.

> +(setting
> +.Ev PORTS_PRIVSEP
> +disables network access for the
> +.Sq _pbuild
> +user, so these tests become, in effect,
> +interactive).
>  .It Ev TEST_LOG
>  Command used to log the results of regression tests to TEST_LOGFILE.
>  Read-only.

Reply | Threaded
Open this post in threaded view
|

Re: infrastructure patch: improve test handling

Kurt Mosiejczuk-9
On Sat, Jun 29, 2019 at 02:57:11PM +0200, Klemens Nanni wrote:
> On Sat, Jun 29, 2019 at 02:43:37PM +0200, Marc Espie wrote:
> > - add test to the list of things that can be rebuilt/cleaned
> > - recognize PORTS_PRIVSEP as a special case for automated testing,
> > specifically, set TEST_IS_INTERACTIVE=network  for testsuites that require
> > network access.
> These make sense but I have yet to test them - I started looking into
> more convenient clean targets just earlier this day :)

The "test=clean" and "retest" targets work great for me. I like them.

> > @@ -3015,9 +3020,18 @@ Empty by default.
> >  .It Ev TEST_IS_INTERACTIVE
> >  Set to
> >  .Sq Yes
> > -if port needs human interaction to run its tests, or set to
> > +if port needs human interaction to run its tests, set to
> >  .Sq X11
> > -if the tests need an active X11 display to work.
> > +if the tests need an active X11 display to work,
> > +set to
> > +.Sq network
> > +if the tests need access network
> Either "network access" or "access to network", but I suggest the former.

I agree on this.

I tested TEST_IS_INTERACTIVE=network with www/py-requests, and it seems to
just run the tests even though I run with PORTS_PRIVSEP=Yes.

superhell$ make show=TEST_IS_INTERACTIVE
network
superhell$ vi Makefile                                                        
superhell$ make show=PORTS_PRIVSEP
Yes
superhell$ make show=_TEST_IS_INTERACTIVE
network
superhell$ make show=_IGNORE_TEST        

superhell$

--Kurt

Reply | Threaded
Open this post in threaded view
|

Re: infrastructure patch: improve test handling

Marc Espie-2
On Sat, Jun 29, 2019 at 01:07:45PM -0400, Kurt Mosiejczuk wrote:

> On Sat, Jun 29, 2019 at 02:57:11PM +0200, Klemens Nanni wrote:
> > On Sat, Jun 29, 2019 at 02:43:37PM +0200, Marc Espie wrote:
> > > - add test to the list of things that can be rebuilt/cleaned
> > > - recognize PORTS_PRIVSEP as a special case for automated testing,
> > > specifically, set TEST_IS_INTERACTIVE=network  for testsuites that require
> > > network access.
> > These make sense but I have yet to test them - I started looking into
> > more convenient clean targets just earlier this day :)
>
> The "test=clean" and "retest" targets work great for me. I like them.
>
> > > @@ -3015,9 +3020,18 @@ Empty by default.
> > >  .It Ev TEST_IS_INTERACTIVE
> > >  Set to
> > >  .Sq Yes
> > > -if port needs human interaction to run its tests, or set to
> > > +if port needs human interaction to run its tests, set to
> > >  .Sq X11
> > > -if the tests need an active X11 display to work.
> > > +if the tests need an active X11 display to work,
> > > +set to
> > > +.Sq network
> > > +if the tests need access network
> > Either "network access" or "access to network", but I suggest the former.
>
> I agree on this.
>
> I tested TEST_IS_INTERACTIVE=network with www/py-requests, and it seems to
> just run the tests even though I run with PORTS_PRIVSEP=Yes.
>
> superhell$ make show=TEST_IS_INTERACTIVE
> network
> superhell$ vi Makefile                                                        
> superhell$ make show=PORTS_PRIVSEP
> Yes
> superhell$ make show=_TEST_IS_INTERACTIVE
> network
> superhell$ make show=_IGNORE_TEST        

RTFM
*INTERACTIVE stuff does not do anything special by default, unless you
trigger it, e.g., see BATCH and INTERACTIVE in bsd.port.mk(5)

(that's also somewhat obvious from the patch)

Reply | Threaded
Open this post in threaded view
|

Re: infrastructure patch: improve test handling

Kurt Mosiejczuk-9
On Sat, Jun 29, 2019 at 09:12:12PM +0200, Marc Espie wrote:

> RTFM
> *INTERACTIVE stuff does not do anything special by default, unless you
> trigger it, e.g., see BATCH and INTERACTIVE in bsd.port.mk(5)

> (that's also somewhat obvious from the patch)

You say it does nothing by default, but before and after the patch if
TEST_IS_INTERACTIVE="x11", and there is no DISPLAY, it throws an error
message and doesn't run the test.

superhell$ grep TEST_IS_INTERACTIVE Makefile
TEST_IS_INTERACTIVE=x11
superhell$ echo $DISPLAY                    

superhell$ echo $XAUTHORITY                  

superhell$ make test                        
===>  Regression tests for itk-3.3p5
The regression tests require a running instance of X.
You will also need to set the environment variable DISPLAY
to point to an active X11 display and XAUTHORITY to point
to the appropriate .Xauthority file.
*** Error 1 in . (/home/ports/infrastructure/mk/bsd.port.mk:2840 '/home/ports/pobj/itk-3.3/.test_done': @exit 1)
*** Error 1 in /usr/ports/x11/itk (/home/ports/infrastructure/mk/bsd.port.mk:2486 'test')
superhell$

Also, the man page says BATCH is to use in conjunction with INTERACTIVE.
Nothing about *INTERACTIVE, just INTERACTIVE. BATCH also just talks
about builds, not testing. TEST_IS_INTERACTIVE does not mention BATCH.

It's also not obvious from the patch, since after the patch,
TEST_IS_INTERACTIVE=x11 acts the same as before the patch, which stops
the tests.

The man page really doesn't indicate what TEST_IS_INTERACTIVE does, in fact.

I'm more than happy to be told to RTFM when I miss something obvious, but
this is not one of those cases.

--Kurt

Reply | Threaded
Open this post in threaded view
|

Re: infrastructure patch: improve test handling

Jeremie Courreges-Anglas-5
In reply to this post by Marc Espie-2
On Sat, Jun 29 2019, Marc Espie <[hidden email]> wrote:
> Documentation AND infra patch.
>
> - add test to the list of things that can be rebuilt/cleaned

This part needs a fix for PORTS_PRIVSEP=Yes, please see below.

> - recognize PORTS_PRIVSEP as a special case for automated testing,
> specifically, set TEST_IS_INTERACTIVE=network  for testsuites that require
> network access.

Not reviewed / tested yet.

>
>
> Index: bsd.port.mk.5
> ===================================================================
> RCS file: /cvs/src/share/man/man5/bsd.port.mk.5,v
> retrieving revision 1.511
> diff -u -p -r1.511 bsd.port.mk.5
> --- bsd.port.mk.5 31 May 2019 21:27:48 -0000 1.511
> +++ bsd.port.mk.5 29 Jun 2019 12:39:47 -0000
> @@ -169,7 +169,7 @@ Clean ports contents.
>  By default, it will clean the work directory.
>  It can be invoked as
>  make clean='[depends build bulk work fake flavors dist install sub package
> -packages plist]'.
> +packages plist test]'.
>  .Bl -tag -width packages
>  .It Va work
>  Clean work directory.
> @@ -195,6 +195,8 @@ Uninstall package.
>  Remove all copies of package file.
>  .It Va plist
>  Remove registered packing lists of all subpackages.
> +.It Va test
> +Clean test cookie.
>  .It Va sub
>  With
>  .Va install
> @@ -702,8 +704,11 @@ see
>  .It Cm reprepare
>  Force running the
>  .Ar prepare
> -target
> -again.
> +target again.
> +.It Cm retest
> +Force running the
> +.Ar test
> +target again.
>  .It Cm show
>  Invoked as make show=name, show the contents of ${name}.
>  Invoked as make show="name1 name2 ...",
> @@ -3015,9 +3020,18 @@ Empty by default.
>  .It Ev TEST_IS_INTERACTIVE
>  Set to
>  .Sq Yes
> -if port needs human interaction to run its tests, or set to
> +if port needs human interaction to run its tests, set to
>  .Sq X11
> -if the tests need an active X11 display to work.
> +if the tests need an active X11 display to work,
> +set to
> +.Sq network
> +if the tests need access network
> +(setting
> +.Ev PORTS_PRIVSEP
> +disables network access for the
> +.Sq _pbuild
> +user, so these tests become, in effect,
> +interactive).
>  .It Ev TEST_LOG
>  Command used to log the results of regression tests to TEST_LOGFILE.
>  Read-only.
>
>
>
>
>
> Index: bsd.port.mk
> ===================================================================
> RCS file: /cvs/ports/infrastructure/mk/bsd.port.mk,v
> retrieving revision 1.1471
> diff -u -p -r1.1471 bsd.port.mk
> --- bsd.port.mk 16 Jun 2019 14:27:42 -0000 1.1471
> +++ bsd.port.mk 29 Jun 2019 12:39:53 -0000
> @@ -253,7 +253,7 @@ _clean += -f
>  .endif
>  
>  _okay_words = depends work fake -f flavors dist install sub packages package \
> - bulk force plist build all
> + bulk force plist build all test
>  .for _w in ${_clean}
>  .  if !${_okay_words:M${_w}}
>  ERRORS += "Fatal: unknown clean command: ${_w}\n(not in ${_okay_words})"
> @@ -928,8 +928,13 @@ TEST_LOGFILE ?= ${WRKDIR}/test.log
>  TEST_LOG = | ${_PBUILD} tee ${TEST_LOGFILE}
>  IS_INTERACTIVE ?= No
>  TEST_IS_INTERACTIVE ?= No
> +.if ${TEST_IS_INTERACTIVE:L} == "network" && ${PORTS_PRIVSEP:L} != "yes"
> +_TEST_IS_INTERACTIVE = No
> +.else
> +_TEST_IS_INTERACTIVE = ${TEST_IS_INTERACTIVE}
> +.endif
>  
> -.if ${TEST_IS_INTERACTIVE:L} == "x11"
> +.if ${_TEST_IS_INTERACTIVE:L} == "x11"
>  TEST_ENV += DISPLAY=${DISPLAY} XAUTHORITY=${XAUTHORITY}
>  XAUTHORITY ?= ${HOME}/.Xauthority
>  .endif
> @@ -1422,9 +1427,9 @@ MISSING_FILES += ${_F}
>  ################################################################
>  TRY_BROKEN ?= No
>  _IGNORE_TEST ?=
> -.if ${TEST_IS_INTERACTIVE:L} != "no" && defined(BATCH)
> +.if ${_TEST_IS_INTERACTIVE:L} != "no" && defined(BATCH)
>  _IGNORE_TEST += "has interactive tests"
> -.elif ${TEST_IS_INTERACTIVE:L} == "no" && defined(INTERACTIVE)
> +.elif ${_TEST_IS_INTERACTIVE:L} == "no" && defined(INTERACTIVE)
>  _IGNORE_TEST += "does not have interactive tests"
>  .endif
>  
> @@ -2826,7 +2831,7 @@ ${_TEST_COOKIE}: ${_BUILD_COOKIE}
>  .if ${NO_TEST:L} == "no"
>   @${ECHO_MSG} "===>  Regression tests for ${FULLPKGNAME}${_MASTER}"
>  # When interactive tests need X11
> -.  if ${TEST_IS_INTERACTIVE:L} == "x11"
> +.  if ${_TEST_IS_INTERACTIVE:L} == "x11"
>  .    if !defined(DISPLAY) || !exists(${XAUTHORITY})
>   @echo 1>&2 "The regression tests require a running instance of X."
>   @echo 1>&2 "You will also need to set the environment variable DISPLAY"
> @@ -3151,6 +3156,9 @@ _internal-clean:
>  .    endfor
>  .  endif
>  .endif
> +.if ${_clean:Mtest}
> + rm -f ${_TEST_COOKIE}

Should be

> + ${_PBUILD} rm -f ${_TEST_COOKIE}

else I get an error with make clean=test:

russell /usr/ports/x11/ratpoison$ make clean=test
===>  Cleaning for ratpoison-1.4.9
rm -f /usr/ports/pobj/ratpoison-1.4.9/build-amd64/.test_done
rm: /usr/ports/pobj/ratpoison-1.4.9/build-amd64/.test_done: Permission denied
*** Error 1 in . (/usr/ports/infrastructure/mk/bsd.port.mk:3160 '_internal-clean')
*** Error 1 in /usr/ports/x11/ratpoison (/usr/ports/infrastructure/mk/bsd.port.mk:2486 'clean')

ok for this part of your diff (with ${_PBUILD} added).  I think it's
a worthwhile addition.  Thanks!

> +.endif
>  
>  print-build-depends:
>  .if !empty(_BUILD_DEP)
> @@ -3473,6 +3481,10 @@ rebuild:
>   @${_PBUILD} rm -f ${_BUILD_COOKIE}
>   @${_MAKE} build
>  
> +retest:
> + @${_PBUILD} rm -f ${_TEST_COOKIE}
> + @${_MAKE} test
> +
>  regen:
>   @${_PBUILD} rm -f ${_GEN_COOKIE}
>   @${_MAKE} gen
> @@ -3549,7 +3561,7 @@ _all_phony = ${_recursive_depends_target
>   post-distpatch post-extract post-install \
>   post-patch post-test pre-build pre-configure pre-extract pre-fake \
>   pre-install pre-patch pre-test prepare \
> - print-build-depends print-run-depends rebuild regen reprepare \
> + print-build-depends print-run-depends rebuild regen reprepare retest \
>   test-depends test-depends-list run-depends-list \
>      show-required-by subpackage uninstall _print-metadata \
>   run-depends-args lib-depends-args all-lib-depends-args wantlib-args \
>


--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE