vincbeck commented on PR #61351: URL: https://github.com/apache/airflow/pull/61351#issuecomment-3915966853
> Thanks for your review @vincbeck ! > > The tests evolved alongside the iterations on keycloak_auth_manager.py throughout this PR. After the last round of changes, the test file clearly needed a refactor to keep the intent of each test case explicit. That refactor is now done. The following tests were added for multi-team support: > > * `test_team_name_ignored_when_multi_team_disabled` — when multi_team is off, team_name in details has no effect on the permission string (stays global, e.g. Dag#GET) > * `test_with_team_name_uses_team_scoped_permission` — when multi_team is on and team_name is set, the permission is scoped (e.g. Dag:team-a#GET) > * `test_without_team_name_uses_global_permission` — when multi_team is on but no team_name, the permission stays global > * `test_list_without_team_name_uses_global_permission` — LIST without team_name produces a global permission (e.g. Dag#LIST) > * `test_list_with_team_name_uses_team_scoped_permission` — LIST with team_name produces a scoped permission (e.g. Dag:team-a#LIST) > * `test_list_with_mismatched_team_delegates_to_keycloak` — when the user's team doesn't match the requested team, the request is still forwarded to Keycloak (not short-circuited) to allow cross-team policies > * `test_filter_authorized_dag_ids_team_mismatch` / `test_filter_authorized_dag_ids_team_match` — filter_authorized_dag_ids delegates per dag and returns the correct subset > * `test_filter_authorized_pools_no_team_returns_empty` — filter_authorized_pools delegates to is_authorized_pool correctly > > The refactor should cover your last review! Overall I like the tests, thanks you! I added some comments, more style related which would help having a code easier to read -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
