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

_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/admin/lists/[email protected]

Reply via email to