[sssd PR#546][opened] TESTS: Re-add tests for `kdestroy -A`

[fidencio]

   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

[sssd PR#546][comment] TESTS: Re-add tests for `kdestroy -A`

[jhrozek]

  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

[sssd PR#546][comment] TESTS: Re-add tests for `kdestroy -A`

[jhrozek]

  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

[sssd PR#546][comment] TESTS: Re-add tests for `kdestroy -A`

[jhrozek]

  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

[sssd PR#546][comment] TESTS: Re-add tests for `kdestroy -A`

[fidencio]

  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

[sssd PR#546][+Blocked] TESTS: Re-add tests for `kdestroy -A`

[fidencio]

  URL: https://github.com/SSSD/sssd/pull/546
Title: #546: TESTS: Re-add tests for `kdestroy -A`

Label: +Blocked

[sssd PR#546][comment] TESTS: Re-add tests for `kdestroy -A`

[jhrozek]

  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

[sssd PR#546][comment] TESTS: Re-add tests for `kdestroy -A`

[fidencio]

  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

[sssd PR#546][comment] TESTS: Re-add tests for `kdestroy -A`

[jhrozek]

  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

[sssd PR#546][comment] TESTS: Re-add tests for `kdestroy -A`

[alexey-tikhonov]

  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

[sssd PR#546][comment] TESTS: Re-add tests for `kdestroy -A`

[alexey-tikhonov]

  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

[sssd PR#546][comment] TESTS: Re-add tests for `kdestroy -A`

[fidencio]

  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

[sssd PR#546][comment] TESTS: Re-add tests for `kdestroy -A`

[fidencio]

  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

[sssd PR#546][synchronized] TESTS: Re-add tests for `kdestroy -A`

[alexey-tikhonov]

   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

[sssd PR#546][comment] TESTS: Re-add tests for `kdestroy -A`

[alexey-tikhonov]

  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

[sssd PR#546][comment] TESTS: Re-add tests for `kdestroy -A`

[alexey-tikhonov]

  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

[sssd PR#546][comment] TESTS: Re-add tests for `kdestroy -A`

[alexey-tikhonov]

  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

[sssd PR#546][comment] TESTS: Re-add tests for `kdestroy -A`

[jhrozek]

  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

[sssd PR#546][comment] TESTS: Re-add tests for `kdestroy -A`

[alexey-tikhonov]

  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

[sssd PR#546][comment] TESTS: Re-add tests for `kdestroy -A`

[alexey-tikhonov]

  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

[sssd PR#546][-Blocked] TESTS: Re-add tests for `kdestroy -A`

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/546
Title: #546: TESTS: Re-add tests for `kdestroy -A`

Label: -Blocked

[sssd PR#546][+Accepted] TESTS: Re-add tests for `kdestroy -A`

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/546
Title: #546: TESTS: Re-add tests for `kdestroy -A`

Label: +Accepted

[sssd PR#546][+Ready to push] TESTS: Re-add tests for `kdestroy -A`

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/546
Title: #546: TESTS: Re-add tests for `kdestroy -A`

Label: +Ready to push

[sssd PR#546][comment] TESTS: Re-add tests for `kdestroy -A`

[pbrezina]

  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

[sssd PR#546][+Pushed] TESTS: Re-add tests for `kdestroy -A`

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/546
Title: #546: TESTS: Re-add tests for `kdestroy -A`

Label: +Pushed

[sssd PR#546][-Accepted] TESTS: Re-add tests for `kdestroy -A`

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/546
Title: #546: TESTS: Re-add tests for `kdestroy -A`

Label: -Accepted

[sssd PR#546][-Ready to push] TESTS: Re-add tests for `kdestroy -A`

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/546
Title: #546: TESTS: Re-add tests for `kdestroy -A`

Label: -Ready to push

[sssd PR#546][closed] TESTS: Re-add tests for `kdestroy -A`

[pbrezina]

   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