[sssd PR#913][opened] Updated test_pam_responder.py

[alexal]

URL: https://github.com/SSSD/sssd/pull/913
Author: alexal
 Title: #913: Updated test_pam_responder.py
Action: opened

PR body:
"""
Integration tests/CI: Updated test_pam_responder.py and removed libnss3-dev from Debian dependency list

- Functions create_nssdb_fixture and create_nssdb_no_cert_fixture inside test_pam_responder.py file will be called only if environment variable HAVE_NSS is defined
 - Since CI build for Debian is using OpenSSL this improvement allowed to drop libnss3-dev from Debian dependency list in contrib/ci/deps.sh

Resolves: https://pagure.io/SSSD/sssd/issue/3914
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/913/head:pr913
git checkout pr913

[sssd PR#913][comment] Updated test_pam_responder.py

[centos-ci]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

centos-ci commented:
"""
Can one of the admins verify this patch?
"""

See the full comment at https://github.com/SSSD/sssd/pull/913#issuecomment-545465635

[sssd PR#913][comment] Updated test_pam_responder.py

[alexey-tikhonov]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

alexey-tikhonov commented:
"""
ok to test
"""

See the full comment at https://github.com/SSSD/sssd/pull/913#issuecomment-545486800

[sssd PR#913][comment] Updated test_pam_responder.py

[alexey-tikhonov]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

alexey-tikhonov commented:
"""
Hi @alexal ,

Thank you for paying attention to this ticket.

Did you consider possibility to drop `libnspr4-dev` from Debian dep list as well? (as suggested by @sumit-bose  [here](https://github.com/SSSD/sssd/pull/720#issuecomment-449041310))

"""

See the full comment at https://github.com/SSSD/sssd/pull/913#issuecomment-545494776

[sssd PR#913][comment] Updated test_pam_responder.py

[alexal]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

alexal commented:
"""
@alexey-tikhonov , I'm sorry I've missed that. I could do that quickly, but I need to set up a Debian machine to run the full test. That may take a while :-)
"""

See the full comment at https://github.com/SSSD/sssd/pull/913#issuecomment-545497554

[sssd PR#913][comment] Updated test_pam_responder.py

[alexal]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

alexal commented:
"""
@alexey-tikhonov , I'm sorry I've missed that. I could do that, but I need to set up a Debian machine to run the full test. That may take a while :-)
"""

See the full comment at https://github.com/SSSD/sssd/pull/913#issuecomment-545497554

[sssd PR#913][comment] Updated test_pam_responder.py

[sumit-bose]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

sumit-bose commented:
"""
> @alexey-tikhonov , I'm sorry I've missed that. I could do that, but I need to set up a Debian machine to run the full test. That may take a while :-)

There is Debian instance in the CI tests which are run whenever the pull-request is updated. So you can just push your changes and check the tests.
"""

See the full comment at https://github.com/SSSD/sssd/pull/913#issuecomment-545504585

[sssd PR#913][comment] Updated test_pam_responder.py

[alexey-tikhonov]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

alexey-tikhonov commented:
"""
centos-ci: `make-intgcheck:         failure  00:19:02 ci-build-debug/ci-make-intgcheck.log`

Unfortunately, there is not easy way to retrieve logs...
"""

See the full comment at https://github.com/SSSD/sssd/pull/913#issuecomment-545504947

[sssd PR#913][+Changes requested] Updated test_pam_responder.py

[alexey-tikhonov]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

Label: +Changes requested

[sssd PR#913][comment] Updated test_pam_responder.py

[alexey-tikhonov]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

alexey-tikhonov commented:
"""
Have set "changes requested" to indicate necessity to:
1) check possibility of `libnspr4-dev` removal
2) investigate cent-os fail
"""

See the full comment at https://github.com/SSSD/sssd/pull/913#issuecomment-545506303

[sssd PR#913][synchronized] Updated test_pam_responder.py

[alexal]

URL: https://github.com/SSSD/sssd/pull/913
Author: alexal
 Title: #913: Updated test_pam_responder.py
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/913/head:pr913
git checkout pr913

[sssd PR#913][comment] Updated test_pam_responder.py

[alexal]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

alexal commented:
"""
@alexey-tikhonov, I spent quite some time this evening and I'm not able to reproduce a problem from the log in sssd-ci/fedora28. Everything works just fine on my Fedora server. Also, still trying to figure out what is wrong with the "default" CentOS 7 build. I've created a new CentOS instance and I will continue my tests tomorrow...
"""

See the full comment at https://github.com/SSSD/sssd/pull/913#issuecomment-545728708

[sssd PR#913][comment] Updated test_pam_responder.py

[sumit-bose]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

sumit-bose commented:
"""
> @alexey-tikhonov, I spent quite some time this evening and I'm not able to reproduce a problem from the log in sssd-ci/fedora28. Everything works just fine on my Fedora server. Also, still trying to figure out what is wrong with the "default" CentOS 7 build. I've created a new CentOS instance and I will continue my tests tomorrow...

