On Mon, Aug 29, 2016 at 6:03 PM, Lukas Slebodnik <[email protected]> wrote: > On (29/08/16 16:38), Lukas Slebodnik wrote: >>On (29/08/16 16:05), Fabiano Fidêncio wrote: >>>On Mon, Aug 29, 2016 at 3:51 PM, Lukas Slebodnik <[email protected]> wrote: >>>> On (29/08/16 14:47), Lukas Slebodnik wrote: >>>>>On (29/08/16 14:36), Fabiano Fidêncio wrote: >>>>>>On Mon, Aug 29, 2016 at 2:27 PM, Lukas Slebodnik <[email protected]> >>>>>>wrote: >>>>>>> On (29/08/16 14:15), Fabiano Fidêncio wrote: >>>>>>>>On Mon, Aug 29, 2016 at 12:36 PM, Lukas Slebodnik <[email protected]> >>>>>>>>wrote: >>>>>>>>> On (29/08/16 12:08), Fabiano Fidêncio wrote: >>>>>>>>>>On Mon, Aug 29, 2016 at 11:12 AM, Jakub Hrozek <[email protected]> >>>>>>>>>>wrote: >>>>>>>>>>> On Mon, Aug 29, 2016 at 10:38:46AM +0200, Lukas Slebodnik wrote: >>>>>>>>>>>> On (29/08/16 07:09), Fabiano Fidêncio wrote: >>>>>>>>>>>> >Hoiwdy! >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> >On Fri, Aug 19, 2016 at 1:08 AM, Fabiano Fidêncio >>>>>>>>>>>> ><[email protected]> wrote: >>>>>>>>>>>> >> This patch is a first attempt to make "make intgcheck" less >>>>>>>>>>>> >> painful/time consuming than it is now. >>>>>>>>>>>> >> >>>>>>>>>>>> >> Although the patch provides a good improvement on having 5 new >>>>>>>>>>>> >> targets, I know it's still not ideal. The ideal case, IMO, >>>>>>>>>>>> >> would be >>>>>>>>>>>> >> being able to select which subset of tests would be run, but >>>>>>>>>>>> >> that's an >>>>>>>>>>>> >> improvement that can be done later on. >>>>>>>>>>>> >> >>>>>>>>>>>> >> Tips are welcome. >>>>>>>>>>>> >> >>>>>>>>>>>> >> Best Regards, >>>>>>>>>>>> > >>>>>>>>>>>> >After a few dicussions and some tests done when we met >>>>>>>>>>>> >personally, I >>>>>>>>>>>> >found out that the patch was broken. >>>>>>>>>>>> >Taking Lukaš suggestion (almost) I've merged the -prepare and >>>>>>>>>>>> >-configure parts and re-worked the way we get the prefix. >>>>>>>>>>>> > >>>>>>>>>>>> >Now it seems to be working! >>>>>>>>>>>> > >>>>>>>>>>>> >Best Regards, >>>>>>>>>>>> >-- >>>>>>>>>>>> >Fabiano Fidêncio >>>>>>>>>>>> >>>>>>>>>>>> >From a33e1dffd063845e709f5fc1cfec93c330445ab4 Mon Sep 17 00:00:00 >>>>>>>>>>>> >2001 >>>>>>>>>>>> >From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <[email protected]> >>>>>>>>>>>> >Date: Thu, 18 Aug 2016 16:24:17 +0200 >>>>>>>>>>>> >Subject: [PATCH v2] BUILD: Add a few more targets for intg tests >>>>>>>>>>>> >MIME-Version: 1.0 >>>>>>>>>>>> >Content-Type: text/plain; charset=UTF-8 >>>>>>>>>>>> >Content-Transfer-Encoding: 8bit >>>>>>>>>>>> > >>>>>>>>>>>> >Running "make intgcheck" has been proven to be a bit painful >>>>>>>>>>>> >(mainly >>>>>>>>>>>> >when the developer is just writing down a single test case), as it >>>>>>>>>>>> >cleans up the build directory and fireis a new build before, >>>>>>>>>>>> >finally, >>>>>>>>>>>> >run the tests. >>>>>>>>>>>> > >>>>>>>>>>>> >In order to make it a little less painful, let's break the whole >>>>>>>>>>>> >operation into 4 new targets: >>>>>>>>>>>> > intgcheck-{prepare,build,run,clean}. >>>>>>>>>>>> > >>>>>>>>>>>> >As expected, "make intgcheck" calls these 4 new operations in the >>>>>>>>>>>> >same >>>>>>>>>>>> >order they were presented, not changing then the current behavior. >>>>>>>>>>>> > >>>>>>>>>>>> >Each operation will trigger the previous one in case there is no >>>>>>>>>>>> >"$$prefix" directory created and the directory is _only_ created >>>>>>>>>>>> >in the >>>>>>>>>>>> >very first operation (intghceck-prepare). >>>>>>>>>>>> > >>>>>>>>>>>> >Signed-off-by: Fabiano Fidêncio <[email protected]> >>>>>>>>>>>> >--- >>>>>>>>>>>> > Makefile.am | 52 >>>>>>>>>>>> > ++++++++++++++++++++++++++++++++++++---------------- >>>>>>>>>>>> > 1 file changed, 36 insertions(+), 16 deletions(-) >>>>>>>>>>>> > >>>>>>>>>>>> >diff --git a/Makefile.am b/Makefile.am >>>>>>>>>>>> >index 30d874e..8372b92 100644 >>>>>>>>>>>> >--- a/Makefile.am >>>>>>>>>>>> >+++ b/Makefile.am >>>>>>>>>>>> >@@ -3076,30 +3076,50 @@ endif >>>>>>>>>>>> > # Integration tests # >>>>>>>>>>>> > ##################### >>>>>>>>>>>> > >>>>>>>>>>>> >-intgcheck: >>>>>>>>>>>> >+intgcheck-prepare: >>>>>>>>>>>> > echo "temporarily disabled" >>>>>>>>>>>> > set -e; \ >>>>>>>>>>>> >- rm -Rf intg; \ >>>>>>>>>>>> >- $(MKDIR_P) intg/bld; \ >>>>>>>>>>>> >- : Use /hopefully/ short prefix to keep D-Bus socket path >>>>>>>>>>>> >short; \ >>>>>>>>>>>> >- prefix=`mktemp --tmpdir --directory sssd-intg.XXXXXXXX`; \ >>>>>>>>>>>> >- $(LN_S) "$$prefix" intg/pfx; \ >>>>>>>>>>>> >- cd intg/bld; \ >>>>>>>>>>>> >+ rm -Rf intg ; \ >>>>>>>>>>>> >+ $(MKDIR_P) intg/bld ; \ >>>>>>>>>>>> >+ : Use /hopefully/ short prefix to keep D-Bus socket path >>>>>>>>>>>> >short ; \ >>>>>>>>>>>> >+ prefix=`mktemp --tmpdir --directory sssd-intg.XXXXXXXX` ; \ >>>>>>>>>>>> >+ $(LN_S) "$$prefix" intg/pfx ; \ >>>>>>>>>>>> >+ cd intg/bld ; \ >>>>>>>>>>>> > $(abs_top_srcdir)/configure \ >>>>>>>>>>>> >- --prefix="$$prefix" \ >>>>>>>>>>>> >+ --prefix=$$prefix \ >>>>>>>>>>>> > --with-ldb-lib-dir="$$prefix"/lib/ldb \ >>>>>>>>>>>> > --enable-intgcheck-reqs \ >>>>>>>>>>>> > --without-semanage \ >>>>>>>>>>>> >- $(INTGCHECK_CONFIGURE_FLAGS); \ >>>>>>>>>>>> >- $(MAKE) $(AM_MAKEFLAGS); \ >>>>>>>>>>>> >- : Force single-thread install to workaround concurrency >>>>>>>>>>>> >issues; \ >>>>>>>>>>>> >- $(MAKE) $(AM_MAKEFLAGS) -j1 install; \ >>>>>>>>>>>> >- : Remove .la files from LDB module directory to avoid loader >>>>>>>>>>>> >warnings; \ >>>>>>>>>>>> >- rm "$$prefix"/lib/ldb/*.la; \ >>>>>>>>>>>> >- $(MAKE) $(AM_MAKEFLAGS) -C src/tests/intg >>>>>>>>>>>> >intgcheck-installed; \ >>>>>>>>>>>> >- cd ../..; \ >>>>>>>>>>>> >+ $(INTGCHECK_CONFIGURE_FLAGS) ; \ >>>>>>>>>>>> >+ cd ../.. >>>>>>>>>>>> >+ >>>>>>>>>>>> >+intgcheck-build: >>>>>>>>>>>> >+ if [ ! -d intg/pfx ]; then $(MAKE) intgcheck-prepare; fi ; \ >>>>>>>>>>>> >+ prefix=`readlink -e intg/pfx` ; \ >>>>>>>>>>>> >+ cd intg/bld ; \ >>>>>>>>>>>> >+ $(MAKE) $(AM_MAKEFLAGS) ; \ >>>>>>>>>>>> >+ : Force single-thread install to workaround concurrency >>>>>>>>>>>> >issues ; \ >>>>>>>>>>>> >+ $(MAKE) $(AM_MAKEFLAGS) -j1 install ; \ >>>>>>>>>>>> >+ : Remove .la files from LDB module directory to avoid loader >>>>>>>>>>>> >warnings ; \ >>>>>>>>>>>> >+ rm "$$prefix"/lib/ldb/*.la ; \ >>>>>>>>>>>> >+ cd ../.. >>>>>>>>>>>> >+ >>>>>>>>>>>> >+intgcheck-run: >>>>>>>>>>>> >+ if [ ! -d intg/pfx ]; then $(MAKE) intgcheck-build; fi ; \ >>>>>>>>>>>> >+ cd intg/bld ; \ >>>>>>>>>>>> >+ $(MAKE) $(AM_MAKEFLAGS) -C src/tests/intg >>>>>>>>>>>> >intgcheck-installed ; \ >>>>>>>>>>>> >+ cd ../.. >>>>>>>>>>>> >+ >>>>>>>>>>>> >+intgcheck-clean: >>>>>>>>>>>> >+ prefix=`readlink -e intg/pfx` ; \ >>>>>>>>>>>> > rm -Rf "$$prefix" intg >>>>>>>>>>>> > >>>>>>>>>>>> >+intgcheck: >>>>>>>>>>>> >+ $(MAKE) intgcheck-prepare >>>>>>>>>>>> >+ $(MAKE) intgcheck-build >>>>>>>>>>>> I would merge intgcheck-prepare and intgcheck-build bas well. >>>>>>>>>> >>>>>>>>>>Can be done. But if that's not a mandatory requirement I still think >>>>>>>>>>it may be useful for someone else in the way it's split. >>>>>>>>>> >>>>>>>>> "prepare" and "build" it's the same for me. >>>>>>>>> It is a prerequisity for >>>>>>>>> <userstory> >>>>>>>>> As a developer, I need a way to run a single test without >>>>>>>>> re-compiling SSSD every time and waiting for all the tests to >>>>>>>>> finish. >>>>>>>>> </userstory> >>>>>>>> >>>>>>>>Okay. I've merged both and kept the "-build" name. >>>>>>>> >>>>>>> "-build" is a little bit confusing IMHO. >>>>>>> Because the target not only build sssd with custom prefix but also >>>>>>> *install* sssd. So prepare might be more appropriate. >>>>>>> >>>>>>> Sorry for nitpicking and fell free to propose different name. >>>>>> >>>>>>s/build/prepare done. >>>>>>Please, see the attached patch. >>>>>> >>>>>>Being completely honest here, I don't like the idea of not having a >>>>>>step that builds and installs. >>>>>>Let's see if someone complains in the future, then we can end up >>>>>>re-adding this extra step. >>>>>> >>>>>Agree, it might change in future when more people will try to write >>>>>tests. >>>>> >>>>>Personally, I will use just a intgcheck-prepare >>>>>then I will jump into intg/bld and then I will use >>>>>ordinary make targets (all, check, install, clean) >>>>> >>>>>Other intgcheck-* targets might be usefull for others >>>>>or they can be a considered as a backword compatibility >>>>>for CI script. >>>>> >>>>>>From 33e66d12b39ef1c4fa6e23b38a644bdbafa6b7e9 Mon Sep 17 00:00:00 2001 >>>>>>From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <[email protected]> >>>>>>Date: Thu, 18 Aug 2016 16:24:17 +0200 >>>>>>Subject: [PATCH v4] BUILD: Add a few more targets for intg tests >>>>>>MIME-Version: 1.0 >>>>>>Content-Type: text/plain; charset=UTF-8 >>>>>>Content-Transfer-Encoding: 8bit >>>>>> >>>>>>Running "make intgcheck" has been proven to be a bit painful (mainly >>>>>>when the developer is just writing down a single test case), as it >>>>>>cleans up the build directory and fireis a new build before, finally, >>>>>>run the tests. >>>>>> >>>>>>In order to make it a little less painful, let's break the whole >>>>>>operation into 3 new targets: >>>>>> intgcheck-{prepare,run,clean}. >>>>>> >>>>>>As expected, "make intgcheck" calls these 3 new operations in the same >>>>>>order they were presented, not changing then the current behavior. >>>>>> >>>>>>Each operation will trigger the previous one in case there is no >>>>>>"$$prefix" directory created and the directory is _only_ created in the >>>>>>very first operation (intghcheck-prepare). >>>>>> >>>>>>A note must be done about how to run a simple test file or a simple test >>>>>>from a test file when running "make intgcheck-run". The option always >>>>>>been here but only makes sense now that we have the intgcheck split in a >>>>>>few useful steps. See the examples below (and for more detailed >>>>>>information, check the py.test documentation): >>>>>> #Run a single file >>>>>> INTGCHECK_PYTEST_ARGS="-k test_netgroup.py" make intgcheck-run >>>>>> #Run a single test from a single file >>>>>> INTGCHECK_PYTEST_ARGS="test_netgroup.py -k test_add_empty_netgroup" make >>>>>> intgcheck-run >>>>>> >>>>>>Signed-off-by: Fabiano Fidêncio <[email protected]> >>>>>>--- >>>>>> Makefile.am | 17 +++++++++++++++-- >>>>>> 1 file changed, 15 insertions(+), 2 deletions(-) >>>>>> >>>>>ACK. >>>>> >>>>>Just waiting for Ci results. We do not want to break them :-) >>>>> >>>> http://sssd-ci.duckdns.org/logs/job/52/62/summary.html >>>> >>>> master: >>>> * 6159c33125f8ee82e88d495ea2aa5d00018ea844 >>>> >>>> And I would like to keep workflow the same for stable branch >>>> >>>> sssd-1-13: >>>> * 3bfe059a2af7e839ef9b54961403ddffa51f77f7 >>> >>>Ouch, seems that one typo was not caught during the review/CI. >>>Sorry about that but we will also need to push the attached patch for >>>both master and 1-13. >>> >>>> >>>> LS >>>> _______________________________________________ >>>> sssd-devel mailing list >>>> [email protected] >>>> https://lists.fedorahosted.org/admin/lists/[email protected] >> >>>From 711e8c3cfca88e1431b6f667946c901f02665b31 Mon Sep 17 00:00:00 2001 >>>From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <[email protected]> >>>Date: Mon, 29 Aug 2016 16:01:59 +0200 >>>Subject: [PATCH] BUILD: Fix typo in intgcheck-run rule >>>MIME-Version: 1.0 >>>Content-Type: text/plain; charset=UTF-8 >>>Content-Transfer-Encoding: 8bit >>> >>>During the review process "intgcheck-build" ended up being merged to the >>>"intgcheck-prepare" rule. >>> >>>Signed-off-by: Fabiano Fidêncio <[email protected]> >>>--- >>> Makefile.am | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>>diff --git a/Makefile.am b/Makefile.am >>>index 4a56d8b..25a81b9 100644 >>>--- a/Makefile.am >>>+++ b/Makefile.am >>>@@ -3099,7 +3099,7 @@ intgcheck-prepare: >>> cd ../.. >>> >>> intgcheck-run: >>>- if [ ! -d intg/pfx ]; then $(MAKE) intgcheck-build; fi; \ >>>+ if [ ! -d intg/pfx ]; then $(MAKE) intgcheck-prepare; fi; \ >>> cd intg/bld; \ >>> $(MAKE) $(AM_MAKEFLAGS) -C src/tests/intg intgcheck-installed; \ >>> cd ../.. >>>-- >>>2.7.4 >>> >>ACK >> >>I tested intgcheck-run but I forgot that directory was already created >>due to intgcheck-prepare. >> > BTW the 1st invocation of make intgcheck-run took > real 0m58.740s > user 3m5.992s > sys 1m54.904s > and the 2nd just a > real 0m4.718s > user 0m0.619s > sys 0m0.159s > > master: > * 9639cf410dd6ba9670748535811f061e0c475bc6
Should be applied to 1-13 as well, right? Best Regards, -- Fabiano Fidêncio _______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/admin/lists/[email protected]
