-
Notifications
You must be signed in to change notification settings - Fork 246
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
tests: removing intg/ts_cache.py #7580
Conversation
Could you share where exactly these tests getting covered in system/test_cache.py? |
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.
Could you share where exactly these rfc2307bis related tests getting covered in system/test_cache.py?
CI is red |
@shridhargadekar It's not. We're not porting tests, the coverage is related to cache timestamp in ldb which is covered. I asked the team how to determine what LDAP tests we should test against rfc2307 and rfc2307bis, but we can easily take any test and parameterize the test to re-run it against 2307bis, so this can easily be done later. |
a6649d3
to
9e764d8
Compare
@danlavu not all tests have the coverage, specifically L655 L628 |
If missing coverage is done later then consider approval from me. |
@shridhargadekar |
9e764d8
to
3afd360
Compare
I added two tests that cover the missing scenarios you mentioned. Test that ensures the group changes are reflected after a clear Updated the assertion for one of the tests that was confusing, mentioned in slack. |
Tests are not green, looking into it. |
3afd360
to
825a777
Compare
The test was not working correctly, it was passing when there were no results, fixed now. |
e393808
to
ac1f101
Compare
@alexey-tikhonov, I converted all the tests to use the LDAP provider, as we discussed, and I corrected the logic; it's working now and not being skipped. I did notice that netgroups in the timestamp cache is empty. There is nothing in the results. Do you know if netgroups is supposed to write to the timestamp cache? |
ac1f101
to
4487665
Compare
4487665
to
b21b7f3
Compare
@shridhargadekar, please review the latest version. |
the following test cases are now covered in system/test_cache.py and this can be removed. * fixed assertion writes_to_both_databases tests * added test detecting modification and deletion for groups ** test is a common user story and functional, changed priority to critical * added "integration" test invalidating user, group, netgroup objects
There is minimal benefit to run these tests against all providers.
b21b7f3
to
ca06f17
Compare
The failure is not related to the patch. |
@danlavu, there is a conflict in sssd-2-9, please open explicit backport PR if needed. |
the following test cases are now covered in system/test_cache.py and this can be removed.