Changed list manipulation. Added 2 tests (for system tables and for w…

…orkspace only tables)

src/databricks/labs/ucx/workspace_access/groups.py

@@ -161,35 +161,34 @@ def prepare_groups_in_environment(self):
             "exist and are of the correct type. If some temporary groups are missing, they'll be created"
         )
         group_names = self.config.selected
+        valid_group_names = []
         if group_names:
             logger.info("Using the provided group listing")
 
             for g in group_names:
                 if g in self.SYSTEM_GROUPS:
                     logger.info(f"Cannot migrate system group {g}. {g} will be skipped.")
-                    group_names.remove(g)
                     continue
                 if not self._get_group(g, "workspace"):
                     logger.info(f"Group {g} not found on the workspace level. {g} will be skipped.")
-                    group_names.remove(g)
                     continue
                 if not self._get_group(g, "account"):
                     logger.info(
                         f"Group {g} not found on the account level. {g} will be skipped. You can add {g} "
                         f"to the account and rerun the job."
                     )
-                    group_names.remove(g)
                     continue
+                valid_group_names.append(g)
         else:
             logger.info(
                 "No group listing provided, all available workspace-level groups that have an account-level "
                 "group with the same name will be used"
             )
             ws_group_names = {_.display_name for _ in self._workspace_groups}
             ac_group_names = {_.display_name for _ in self._account_groups}
-            group_names = list(ws_group_names.intersection(ac_group_names))
+            valid_group_names = list(ws_group_names.intersection(ac_group_names))
 
-        self._set_migration_groups(group_names)
+        self._set_migration_groups(valid_group_names)
         logger.info("Environment prepared successfully")
 
     @property

tests/unit/workspace_access/test_groups.py

@@ -318,6 +318,44 @@ def test_replace_workspace_groups_with_account_groups_should_call_delete_and_do(
     )
 
 
+def test_system_groups():
+    client = Mock()
+    test_ws_group = Group(display_name="admins", meta=ResourceMeta(resource_type="WorkspaceGroup"))
+    test_acc_group = Group(display_name="admins", meta=ResourceMeta(resource_type="Group"))
+    backup_group_id = "100"
+    client.groups.list.return_value = [test_ws_group]
+    client.groups.create.return_value = Group(
+        display_name="dbr_backup_de", meta=ResourceMeta(resource_type="WorkspaceGroup"), id=backup_group_id
+    )
+    client.api_client.do.return_value = {
+        "Resources": [g.as_dict() for g in [test_acc_group]],
+    }
+
+    group_conf = GroupsConfig(backup_group_prefix="dbr_backup_", selected=["admins"])
+    manager = GroupManager(client, group_conf)
+    manager.prepare_groups_in_environment()
+    assert len(manager._migration_state.groups) == 0
+
+
+def test_workspace_only_groups():
+    client = Mock()
+    test_ws_group = Group(display_name="ws_group", meta=ResourceMeta(resource_type="WorkspaceGroup"))
+    test_acc_group = Group(display_name="acc_group", meta=ResourceMeta(resource_type="Group"))
+    backup_group_id = "100"
+    client.groups.list.return_value = [test_ws_group, test_acc_group]
+    client.groups.create.return_value = Group(
+        display_name="dbr_backup_de", meta=ResourceMeta(resource_type="WorkspaceGroup"), id=backup_group_id
+    )
+    client.api_client.do.return_value = {
+        "Resources": [g.as_dict() for g in [test_acc_group]],
+    }
+
+    group_conf = GroupsConfig(backup_group_prefix="dbr_backup_", selected=["ws_group"])
+    manager = GroupManager(client, group_conf)
+    manager.prepare_groups_in_environment()
+    assert len(manager._migration_state.groups) == 0
+
+
 def test_delete_backup_groups():
     client = Mock()