On Mon, Aug 29, 2016 at 3:53 PM, Jakub Hrozek <jhro...@redhat.com> wrote:
> On Mon, Aug 29, 2016 at 03:51:06PM +0200, Lukas Slebodnik 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
>
> awesome!
>
> Can we:
> 1) file tickets for any additional work?
Can you file the tickets for what you feel like is missing?
> 2) write a wiki page for developers with instructions how to run the
> tests?
I'll add something on https://fedorahosted.org/sssd/wiki/Contribute
about how to run the tests.
Would be nice if we can have some documentation about how to write the
tests as well? Someone who touched this for the last time could sum
up the experience and write down the document.
Best Regards,
--
Fabiano Fidêncio
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
