On Thu, Dec 04, 2014 at 11:26:09AM +0100, Lukas Slebodnik wrote:
On (03/12/14 22:02), Jakub Hrozek wrote:
>On Wed, Dec 03, 2014 at 09:28:28PM +0100, Lukas Slebodnik wrote:
>> On (03/12/14 20:00), Jakub Hrozek wrote:
>> >On Wed, Dec 03, 2014 at 06:37:35PM +0100, Lukas Slebodnik wrote:
>> >> On (03/12/14 18:12), Jakub Hrozek wrote:
>> >> >test_child was declared to be build always but only actually built
when
>> >> >cmocka was available, which is not the case on all platforms.
>> >> >
>> >> >We can either 1) move the test_child outside the HAVE_CMOCKA or 2)
only
>> >> >append test_child to check_PROGRAMS when cmocka is available which
is what
>> >> >the attached patch does.
>> >> >
>> >> >I'm fine with either option. Thanks for opinions/review.
>> >>
>> >> >From 67db9748960fbaf6deac4c01084bc23bc1321723 Mon Sep 17 00:00:00
2001
>> >> >From: Jakub Hrozek <jhrozek(a)redhat.com>
>> >> >Date: Wed, 3 Dec 2014 17:00:25 +0100
>> >> >Subject: [PATCH] TESTS: Build test_child even without cmocka
>> >> >
>> >> >---
>> >> > Makefile.am | 5 ++++-
>> >> > 1 file changed, 4 insertions(+), 1 deletion(-)
>> >> >
>> >> >diff --git a/Makefile.am b/Makefile.am
>> >> >index
7a36543061789bbd370fcbb118b66edeb8025e89..6d754f062245cc073c945218f6755dab1c59c543 100644
>> >> >--- a/Makefile.am
>> >> >+++ b/Makefile.am
>> >> >@@ -236,10 +236,13 @@ endif # HAVE_CMOCKA
>> >> > check_PROGRAMS = \
>> >> > stress-tests \
>> >> > krb5-child-test \
>> >> >- test-child \
>> >>
>> >> test-child is simple program without external dependencies.
>> >> It does not depend on libcmocka therefore should not be in block
HAVE_CMOCKA.
>> >
>> >But currently it's only built to supplement a cmocka program, so
>> >building it would be useless.
>> >
>> I am not strictly against this micro optimisation.
>>
>> >>
>> >> On the other hand, I am not strictly against. If you decide to do such
change
>> >> then please also change the name of this binary. This binary isn't
test.
>> >> It is executable file which is used by another test.
>> >
>> >it's a child program used by the test_child_common.c unit and actually
>> >depends on it via environment variables. Can you suggest a better name?
>> >I generally suck at naming things and tbh don't consider the name too
>> >important to spend much time on.
>>
>> The name was confusing for me. I wanted to execute child test to fix a valgrind
>> issue. And my 1st attempt was wong.
>>
>> sh$ ./test-child
>> (Wed Dec 3 21:18:14:367425 2014) [sssd] [main] (0x0400): test_child completed
successfully
>>
>> the 2nd attept works.
>> sh$ ./test_child_common
>> [==========] Running 2 test(s).
>> [ RUN ] test_exec_child
>> [ OK ] test_exec_child
>> [ RUN ] test_exec_child_extra_args
>> [ OK ] test_exec_child_extra_args
>> [==========] 2 test(s) run.
>> [ PASSED ] 2 test(s).
>>
>> 0 FAILED TEST(S)
>>
>>
>> We have {ldap,krb5,selinux}-child.
>> IMHO the better name would be simple-child or dummy-child (maybe example-child)
>
>Well, I tried to also couple the tests via name...but as I said, I'm not
>the best when it comes to naming anything (I hope my wife will be the
>one who names our kids ;-))
>
>See additional patch that renames the patch to dummy-child. Too bad we
>haven't named the test 'properly' before pushing it, but..oh well.
>
>>
>> BTW confusion can be caused by the fact that we are not uniform with name
>> of tests.
>>
>> Some tests have suffix "-tests" sydb-tests, dyndns-tests ...
>> and others have prefix "test[-_]": test-find-uid, test-authtok,
test_sss_idmap,
>> test_search_bases ...
>>
>
>We can agree on a naming scheme for new tests (I've been using
>test-$module myself) but please don't spend any time on renaming old
>tests. That's just busywork.
Do you talk about name of the c module or name of resulting binary file?
dyndns_tests_SOURCES = \
^^^^^^^^^^^^
similar to name of binary file. The name is defined in check_PROGRAMS
$(SSSD_RESOLV_OBJ) \
src/tests/cmocka/test_dyndns.c \
^^^^^^^^^^^^^
name of module
If you want to rename resulting binary you needn't touch C modules.
If you feel strongly about the inconsistent names, feel free to send a
patch. I think we have more important things to do.
I agree, though, that being consistent with new tests would be nice.
>From 67db9748960fbaf6deac4c01084bc23bc1321723 Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhrozek(a)redhat.com>
>Date: Wed, 3 Dec 2014 17:00:25 +0100
>Subject: [PATCH 1/2] TESTS: Build test_child even without cmocka
>
>---
ACK
>From 49d77f1077ff1bb73c050b2117d344193191b770 Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhrozek(a)redhat.com>
>Date: Wed, 3 Dec 2014 21:50:02 +0100
>Subject: [PATCH 2/2] Rename test-child to dummy-child
>
>---
> Makefile.am | 8 ++++----
> src/tests/cmocka/{test_child.c => dummy_child.c} | 0
> src/tests/cmocka/test_child_common.c | 2 +-
> 3 files changed, 5 insertions(+), 5 deletions(-)
> rename src/tests/cmocka/{test_child.c => dummy_child.c} (100%)
>
ACK
Thanks, pushed to master:
fb3c5cdfcda069a5fbeb7b9d200c0881911364b8
9f521c61c17cecd9625ebc1b33c666fa3488622c