On Mon, Aug 29, 2016 at 3:51 PM, Lukas Slebodnik <lslebodn(a)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 <lslebodn(a)redhat.com>
wrote:
>>> On (29/08/16 14:15), Fabiano Fidêncio wrote:
>>>>On Mon, Aug 29, 2016 at 12:36 PM, Lukas Slebodnik
<lslebodn(a)redhat.com> wrote:
>>>>> On (29/08/16 12:08), Fabiano Fidêncio wrote:
>>>>>>On Mon, Aug 29, 2016 at 11:12 AM, Jakub Hrozek
<jhrozek(a)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
<fidencio(a)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?=
<fidencio(a)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
<fidencio(a)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?= <fidencio(a)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 <fidencio(a)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.