[SSSD] [PATCH v4] Add integration tests

Nikolai Kondrashov Nikolai.Kondrashov at redhat.com
Mon May 25 14:55:00 UTC 2015


On 05/25/2015 11:44 AM, Lukas Slebodnik wrote:
> A)
> The 1st patch is not necessary.
> CI passed without 1st patch
> http://sssd-ci.duckdns.org/logs/job/15/89/summary.html (fedora 20 failed due to
> dyndnd-test)
>
> B) You are misusing autoconf macro AM_CONDITIONAL.
> It shoudl be used just in case you want to have "if conditions" in automake
> files. ENABLE_INTGCHECK
>
> Please try to look into documentation of autoconf macros
> AC_SUBST
> AC_DEFINE (AC_DEFINE_UNQUOTED)
> AM_CONDITIONAL
>
> In most cases you just need just AC_SUBST for HAVE_$sth
> or just AC_CHECK_PROG.

Yes this can make things simpler and more reliable. I remade the configuration
to just use shell variables, as AC_SUBST is not necessary, because we don't
use these in any configure-processed files (e.g. Makefile.am's).

Please see the first attached patch to see what I mean.

> This also reminds me you can squas
> "small external" files into src/external/intgcheck.m4
> It does not worth to have one file just with a line AC_CHECK_PROG
>
>   src/external/fakeroot.m4      |   2 +
>   src/external/intgcheck.m4     |  23 ++++
>   src/external/ldap.m4          |   6 +
>   src/external/pytest.m4        |   3 +

Regarding this, wouldn't it make the corresponding macros somewhat harder to
find? We have a few short files under src/external with clear names and
contents that doesn't quite fit anywhere and I thought that rather neat.

I can stuff fakeroot and py.test checks into intgcheck.m4 without much harm, I
think. However, would it really be good to move slapd and ldapmodify checking
out of ldap.m4? Wouldn't it be confusing?

> @see
> diff --git a/src/external/pytest.m4 b/src/external/pytest.m4
> index 00f28e5..b6305d1 100644
> --- a/src/external/pytest.m4
> +++ b/src/external/pytest.m4
> @@ -1,3 +1,2 @@
>   AC_PATH_PROG([PYTEST], [py.test])
> -AC_SUBST([PYTEST])
> -AM_CONDITIONAL([HAVE_PYTEST], [test x"$PYTEST" != x])
> +AC_SUBST([HAVE_PYTEST], [test x"$PYTEST" != x])

Hmm, would the last line really work? To me it looks like not, am I missing
something?

> C) We can simplify make target intgcheck
> It significantly improve user experince.
> (the same as "make distcheck" or make tests in samba)

Ah, good catch, yes. Thank you, Lukas. However, leaving the configure option
named "--enable-intgcheck" will be confusing, so I tried to rename it to
something more relevant to what it will be doing now. It still feels a bit
awkward, but perhaps it's good enough.

Please take a look at the second attached patch, for illustration.

> D) please find better name for "src/tests/intg/misc.py"

I think "misc.py" is perfectly fine, but since our previous discussion on
similar matter stalled, please suggest a name and I'll name it as you say and
we'll be done with it.

> I will file other tickets with improvements after pushing patch to master.

Thank you, Lukas.

> BTW there is a failed intgtest
> http://sssd-ci.duckdns.org/logs/job/15/90/fedora20/ci-build-debug/ci-make-intgcheck.log
> It would be good to prevent such fialures. I've already sent patches to fix
> negcache-test and SSSDConfig-test. We do not need to have another problematic test

Argh, I've seen this, wanted to fix it and forgot. Can't reproduce it at the
moment, but I'll keep trying. Meanwhile, do you have any ideas of what might
be causing this?

Thank you.

Nick
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-intgcheck-Use-shell-variables-directly-for-dep-check.patch
Type: text/x-patch
Size: 3803 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150525/dd5b4569/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-intgcheck-Rename-enable-intgcheck-to-enable-intgchec.patch
Type: text/x-patch
Size: 2754 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150525/dd5b4569/attachment-0001.bin>


More information about the sssd-devel mailing list