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)
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.