Skip to content
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: housekeeping - netgroups #7491

Closed
wants to merge 1 commit into from

Conversation

danlavu
Copy link

@danlavu danlavu commented Jul 12, 2024

housekeeping, the following is looked at and may have been done:

  • fixed typos and standardized formatting
  • renamed test cases to improve the clarity of what the test does
  • improved docstring language, setup, steps and expected results
  • synced code with the docstring order
  • removed necessary configuration relevant to the test
  • added pytest.mark.importance to test cases
  • added error messages to assertions

noteable changes:

  • moved netgroup tests to the the cache tests file, it tests the cache
    for netgroup objects

@pbrezina
Copy link
Member

I'll review this once #7490 is merged, to avoid multiple iterations.

@danlavu danlavu force-pushed the tests-housekeeping-netgroups branch 2 times, most recently from 96219cf to c0d7d0d Compare July 14, 2024 00:27
@danlavu danlavu force-pushed the tests-housekeeping-netgroups branch 2 times, most recently from cf79fdf to e74abf2 Compare July 18, 2024 14:01
@danlavu danlavu force-pushed the tests-housekeeping-netgroups branch 2 times, most recently from 295d0a2 to 835f99c Compare July 18, 2024 20:20
@alexey-tikhonov
Copy link
Member

@danlavu, please rebase/resolve conflict.

housekeeping, the following is looked at and may have been done:

* fixed typos and standardized formatting
* renamed test cases to improve the clarity of what the test does
* improved docstring language, setup, steps and expected results
* synced code with the docstring order
* removed necessary configuration relevant to the test
* added pytest.mark.importance to test cases
* added error messages to assertions

noteable changes:

* moved netgroup tests to the the cache tests file, it tests the cache
  for netgroup objects
@danlavu danlavu force-pushed the tests-housekeeping-netgroups branch from 835f99c to b454510 Compare July 25, 2024 13:07
@pbrezina
Copy link
Member

pbrezina commented Aug 1, 2024

Hi Dan, the three changes to test_cache.py that just adds the importance marker should go in separate commit since they have nothing in common with netgroups.

I personally don't like the idea of adding everything into test_cache.py. SSSD's primary function is to cache stuff, therefore this file will grow too much over the time. I think it makes perfect sense to split it into objects own modules like test_netgroup.py.

@danlavu
Copy link
Author

danlavu commented Aug 1, 2024

@pbrezina this was an oversight on my part. There is another netgroup test in cache and I thought these were those. Where it specifically looked in the ldb cache for the netgroup object.

@danlavu danlavu closed this Aug 1, 2024
@danlavu danlavu deleted the tests-housekeeping-netgroups branch November 4, 2024 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants