- Notifications
You must be signed in to change notification settings - Fork676
chore: ensure reset_gitlab() succeeds#1783
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
abfcb9e tofa2b2c1Comparefa2b2c1 to711ae44Compare112f07f toc9243baComparec9243ba toe409638Comparetests/functional/conftest.py Outdated
| defwait_for_list_empty( | ||
| rest_manager:gitlab.base.RESTManager,description:str | ||
| )->None: | ||
| """Wait for the list() to become empty or fail test if timeout is exceeded""" | ||
| for_inrange(max_iterations): | ||
| ifrest_manager.list()== []: | ||
| break | ||
| time.sleep(SLEEP_INTERVAL) | ||
| assertrest_manager.list()== [], ( | ||
| f"Did not delete all{description}. " | ||
| f"Elapsed_time:{time.perf_counter()-start_time}" | ||
| ) | ||
| wait_for_list_empty(rest_manager=gl.projects,description="projects") | ||
| wait_for_list_empty(rest_manager=gl.groups,description="groups") | ||
| wait_for_list_empty(rest_manager=gl.variables,description="variables") | ||
| for_inrange(max_iterations): | ||
| iflen(gl.users.list())<=1: | ||
| break | ||
| time.sleep(SLEEP_INTERVAL) | ||
| assert [user.usernameforuseringl.users.list()]== ["root"], ( | ||
| f"Did not delete all users. " | ||
| f"elapsed_time:{time.perf_counter()-start_time}" | ||
| ) |
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! Can we even just not care about the root user specifics and use the same helper for both cases e.g. something like this (not sure what to name it then 😀 ):
(not sure if I missed something)
| defwait_for_list_empty( | |
| rest_manager:gitlab.base.RESTManager,description:str | |
| )->None: | |
| """Wait for the list() to become empty or fail test if timeout is exceeded""" | |
| for_inrange(max_iterations): | |
| ifrest_manager.list()== []: | |
| break | |
| time.sleep(SLEEP_INTERVAL) | |
| assertrest_manager.list()== [], ( | |
| f"Did not delete all{description}. " | |
| f"Elapsed_time:{time.perf_counter()-start_time}" | |
| ) | |
| wait_for_list_empty(rest_manager=gl.projects,description="projects") | |
| wait_for_list_empty(rest_manager=gl.groups,description="groups") | |
| wait_for_list_empty(rest_manager=gl.variables,description="variables") | |
| for_inrange(max_iterations): | |
| iflen(gl.users.list())<=1: | |
| break | |
| time.sleep(SLEEP_INTERVAL) | |
| assert [user.usernameforuseringl.users.list()]== ["root"], ( | |
| f"Did not delete all users. " | |
| f"elapsed_time:{time.perf_counter()-start_time}" | |
| ) | |
| defwait_for_list_empty( | |
| rest_manager:gitlab.base.RESTManager,description:str,length:int=0 | |
| )->None: | |
| """Wait for the list() to have required length or fail test if timeout is exceeded""" | |
| for_inrange(max_iterations): | |
| iflen(rest_manager.list())<=length: | |
| break | |
| time.sleep(SLEEP_INTERVAL) | |
| assertlen(rest_manager.list())<=length, ( | |
| f"Did not delete all{description}. " | |
| f"Elapsed_time:{time.perf_counter()-start_time}" | |
| ) | |
| wait_for_list_empty(rest_manager=gl.projects,description="projects") | |
| wait_for_list_empty(rest_manager=gl.groups,description="groups") | |
| wait_for_list_empty(rest_manager=gl.variables,description="variables") | |
| wait_for_list_empty(rest_manager=gl.users,description="users",length=1) |
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.
Looks nice. I will do that. Now which PR do we think will land first? Because the 2nd PR to land will need to be rebased.
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.
This seems like a smaller change so I'd go with this one unless you think it'll be a very tedious rebase on the other one 😁
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.
This seems like a smaller change so I'd go with this one unless you think it'll be a very tedious rebase on the other one 😁
Works for me. Order doesn't matter for difficulty I believe.
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.
Also I think this one is better to go first. Should make other one cleaner in not needing to touchwait_for_sidekiq code.
Ensure reset_gitlab() succeeds by waiting to make sure everything hasbeen deleted as expected. If the timeout is exceeded fail the test.Not using `wait_for_sidekiq` as it didn't work. During testing Ididn't see any sidekiq processes as being busy even though noteverything was deleted.
e409638 to0aa0b27Compare
Uh oh!
There was an error while loading.Please reload this page.
Ensure reset_gitlab() succeeds by waiting to make sure everything has
been deleted as expected. If the timeout is exceeded fail the test.
Not using
wait_for_sidekiqas it didn't work. During testing Ididn't see any sidekiq processes as being busy even though not
everything was deleted.