On Mon, Aug 29, 2016 at 03:57:47PM +0200, Fabiano Fidêncio wrote:
> On Mon, Aug 29, 2016 at 3:53 PM, Jakub Hrozek <[email protected]> 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 <[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
> >
> > awesome!
> >
> > Can we:
> > 1) file tickets for any additional work?
>
> Can you file the tickets for what you feel like is missing?
Only a very minor issue:
https://fedorahosted.org/sssd/ticket/3160
but I really like this commit, you already improved the Makefile targets
a lot :)
>
> > 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.
Yes :)
_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/admin/lists/[email protected]
