[ovs-dev,v2,3/3] tests/automake.mk: Prohibition of parallel system-traffic test execution.

On Wed, Mar 02, 2016 at 08:25:41AM +0300, Ilya Maximets wrote:
> 
> 
> On 01.03.2016 19:08, Ben Pfaff wrote:
> > On Tue, Mar 01, 2016 at 08:31:43AM +0300, Ilya Maximets wrote:
> >> On 29.02.2016 21:44, Ben Pfaff wrote:
> >>> On Mon, Feb 29, 2016 at 04:06:52PM +0300, Ilya Maximets wrote:
> >>>> 'make check-system-userspace', 'make check-kernel' and 'make check-kmod'
> >>>> work with real environment and can not be run simultaneously.
> >>>>
> >>>> To prevent violation of the system environment strip out options for
> >>>> parallel execution from TESTSUITEFLAGS for this targets.
> >>>>
> >>>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> >>>
> >>> TESTSUITEFLAGS_WITHOUT_JOBS can be more simply defined as just
> >>> $(filter-out -j% --jobs=%, $(TESTSUITEFLAGS))
> >>
> >> This can't filter out something like '-j 8' because of whitespaces inside
> >> the pattern. Anyway, unfortunately, I found that this all is a GNU
> >> extensions for make and we can't use them for portability reasons.
> >> '$(filter-out', '$(shell' and even 'define' should be replaced with
> >> another portable implementation.
> > 
> > OVS requires GNU make, see INSTALL.md.
> 
> Sorry, I forgot about it.
> 
> > 
> >>> Do you think it's really worth warning about this?
> >>
> >> It's very confusing that I can use usual 'make check' in parallel mode,
> >> but can't do the same with other testsuites. In addition, this
> >> behaviour isn't documented at all.
> > 
> > I think it's worth disabling parallelism, I'm just not sure it's worth
> > warning.
> 
> Thanks for clarifying.
> May be it isn't. I'll prepare v4 without warning.
> 
> About filtering method:
> As I already told, 'filter-out' can't remove parameters with whitespaces
> inside (Ex. '-j 8').
> So, there are 2 options:
> 1. TESTSUITEFLAGS_WITHOUT_JOBS = $(shell echo '$(TESTSUITEFLAGS)' | sed -r 's/(^| )(-j|--jobs=) *[0-9]+//g')
> 2. TESTSUITEFLAGS_WITHOUT_JOBS = `echo '$(TESTSUITEFLAGS)' | sed -r 's/(^| )(-j|--jobs=) *[0-9]+//g'`
> 
> In second case will be printed full command line with 'echo' and 'sed' like this:
> /bin/sh './tests/system-userspace-testsuite' -C tests  AUTOTEST_PATH='...' `echo '-j8' | sed -r 's/(^| )(-j|--jobs=) *[0-9]+//g'`
> In first case result of filtering will be displayed.
> 
> Which one is better in your opinion? Another solutions are welcome.

How about this:

On 02.03.2016 22:28, Ben Pfaff wrote:
> How about this:
> 
> diff --git a/tests/automake.mk b/tests/automake.mk
> index 592f648..3c5c848 100644
> --- a/tests/automake.mk
> +++ b/tests/automake.mk
> @@ -209,7 +209,7 @@ EXTRA_DIST += tests/run-ryu
>  
>  # Run kmod tests. Assume kernel modules has been installed or linked into the kernel
>  check-kernel: all tests/atconfig tests/atlocal $(SYSTEM_KMOD_TESTSUITE)
> -	$(SHELL) '$(SYSTEM_KMOD_TESTSUITE)' -C tests  AUTOTEST_PATH='$(AUTOTEST_PATH)' -d $(TESTSUITEFLAGS)
> +	$(SHELL) '$(SYSTEM_KMOD_TESTSUITE)' -C tests  AUTOTEST_PATH='$(AUTOTEST_PATH)' -d $(TESTSUITEFLAGS) -j1
>  
>  # Testing the out of tree Kernel module
>  check-kmod: all tests/atconfig tests/atlocal $(SYSTEM_KMOD_TESTSUITE)
> @@ -218,7 +218,7 @@ check-kmod: all tests/atconfig tests/atlocal $(SYSTEM_KMOD_TESTSUITE)
>  	$(MAKE) check-kernel
>  
>  check-system-userspace: all tests/atconfig tests/atlocal $(SYSTEM_USERSPACE_TESTSUITE)
> -	$(SHELL) '$(SYSTEM_USERSPACE_TESTSUITE)' -C tests  AUTOTEST_PATH='$(AUTOTEST_PATH)' $(TESTSUITEFLAGS)
> +	$(SHELL) '$(SYSTEM_USERSPACE_TESTSUITE)' -C tests  AUTOTEST_PATH='$(AUTOTEST_PATH)' $(TESTSUITEFLAGS) -j1
>  
>  clean-local:
>  	test ! -f '$(TESTSUITE)' || $(SHELL) '$(TESTSUITE)' -C tests --clean

It's a dirty hack, but elegant. Thanks.

Will you format this as a patch, or should I post v4?

Best regards, Ilya Maximets.

On Thu, Mar 03, 2016 at 05:01:53PM +0300, Ilya Maximets wrote:
> On 02.03.2016 22:28, Ben Pfaff wrote:
> > How about this:
> > 
> > diff --git a/tests/automake.mk b/tests/automake.mk
> > index 592f648..3c5c848 100644
> > --- a/tests/automake.mk
> > +++ b/tests/automake.mk
> > @@ -209,7 +209,7 @@ EXTRA_DIST += tests/run-ryu
> >  
> >  # Run kmod tests. Assume kernel modules has been installed or linked into the kernel
> >  check-kernel: all tests/atconfig tests/atlocal $(SYSTEM_KMOD_TESTSUITE)
> > -	$(SHELL) '$(SYSTEM_KMOD_TESTSUITE)' -C tests  AUTOTEST_PATH='$(AUTOTEST_PATH)' -d $(TESTSUITEFLAGS)
> > +	$(SHELL) '$(SYSTEM_KMOD_TESTSUITE)' -C tests  AUTOTEST_PATH='$(AUTOTEST_PATH)' -d $(TESTSUITEFLAGS) -j1
> >  
> >  # Testing the out of tree Kernel module
> >  check-kmod: all tests/atconfig tests/atlocal $(SYSTEM_KMOD_TESTSUITE)
> > @@ -218,7 +218,7 @@ check-kmod: all tests/atconfig tests/atlocal $(SYSTEM_KMOD_TESTSUITE)
> >  	$(MAKE) check-kernel
> >  
> >  check-system-userspace: all tests/atconfig tests/atlocal $(SYSTEM_USERSPACE_TESTSUITE)
> > -	$(SHELL) '$(SYSTEM_USERSPACE_TESTSUITE)' -C tests  AUTOTEST_PATH='$(AUTOTEST_PATH)' $(TESTSUITEFLAGS)
> > +	$(SHELL) '$(SYSTEM_USERSPACE_TESTSUITE)' -C tests  AUTOTEST_PATH='$(AUTOTEST_PATH)' $(TESTSUITEFLAGS) -j1
> >  
> >  clean-local:
> >  	test ! -f '$(TESTSUITE)' || $(SHELL) '$(TESTSUITE)' -C tests --clean
> 
> It's a dirty hack, but elegant. Thanks.
> 
> Will you format this as a patch, or should I post v4?

Please post v4, if you don't mind.