Hi,

I guess both CentOS-7 and F28 use NSS. I think my comment about the environment variables might have been misleading, HAVE_NSS is not automatically added to the environment. You have to add an environment variable explicitly in the Makefile like e.g. PAM_CERT_DB_PATH or SOFTHSM2_CONF.

HTH

bye,
Sumit
"""

See the full comment at https://github.com/SSSD/sssd/pull/913#issuecomment-545749156

[sssd PR#913][synchronized] Updated test_pam_responder.py

[alexal]

URL: https://github.com/SSSD/sssd/pull/913
Author: alexal
 Title: #913: Updated test_pam_responder.py
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/913/head:pr913
git checkout pr913

[sssd PR#913][synchronized] Updated test_pam_responder.py

[alexal]

URL: https://github.com/SSSD/sssd/pull/913
Author: alexal
 Title: #913: Updated test_pam_responder.py
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/913/head:pr913
git checkout pr913

[sssd PR#913][synchronized] Updated test_pam_responder.py

[alexal]

URL: https://github.com/SSSD/sssd/pull/913
Author: alexal
 Title: #913: Updated test_pam_responder.py
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/913/head:pr913
git checkout pr913

[sssd PR#913][comment] Updated test_pam_responder.py

[alexal]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

alexal commented:
"""
@sumit-bose thanks for the clarification. @alexey-tikhonov with the most recent code change, the integration tests ran successfully on CentOS, Fedora 28 and 30, but two different test failed on Debian and Fedora 29. Those were working yesterday, which is confusing.
"""

See the full comment at https://github.com/SSSD/sssd/pull/913#issuecomment-546154834

[sssd PR#913][comment] Updated test_pam_responder.py

[sumit-bose]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

sumit-bose commented:
"""
> @sumit-bose thanks for the clarification. @alexey-tikhonov with the most recent code change, the integration tests ran successfully on CentOS, Fedora 28 and 30, but two different test failed on Debian and Fedora 29. Those were working yesterday, which is confusing.

Unfortunately some of the integration tests are flakey when running on the CI systems. The two failures are not related to your patch and there is a fair chance that if the tests are run again they will pass.
"""

See the full comment at https://github.com/SSSD/sssd/pull/913#issuecomment-546203381

[sssd PR#913][comment] Updated test_pam_responder.py

[alexal]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

alexal commented:
"""
@sumit-bose thanks, so everything looks good from your point of view?
"""

See the full comment at https://github.com/SSSD/sssd/pull/913#issuecomment-546310977

[sssd PR#913][comment] Updated test_pam_responder.py

[sumit-bose]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

sumit-bose commented:
"""
> @sumit-bose thanks, so everything looks good from your point of view?

Yes, thank you for the patch. I just think it would be easier for people adding new tests if you would move the check of the environment variable inside `create_nssdb_fixture` and `create_nssdb_no_cert_fixture`.

bye,
Sumit
"""

See the full comment at https://github.com/SSSD/sssd/pull/913#issuecomment-546316374

[sssd PR#913][synchronized] Updated test_pam_responder.py

[alexal]

URL: https://github.com/SSSD/sssd/pull/913
Author: alexal
 Title: #913: Updated test_pam_responder.py
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/913/head:pr913
git checkout pr913

[sssd PR#913][comment] Updated test_pam_responder.py

[alexal]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

alexal commented:
"""
@sumit-bose thanks, I've made those changes. Please see the updated commit.
"""

See the full comment at https://github.com/SSSD/sssd/pull/913#issuecomment-546328637

[sssd PR#913][comment] Updated test_pam_responder.py

[alexal]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

alexal commented:
"""
@sumit-bose you are absolutely right, all tests on Debian were completed successfully.
"""

See the full comment at https://github.com/SSSD/sssd/pull/913#issuecomment-546358739

[sssd PR#913][comment] Updated test_pam_responder.py

[alexal]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

alexal commented:
"""
@sumit-bose , @alexey-tikhonov all checks have passed.
"""

See the full comment at https://github.com/SSSD/sssd/pull/913#issuecomment-546369962

[sssd PR#913][comment] Updated test_pam_responder.py

[sumit-bose]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

sumit-bose commented:
"""
Thanks, ACK.
"""

See the full comment at https://github.com/SSSD/sssd/pull/913#issuecomment-546372355

[sssd PR#913][-Changes requested] Updated test_pam_responder.py

[sumit-bose]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

Label: -Changes requested

[sssd PR#913][+Accepted] Updated test_pam_responder.py

[sumit-bose]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

Label: +Accepted

[sssd PR#913][-Accepted] Updated test_pam_responder.py

[alexey-tikhonov]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

Label: -Accepted

[sssd PR#913][comment] Updated test_pam_responder.py

[alexey-tikhonov]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

alexey-tikhonov commented:
"""
> @sumit-bose thanks, I've made those changes. Please see the updated commit.

