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 <lsleb...@redhat.com> 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 <lsleb...@redhat.com> 
>>>>>wrote:
>>>>>> On (29/08/16 14:15), Fabiano Fidêncio wrote:
>>>>>>>On Mon, Aug 29, 2016 at 12:36 PM, Lukas Slebodnik <lsleb...@redhat.com> 
>>>>>>>wrote:
>>>>>>>> On (29/08/16 12:08), Fabiano Fidêncio wrote:
>>>>>>>>>On Mon, Aug 29, 2016 at 11:12 AM, Jakub Hrozek <jhro...@redhat.com> 
>>>>>>>>>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 
>>>>>>>>>>> ><fiden...@redhat.com> 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?= <fiden...@redhat.com>
>>>>>>>>>>> >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 <fiden...@redhat.com>
>>>>>>>>>>> >---
>>>>>>>>>>> > 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?= <fiden...@redhat.com>
>>>>>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 <fiden...@redhat.com>
>>>>>---
>>>>> 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
>>> sssd-devel@lists.fedorahosted.org
>>> https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
>
>>From 711e8c3cfca88e1431b6f667946c901f02665b31 Mon Sep 17 00:00:00 2001
>>From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com>
>>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 <fiden...@redhat.com>
>>---
>> 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

LS
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

Reply via email to