YuriyKrasilnikov commented on PR #37395:
URL: https://github.com/apache/superset/pull/37395#issuecomment-3791255666

   ## Response to bito-code-review suggestion
   
   ### Analysis of the suggestion
   
   The bot noted that `test_public_api_includes_guest_rls` name suggests 
testing public API, but the code calls internal method.
   
   **Investigation of Superset test naming patterns:**
   
   ```bash
   # Examples from codebase:
   test_csv_reader_cast_column_types_function  # tests _cast_column_types
   test_get_sqla_engine_user_impersonation     # tests internal behavior
   test_query_context_modified_tampered        # describes scenario
   ```
   
   Superset pattern: `test_<functionality>_<scenario>` — no "public_api" or 
"internal_api" prefixes.
   
   ### Changes made
   
   Renamed tests to follow Superset naming convention:
   
   | Old Name | New Name |
   |----------|----------|
   | `test_public_api_includes_guest_rls` | 
`test_rls_filters_include_guest_when_enabled` |
   | `test_internal_api_excludes_guest_rls_when_requested` | 
`test_rls_filters_exclude_guest_when_requested` |
   | `test_internal_api_includes_guest_rls_by_default` | 
`test_rls_filters_include_guest_by_default` |
   
   ### Method naming verification
   
   The internal method name `_get_sqla_row_level_filters_internal` follows 
existing Superset patterns:
   - `_get_chart_preview_internal` (mcp_service/chart/tool/get_chart_preview.py)
   - `_get_screenshot_internal` (mcp_service/screenshot/pooled_screenshot.py)
   
   All 6 tests pass after renaming.


-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to