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.

Sign up for GitHub

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

Make groups integration tests less flaky #965

Merged
merged 6 commits into from
Feb 21, 2024
Merged

Make groups integration tests less flaky #965

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c07e818
fix `test_env_or_skip`
nkvuong Feb 21, 2024
196c2ef
mark flaky tests
nkvuong Feb 21, 2024
8daffce
refactor groups integration tests
nkvuong Feb 21, 2024
f776924
fix
nkvuong Feb 21, 2024
e7e3ac8
fix
nkvuong Feb 21, 2024
2647f91
group rename
nkvuong Feb 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion tests/integration/framework/test_fixtures.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import logging

import pytest

# pylint: disable-next=import-private-name
from _pytest.outcomes import Failed, Skipped
from databricks.labs.blueprint.commands import CommandExecutor
from databricks.sdk.service.workspace import AclPermission

Expand Down Expand Up @@ -89,7 +92,7 @@ def test_sql_backend_works(ws, wsfs_wheel):


def test_env_or_skip(env_or_skip):
with pytest.raises((pytest.Skipped, pytest.Failed)):
with pytest.raises((Skipped, Failed)):
env_or_skip("NO_ENV_VAR_HERE")


Expand Down
152 changes: 52 additions & 100 deletions tests/integration/workspace_access/test_groups.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,12 @@ def test_prepare_environment_no_groups_selected(ws, make_ucx_group, sql_backend,
assert ws_group.display_name in names


# group rename is eventually consistent
@retried(on=[AssertionError], timeout=timedelta(minutes=1))
def check_group_renamed(ws, ws_group):
assert ws.groups.get(ws_group.id).display_name == "ucx-temp-" + ws_group.display_name


@retried(on=[NotFound], timeout=timedelta(minutes=2))
def test_rename_groups(ws, make_ucx_group, sql_backend, inventory_schema):
# FIXME - test_rename_groups - TimeoutError: Timed out after 0:01:00
Expand All @@ -58,7 +64,7 @@ def test_rename_groups(ws, make_ucx_group, sql_backend, inventory_schema):
group_manager = GroupManager(sql_backend, ws, inventory_schema, [ws_group.display_name], "ucx-temp-")
group_manager.rename_groups()

assert ws.groups.get(ws_group.id).display_name == "ucx-temp-" + ws_group.display_name
check_group_renamed(ws, ws_group)


@retried(on=[NotFound], timeout=timedelta(minutes=2))
Expand Down Expand Up @@ -87,9 +93,8 @@ def test_reflect_account_groups_on_workspace(ws, make_ucx_group, sql_backend, in
assert not reflected_group.roles # Cannot create roles currently
assert not reflected_group.entitlements # Entitlements aren't reflected there

assert (
ws.groups.get(ws_group.id).display_name == "ucx-temp-" + ws_group.display_name
) # At this time previous ws level groups aren't deleted
check_group_renamed(ws, ws_group)
# At this time previous ws level groups aren't deleted


@retried(on=[NotFound], timeout=timedelta(minutes=2))
Expand Down Expand Up @@ -124,127 +129,70 @@ def test_delete_ws_groups_should_not_delete_non_reflected_acc_groups(ws, make_uc
group_manager.rename_groups()
group_manager.delete_original_workspace_groups()

assert ws.groups.get(ws_group.id).display_name == "ucx-temp-" + ws_group.display_name
check_group_renamed(ws, ws_group)


def validate_migrate_groups(group_manager: GroupManager, ws_group: Group, to_group: Group):
assert group_manager.has_workspace_group(ws_group.display_name), f'missing workspace group: {ws_group.display_name}'
if not group_manager.has_workspace_group(ws_group.display_name):
raise NotFound(f'missing workspace group: {ws_group.display_name}')
group_manager.rename_groups()
assert group_manager.has_workspace_group(f"ucx-temp-{ws_group.display_name}"), 'missing temp group'
if not group_manager.has_workspace_group(f"ucx-temp-{ws_group.display_name}"):
raise NotFound('missing temp group')
group_manager.reflect_account_groups_on_workspace()
assert group_manager.has_account_group(to_group.display_name), f'missing account group: {to_group.display_name}'


@retried(on=[NotFound], timeout=timedelta(minutes=2))
def test_group_name_change_prefix(ws, sql_backend, inventory_schema, make_ucx_group, make_random):
ws_display_name = f"ucx_{make_random(4)}"
ws_group, accnt_group = make_ucx_group(
workspace_group_name=ws_display_name, account_group_name=f"SAMPLE_{ws_display_name}"
)
logger.info(
f"Attempting Mapping From Workspace Group {ws_group.display_name} to "
f"Account Group {accnt_group.display_name}"
)
group_manager = GroupManager(
sql_backend, ws, inventory_schema, [ws_group.display_name], "ucx-temp-", "^", "SAMPLE_"
)
validate_migrate_groups(group_manager, ws_group, accnt_group)
if not group_manager.has_account_group(to_group.display_name):
raise NotFound(f'missing account group: {to_group.display_name}')


@retried(on=[NotFound], timeout=timedelta(minutes=2))
def test_group_name_change_suffix(ws, sql_backend, inventory_schema, make_ucx_group, make_random):
ws_display_name = f"ucx_{make_random(4)}"
ws_group, accnt_group = make_ucx_group(
workspace_group_name=ws_display_name, account_group_name=f"{ws_display_name}_SAMPLE"
)
logger.info(
f"Attempting Mapping From Workspace Group {ws_group.display_name} to "
f"Account Group {accnt_group.display_name}"
)
group_manager = GroupManager(
sql_backend, ws, inventory_schema, [ws_group.display_name], "ucx-temp-", "$", "_SAMPLE"
)
validate_migrate_groups(group_manager, ws_group, accnt_group)


@retried(on=[NotFound], timeout=timedelta(minutes=2))
def test_group_name_change_substitute(ws, sql_backend, inventory_schema, make_ucx_group, make_random):
random_elem = f"{make_random(4)}"
ws_display_name = f"ucx_engineering_{random_elem}"
acct_display_name = f"ucx_eng_{random_elem}"
ws_group, accnt_group = make_ucx_group(workspace_group_name=ws_display_name, account_group_name=acct_display_name)
logger.info(
f"Attempting Mapping From Workspace Group {ws_group.display_name} to "
f"Account Group {accnt_group.display_name}"
)
group_manager = GroupManager(
sql_backend, ws, inventory_schema, [ws_group.display_name], "ucx-temp-", "engineering", "eng"
)
validate_migrate_groups(group_manager, ws_group, accnt_group)


@retried(on=[NotFound], timeout=timedelta(minutes=2))
def test_group_matching_names(ws, sql_backend, inventory_schema, make_ucx_group, make_random):
rand_elem = make_random(4)
ws_group, accnt_group = make_ucx_group(f"test_group_{rand_elem}", f"same_group_[{rand_elem}]")
logger.info(
f"Attempting Mapping From Workspace Group {ws_group.display_name} to "
f"Account Group {accnt_group.display_name}"
)
@pytest.mark.parametrize("strategy", ["prefix", "suffix", "substitute", "matching"])
def test_group_name_change(ws, sql_backend, inventory_schema, make_ucx_group, make_random, strategy):
random_element = f"ucx{make_random(4)}"
ws_group, account_group = None, None
workspace_group_regex, workspace_group_replace, account_group_regex = None, None, None
match strategy:
case "prefix":
ws_group, account_group = make_ucx_group(random_element, f"SAMPLE_{random_element}")
workspace_group_regex, workspace_group_replace = "^", "SAMPLE_"
case "suffix":
ws_group, account_group = make_ucx_group(random_element, f"{random_element}_SAMPLE")
workspace_group_regex, workspace_group_replace = "$", "_SAMPLE"
case "substitute":
ws_group, account_group = make_ucx_group(f"ucx_engineering_{random_element}", f"ucx_eng_{random_element}")
workspace_group_regex, workspace_group_replace = "engineering", "eng"
case "matching":
ws_group, account_group = make_ucx_group(f"test_group_{random_element}", f"same_group_[{random_element}]")
workspace_group_regex, account_group_regex = r"([0-9a-zA-Z]*)$", r"\[([0-9a-zA-Z]*)\]"
group_manager = GroupManager(
sql_backend,
ws,
inventory_schema,
[ws_group.display_name],
"ucx-temp-",
workspace_group_regex=r"([0-9a-zA-Z]*)$",
account_group_regex=r"\[([0-9a-zA-Z]*)\]",
workspace_group_regex,
workspace_group_replace,
account_group_regex,
)
validate_migrate_groups(group_manager, ws_group, accnt_group)


@retried(on=[NotFound], timeout=timedelta(minutes=2))
def test_group_matching_names_with_diff_users(
ws, sql_backend, inventory_schema, make_random, make_user, make_group, make_acc_group
):
rand_elem = make_random(4)
workspace_group_name = f"test_group_{rand_elem}"
account_group_name = f"same_group_[{rand_elem}]"
user1 = make_user()
user2 = make_user()
members1 = [user1.id]
members2 = [user2.id]
ws_group = make_group(display_name=workspace_group_name, members=members1, entitlements=["allow-cluster-create"])
accnt_group = make_acc_group(display_name=account_group_name, members=members2)

logger.info(
f"Attempting Mapping From Workspace Group {ws_group.display_name} to "
f"Account Group {accnt_group.display_name}"
f"Account Group {account_group.display_name}"
)
group_manager = GroupManager(
sql_backend,
ws,
inventory_schema,
[ws_group.display_name],
"ucx-temp-",
workspace_group_regex=r"([0-9a-zA-Z]*)$",
account_group_regex=r"\[([0-9a-zA-Z]*)\]",
)

membership = group_manager.validate_group_membership()
assert len(membership) > 0
validate_migrate_groups(group_manager, ws_group, account_group)


@retried(on=[NotFound], timeout=timedelta(minutes=2))
def test_group_matching_names_with_same_users(
ws, sql_backend, inventory_schema, make_random, make_user, make_group, make_acc_group
@pytest.mark.parametrize("same_user", [True, False])
def test_group_matching_names(
ws, sql_backend, inventory_schema, make_random, make_user, make_group, make_acc_group, same_user
):
rand_elem = make_random(4)
workspace_group_name = f"test_group_{rand_elem}"
account_group_name = f"same_group_[{rand_elem}]"
user1 = make_user()
members1 = [user1.id]
members2 = [user1.id]
if not same_user:
user2 = make_user()
members2 = [user2.id]
ws_group = make_group(display_name=workspace_group_name, members=members1, entitlements=["allow-cluster-create"])
acc_group = make_acc_group(display_name=account_group_name, members=members2)

Expand All @@ -257,12 +205,16 @@ def test_group_matching_names_with_same_users(
inventory_schema,
[ws_group.display_name],
"ucx-temp-",
workspace_group_regex=r"([0-9a-zA-Z]*)$",
account_group_regex=r"\[([0-9a-zA-Z]*)\]",
r"([0-9a-zA-Z]*)$",
None,
r"\[([0-9a-zA-Z]*)\]",
)

membership = group_manager.validate_group_membership()
assert len(membership) == 0
if same_user:
assert len(membership) == 0
else:
assert len(membership) > 0


# average runtime is 100 seconds
Expand Down
Loading