Totally nitpick, but now commit message doesn't match patch :)

I mean this part: `Functions create_nssdb_fixture and create_nssdb_no_cert_fixture inside test_pam_responder.py file will be called only if environment variable USE_NSS is defined and set`
"""

See the full comment at https://github.com/SSSD/sssd/pull/913#issuecomment-546450907

[sssd PR#913][+Changes requested] Updated test_pam_responder.py

[alexey-tikhonov]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

Label: +Changes requested

[sssd PR#913][synchronized] Updated test_pam_responder.py

[alexal]

URL: https://github.com/SSSD/sssd/pull/913
Author: alexal
 Title: #913: Updated test_pam_responder.py
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/913/head:pr913
git checkout pr913

[sssd PR#913][edited] Updated test_pam_responder.py

[alexal]

URL: https://github.com/SSSD/sssd/pull/913
Author: alexal
 Title: #913: Updated test_pam_responder.py
Action: edited

Changed field: body
Original value:
"""
Integration tests/CI: Updated test_pam_responder.py and removed libnss3-dev from Debian dependency list

- Functions create_nssdb_fixture and create_nssdb_no_cert_fixture inside test_pam_responder.py file will be called only if environment variable HAVE_NSS is defined
 - Since CI build for Debian is using OpenSSL this improvement allowed to drop libnss3-dev from Debian dependency list in contrib/ci/deps.sh

Resolves: https://pagure.io/SSSD/sssd/issue/3914
"""

[sssd PR#913][comment] Updated test_pam_responder.py

[alexal]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

alexal commented:
"""
@alexey-tikhonov good catch. I've updated the commit message.
"""

See the full comment at https://github.com/SSSD/sssd/pull/913#issuecomment-546452962

[sssd PR#913][comment] Updated test_pam_responder.py

[alexal]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

alexal commented:
"""
@alexey-tikhonov just wondering, have you had a chance to check the commit message? Everything looks good?
"""

See the full comment at https://github.com/SSSD/sssd/pull/913#issuecomment-546919328

[sssd PR#913][comment] Updated test_pam_responder.py

[alexey-tikhonov]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

alexey-tikhonov commented:
"""
> @alexey-tikhonov just wondering, have you had a chance to check the commit message? Everything looks good?

Yes, thank you.
"""

See the full comment at https://github.com/SSSD/sssd/pull/913#issuecomment-547338050

[sssd PR#913][-Changes requested] Updated test_pam_responder.py

[alexey-tikhonov]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

Label: -Changes requested

[sssd PR#913][+Accepted] Updated test_pam_responder.py

[alexey-tikhonov]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

Label: +Accepted

[sssd PR#913][+Ready to push] Updated test_pam_responder.py

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

Label: +Ready to push

[sssd PR#913][comment] Updated test_pam_responder.py

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

pbrezina commented:
"""
* `master`
    * 16124d411e17096a9374410a0c89193a6dac22ea - Updated test_pam_responder.py

"""

See the full comment at https://github.com/SSSD/sssd/pull/913#issuecomment-548334674

[sssd PR#913][+Pushed] Updated test_pam_responder.py

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

Label: +Pushed

[sssd PR#913][-Accepted] Updated test_pam_responder.py

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

Label: -Accepted

[sssd PR#913][-Ready to push] Updated test_pam_responder.py

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/913
Title: #913: Updated test_pam_responder.py

Label: -Ready to push

[sssd PR#913][closed] Updated test_pam_responder.py

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/913
Author: alexal
 Title: #913: Updated test_pam_responder.py
Action: closed

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/913/head:pr913
git checkout pr913