-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes bug in compare_user on Linux systems #13223
Conversation
fdd9186
to
5bbc061
Compare
@marcparadise @jaymzh This looks like it should fix the user resource for subsequent runs failing with calling |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this, this is a good catch!
I have put one request inline; separately in the spirit of 'if we had tested this function earlier, this would have been caught sooner' - can you add unit (and ideally functional) test cases to cover this?
5bbc061
to
1ed4089
Compare
Hi, I would love to add a functional test, but I don't have experience in this matter. I'll see what I can do. As I view it, there should be a |
@tpowell-progress can help out with this |
The current openSUSE build failure is a related to the build container at first glance, and not this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice catch. Though I agree, tests would be good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revoking approval, b/c tests
a0928c2
to
cf80d39
Compare
d4c3b8a introduced a bug making `compare_user` on Linux systems return `%i{expire_date inactive}` instead of wether `@change_desc` is empty. Signed-off-by: Frederik Thuysbaert <frederik.thuysbaert@combell.group>
cf80d39
to
f4fab1f
Compare
Kudos, SonarCloud Quality Gate passed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fretb I see you have written tests. Nice work!
Apologies for the delay in circling back... build issues and internet outages.
Description
d4c3b8a introduced a bug making
compare_user
on Linux systems return%i{expire_date inactive}
instead of wether@change_desc
is empty.Types of changes
Checklist: