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?
2) write a wiki page for developers with instructions how to run the
tests?
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
