On (11/10/16 19:56), Jakub Hrozek wrote:
>On Tue, Sep 13, 2016 at 09:27:32AM +0200, Lukas Slebodnik wrote:
>> On (09/08/16 10:18), Jakub Hrozek wrote:
>> >On Tue, Aug 09, 2016 at 08:04:38AM +0200, Lukas Slebodnik wrote:
>> >> On (09/05/16 10:07), Jakub Hrozek wrote:
>> >> >On Wed, May 04, 2016 at 11:36:57PM +0200, Lukas Slebodnik wrote:
>> >> >> On (27/04/16 10:51), Jakub Hrozek wrote:
>> >> >> >Hi,
>> >> >> >
>> >> >> >the attached patches implement unit tests for the pam_sss
module using
>> >> >> >pam_wrapper and libpamtest. In my testing, the coverage is
around 75%
>> >> >> >with mostly the parts that require running as root being
untested.
>> >> >> >
>> >> >> >I worked on this patchset even though the features for 1.14
are in full
>> >> >> >swing because there are several tickets that will require
us to patch
>> >> >> >pam_sss, so it's important to have the code that
changes tested. In
>> >> >> >addition, when we merge Dan's patches to use TLS with
integration tests,
>> >> >> >then we'll be able to also test authentication in
integration tests
>> >> >> >easily using libpamtest-python.
>> >> >> >
>> >> >> >However, our CI fails for me constantly:
>> >> >> >
http://sssd-ci.duckdns.org/logs/job/42/75/fedora_rawhide/ci.html
>> >> >> >The strange thing is that running CI locally works fine and
so does make
>> >> >> >check. Can anyone help point me in the right direction as
to what should
>> >> >> >I check next? I suspect some of the environment variables
might not be
>> >> >> >set correctly, but I don't see why..
>> >> >>
>> >> >> Are you sure it pass locally with valgrind?
>> >> >>
>> >> >> It failed because there are valgrind errors.
>> >> >> I can see then on fedora 22 and fedora 23
>> >> >>
http://sssd-ci.duckdns.org/logs-test/job/2/70/fedora22/ci-build-debug/src...
>> >> >>
http://sssd-ci.duckdns.org/logs-test/job/2/70/fedora22/ci-build-debug/src...
>> >> >>
>> >> >> I cannot see them on fedora 24 or fedora rawhide
>> >> >>
http://sssd-ci.duckdns.org/logs-test/job/2/70/fedora_rawhide/ci-build-deb...
>> >> >> but it fails as well
>> >> >>
http://sssd-ci.duckdns.org/logs-test/job/2/70/fedora_rawhide/ci-build-deb...
>> >> >>
>> >> >> If the problem is with missing environment variables
>> >> >> then you might log all environment variables in main function
>> >> >
>> >> >just a quick update: the issues in tests were resolved. I hit two
bugs
>> >> >in pam_wrapper:
>> >> >
https://github.com/jhrozek/pam_wrapper/commit/ff7ec1c5ea7ed2360cbc59bd58f...
>> >> >
https://github.com/jhrozek/pam_wrapper/commit/c9b11ac8947bc0ff2ec3c4140b4...
>> >> >so once Andreas approves them, I will build a new pam_wrapper RPM
for
>> >> >Fedora and EPEL so that we can formally review the pam_sss test
patches.
>> >> Bump
>> >
>> >I only had time to rebase the patches:
>> >
https://github.com/jhrozek/sssd/tree/pwrap
>> >But now the tests fail, so I did something wrong, somewhere. I don't
>> >think I will have time to look into this until next week, sorry.
>>
>>
http://sssd-ci.duckdns.org/logs-test/job/4/48/summary.html
>>
http://sssd-ci.duckdns.org/logs-test/job/4/49/summary.html
>>
>> All test passed only on rhel6 :-(
>> Maybe because new test was skipped.
>>
>> cwrap tests were not executed on debian_testing due to following failures
>>
/var/lib/jenkins/workspace/ci-test/label/debian_testing/src/tests/cwrap/become_user-tests.sh:
>> 3: .: Can't open /cwrap_test_setup.sh
>> FAIL become_user-tests.sh (exit status: 2)
>>
>> @see
>>
http://sssd-ci.duckdns.org/logs-test/job/4/48/debian_testing/ci-build-deb...
>>
http://sssd-ci.duckdns.org/logs-test/job/4/48/debian_testing/ci-build-deb...
>>
http://sssd-ci.duckdns.org/logs-test/job/4/48/debian_testing/ci-build-deb...
>>
http://sssd-ci.duckdns.org/logs-test/job/4/48/debian_testing/ci-build-deb...
>>
>> mock build failed on epel7 (maybe missing pam_sss-tests.sh in tarball)
>> make[3]: Entering directory
`/builddir/build/BUILD/sssd-1.14.2/src/tests/cwrap'
>> make[4]: Entering directory
`/builddir/build/BUILD/sssd-1.14.2/src/tests/cwrap'
>> make[4]: *** No rule to make target `pam_sss-tests.sh', needed by
>> `pam_sss-tests.sh.log'. Stop.
>> make[4]: *** Waiting for unfinished jobs...
>>
>>
>> The pam_sss-tests test failed on fedora24. There are valgrind errors.
>> ==28387== Invalid read of size 1
>> ==28387== at 0x9CC2600: __GI_strchr (in /usr/lib64/libc-2.23.so)
>> ==28387== by 0x402234: service_arg (test_wrapper_pam_sss.c:93)
>> ==28387== by 0x402D73: test_pam_authenticate_root_ignore
(test_wrapper_pam_sss.c:571)
>> ==28387== by 0x565B542: ??? (in /usr/lib64/libcmocka.so.0.3.1)
>> ==28387== by 0x565BC7A: _cmocka_run_group_tests (in
/usr/lib64/libcmocka.so.0.3.1)
>> ==28387== by 0x401AE0: main (test_wrapper_pam_sss.c:1343)
>> ==28387== Address 0xc81b6b3 is 0 bytes after a block of size 227 alloc'd
>> ==28387== at 0x4C2BBAD: malloc (vg_replace_malloc.c:299)
>> ==28387== by 0x97FF38D: talloc_named_const (in /usr/lib64/libtalloc.so.2.1.6)
>> ==28387== by 0x4021A4: service_arg (test_wrapper_pam_sss.c:67)
>> ==28387== by 0x402D73: test_pam_authenticate_root_ignore
(test_wrapper_pam_sss.c:571)
>> ==28387== by 0x565B542: ??? (in /usr/lib64/libcmocka.so.0.3.1)
>> ==28387== by 0x565BC7A: _cmocka_run_group_tests (in
/usr/lib64/libcmocka.so.0.3.1)
>> ==28387== by 0x401AE0: main (test_wrapper_pam_sss.c:1343)
>>
>>
http://sssd-ci.duckdns.org/logs-test/job/4/48/fedora24/ci-build-debug/src...
>>
http://sssd-ci.duckdns.org/logs-test/job/4/48/fedora24/ci-build-debug/src...
>>
>> It would be good to merge patches. Currently we have approcimately 30% of line
>> coverage and only 50% of functions are partially covered.
>> (measured with unit + integration tests)
>
>Attached patches passed CI:
>
http://sssd-ci.duckdns.org/logs/job/54/85/summary.html
>
>Since the goal is to have some integration tests with authentication,
>I'll also try to add a python-based integration tests. The attached
>patches "just" cover pam_sss.so with test cases.
>From d5ac8905458e50926f194e5e2bd0eab14b11a538 Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhrozek(a)redhat.com>
>Date: Thu, 15 Oct 2015 22:26:15 +0200
>Subject: [PATCH 01/12] PAM: Move PAM packet parsing functionality to a
> separate module
>
1.) You need to rebase due to pam_response_filter changes
+ fix mocking issues in pam-srv-tests
(maybe I just rebased patches in bad way)
2.) Fix dlopen-tests:
Running suite(s): dlopen
Unchecked library found: pam_test_sss.so
3.) make check failed under root
pam_sss.so has special case for UID 0
Ideal would be test even both cases (UID == 0 and UID != 0)
with uid_wrapper.
4.) hardcoded timezone strings in test
It fails with different timezone e.g.
export TZ=America/Havana
make check
5) Please use asserts or debug_messages in function service_arg
There might be more issues because I reviewed patches briefly
due to my own rebase of patches and failed unit tests.