URL: https://github.com/SSSD/sssd/pull/546 Author: fidencio Title: #546: TESTS: Re-add tests for `kdestroy -A` Action: opened
PR body: """ This reverts commit 89726be5a05493b7af312f0be9ac5ecb6f1822e1 and also do a few modifications on it in order to ensure we don't have any regression on https://pagure.io/SSSD/sssd/issue/3413
As this patch depends on a krb5 patch applied to the distros we run our internal CI on, I've opened a bug report providing patches for Fedora[0] and Debian[1].
[0]: https://bugzilla.redhat.com/show_bug.cgi?id=1561917 [1]: https://salsa.debian.org/debian/krb5/merge_requests/1 """
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/546/head:pr546 git checkout pr546
URL: https://github.com/SSSD/sssd/pull/546 Title: #546: TESTS: Re-add tests for `kdestroy -A`
jhrozek commented: """ I'll ack and push the patch if you show me a CI run from our internal Jenkins :-) """
See the full comment at https://github.com/SSSD/sssd/pull/546#issuecomment-377330980
URL: https://github.com/SSSD/sssd/pull/546 Title: #546: TESTS: Re-add tests for `kdestroy -A`
jhrozek commented: """ but the hunk itself of course LGTM """
See the full comment at https://github.com/SSSD/sssd/pull/546#issuecomment-377331057
URL: https://github.com/SSSD/sssd/pull/546 Title: #546: TESTS: Re-add tests for `kdestroy -A`
jhrozek commented: """ so, not totally related to this thread, but is this one of the cases where a label like "blocked" or "depends-on" would be useful? iirc you suggested something like this on sssd-devel the other day. """
See the full comment at https://github.com/SSSD/sssd/pull/546#issuecomment-377331452
URL: https://github.com/SSSD/sssd/pull/546 Title: #546: TESTS: Re-add tests for `kdestroy -A`
fidencio commented: """ Yep, that's exactly the situation I could see the "blocked"/"depends-on" tag being used. """
See the full comment at https://github.com/SSSD/sssd/pull/546#issuecomment-377343971
URL: https://github.com/SSSD/sssd/pull/546 Title: #546: TESTS: Re-add tests for `kdestroy -A`
Label: +Blocked
URL: https://github.com/SSSD/sssd/pull/546 Title: #546: TESTS: Re-add tests for `kdestroy -A`
jhrozek commented: """ On my up-to-date F-27 this test already passes with krb5-libs-1.15.2-8.fc27.x86_64. I started CI but I don't have the results yet.
The only small issue is how to make sure that on other platforms where the test might fail, it is at least easy to see why it fails. So I would propose to either move the test with 3 principals to a separate function and add a doc comment or at least add a better comment that this might fail due to a krb5 bug. """
See the full comment at https://github.com/SSSD/sssd/pull/546#issuecomment-384774349
URL: https://github.com/SSSD/sssd/pull/546 Title: #546: TESTS: Re-add tests for `kdestroy -A`
fidencio commented: """ @jhrozek, The problem here is that we're missing: - el6 and el7: Those could be solved by using a copr - debian: I've opened a PR (https://salsa.debian.org/debian/krb5/merge_requests/1) but I have got no answer
IMHO, there's not much we can do than wait till the debian patch gets merged, unfortunately. """
See the full comment at https://github.com/SSSD/sssd/pull/546#issuecomment-384800979
URL: https://github.com/SSSD/sssd/pull/546 Title: #546: TESTS: Re-add tests for `kdestroy -A`
jhrozek commented: """ We can mark the test as expected failure. I'm not sure myself if it's better or not, but tl;dr marking the test as expected failure would still print "failed" when you run the tests from the command line, but not fail the whole testsuite.
Personally, I would be OK with that because I run the test on Fedora where the bug is fixed. """
See the full comment at https://github.com/SSSD/sssd/pull/546#issuecomment-384888704
URL: https://github.com/SSSD/sssd/pull/546 Title: #546: TESTS: Re-add tests for `kdestroy -A`
alexey-tikhonov commented: """ retest this please """
See the full comment at https://github.com/SSSD/sssd/pull/546#issuecomment-532146213
URL: https://github.com/SSSD/sssd/pull/546 Title: #546: TESTS: Re-add tests for `kdestroy -A`
alexey-tikhonov commented: """ Hi @fidencio, is there any chance you could rebase this on the top of current master, please? """
See the full comment at https://github.com/SSSD/sssd/pull/546#issuecomment-532201421
URL: https://github.com/SSSD/sssd/pull/546 Title: #546: TESTS: Re-add tests for `kdestroy -A`
fidencio commented: """ @alexey-tikhonov,
Thanks for taking a look in the PR. At this point, being out of SSSD project for more than 1 year, without touching this PR for more than 1.5 years, I don't actually feel comfortable rebasing it and re-submitting without giving it some proper tests. Unfortunately, I have to time for testing it properly.
In the end, it's a 10 lines patch, feel free to just copy and paste it accordingly. Mentioning the original author (me) is highly desirable.
Best Regards, """
See the full comment at https://github.com/SSSD/sssd/pull/546#issuecomment-532218355
URL: https://github.com/SSSD/sssd/pull/546 Title: #546: TESTS: Re-add tests for `kdestroy -A`
fidencio commented: """ @alexey-tikhonov,
Thanks for taking a look in the PR. At this point, being out of SSSD project for more than 1 year, without touching this PR for more than 1.5 years, I don't actually feel comfortable rebasing it and re-submitting without giving it some proper tests. Unfortunately, I have no time for testing it properly.
In the end, it's a 10 lines patch, feel free to just copy and paste it accordingly. Mentioning the original author (me) is highly desirable.
Best Regards, """
See the full comment at https://github.com/SSSD/sssd/pull/546#issuecomment-532218355
URL: https://github.com/SSSD/sssd/pull/546 Author: fidencio Title: #546: TESTS: Re-add tests for `kdestroy -A` Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/546/head:pr546 git checkout pr546
URL: https://github.com/SSSD/sssd/pull/546 Title: #546: TESTS: Re-add tests for `kdestroy -A`
alexey-tikhonov commented: """ retest this please """
See the full comment at https://github.com/SSSD/sssd/pull/546#issuecomment-532655473
URL: https://github.com/SSSD/sssd/pull/546 Title: #546: TESTS: Re-add tests for `kdestroy -A`
alexey-tikhonov commented: """ retest this please """
See the full comment at https://github.com/SSSD/sssd/pull/546#issuecomment-532655473
URL: https://github.com/SSSD/sssd/pull/546 Title: #546: TESTS: Re-add tests for `kdestroy -A`
alexey-tikhonov commented: """ retest this please """
See the full comment at https://github.com/SSSD/sssd/pull/546#issuecomment-532709938
URL: https://github.com/SSSD/sssd/pull/546 Title: #546: TESTS: Re-add tests for `kdestroy -A`
jhrozek commented: """ Alexey, I don't know if Debian already picked up the fixed libkrb5. If not, I think it would be nice to add this as a separate test and mark it as xfail or similar. """
See the full comment at https://github.com/SSSD/sssd/pull/546#issuecomment-532818809
URL: https://github.com/SSSD/sssd/pull/546 Title: #546: TESTS: Re-add tests for `kdestroy -A`
alexey-tikhonov commented: """
Alexey, I don't know if Debian already picked up the fixed libkrb5.
I guess yes as GH CI: "sssd-ci/debian10 — Success."
But I will check.
"""
See the full comment at https://github.com/SSSD/sssd/pull/546#issuecomment-533055254
URL: https://github.com/SSSD/sssd/pull/546 Title: #546: TESTS: Re-add tests for `kdestroy -A`
alexey-tikhonov commented: """ Debian10 has fixed libkrb5. Fedora picked up fix long time ago. And even CentOS CI seems to be fine.
ACK.
"""
See the full comment at https://github.com/SSSD/sssd/pull/546#issuecomment-533586088
URL: https://github.com/SSSD/sssd/pull/546 Title: #546: TESTS: Re-add tests for `kdestroy -A`
Label: -Blocked
URL: https://github.com/SSSD/sssd/pull/546 Title: #546: TESTS: Re-add tests for `kdestroy -A`
Label: +Accepted
URL: https://github.com/SSSD/sssd/pull/546 Title: #546: TESTS: Re-add tests for `kdestroy -A`
Label: +Ready to push
URL: https://github.com/SSSD/sssd/pull/546 Title: #546: TESTS: Re-add tests for `kdestroy -A`
pbrezina commented: """ * `master` * 43aae7e3b804c781c6502f0eac018ce1ba93592c - TESTS: Re-add tests for `kdestroy -A`
"""
See the full comment at https://github.com/SSSD/sssd/pull/546#issuecomment-534077020
URL: https://github.com/SSSD/sssd/pull/546 Title: #546: TESTS: Re-add tests for `kdestroy -A`
Label: +Pushed
URL: https://github.com/SSSD/sssd/pull/546 Title: #546: TESTS: Re-add tests for `kdestroy -A`
Label: -Accepted
URL: https://github.com/SSSD/sssd/pull/546 Title: #546: TESTS: Re-add tests for `kdestroy -A`
Label: -Ready to push
URL: https://github.com/SSSD/sssd/pull/546 Author: fidencio Title: #546: TESTS: Re-add tests for `kdestroy -A` Action: closed
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/546/head:pr546 git checkout pr546
sssd-devel@lists.fedorahosted.org
-
alexey-tikhonov -
fidencio
-
jhrozek
-
pbrezina