-
Notifications
You must be signed in to change notification settings - Fork 643
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
chore: ensure reset_gitlab() succeeds #1783
Conversation
abfcb9e
to
fa2b2c1
Compare
fa2b2c1
to
711ae44
Compare
112f07f
to
c9243ba
Compare
c9243ba
to
e409638
Compare
tests/functional/conftest.py
Outdated
def wait_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 _ in range(max_iterations): | ||
if rest_manager.list() == []: | ||
break | ||
time.sleep(SLEEP_INTERVAL) | ||
assert rest_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 _ in range(max_iterations): | ||
if len(gl.users.list()) <= 1: | ||
break | ||
time.sleep(SLEEP_INTERVAL) | ||
assert [user.username for user in gl.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)
def wait_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 _ in range(max_iterations): | |
if rest_manager.list() == []: | |
break | |
time.sleep(SLEEP_INTERVAL) | |
assert rest_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 _ in range(max_iterations): | |
if len(gl.users.list()) <= 1: | |
break | |
time.sleep(SLEEP_INTERVAL) | |
assert [user.username for user in gl.users.list()] == ["root"], ( | |
f"Did not delete all users. " | |
f"elapsed_time: {time.perf_counter() - start_time}" | |
) | |
def wait_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 _ in range(max_iterations): | |
if len(rest_manager.list()) <= length: | |
break | |
time.sleep(SLEEP_INTERVAL) | |
assert len(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 touch wait_for_sidekiq
code.
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_sidekiq` as it didn't work. During testing I didn't see any sidekiq processes as being busy even though not everything was deleted.
e409638
to
0aa0b27
Compare
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_sidekiq
as it didn't work. During testing Ididn't see any sidekiq processes as being busy even though not
everything was deleted.