codeant-ai-for-open-source[bot] commented on code in PR #38638:
URL: https://github.com/apache/superset/pull/38638#discussion_r2943000260
##########
tests/integration_tests/charts/data/api_tests.py:
##########
@@ -1810,3 +1810,224 @@ def test_chart_data_subquery_allowed(
rv = test_client.post(CHART_DATA_URI, json=physical_query_context)
assert rv.status_code == status_code
+
+
[email protected]_data_flow
[email protected](
+ reason=(
+ "TODO: Fix test class to work with DuckDB example data format. "
+ "Birth names fixture conflicts with new example data structure."
+ )
+)
Review Comment:
**Suggestion:** The newly added dashboard-filter GET test suite is fully
disabled by a class-level skip marker, so none of the new behavior is actually
validated in CI. This can let regressions in `filters_dashboard_id` handling
ship unnoticed; remove the class-level skip (or narrow skips to only specific
failing tests) so these tests execute. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ New `filters_dashboard_id` tests never execute in CI.
- ⚠️ GET chart-data error-path coverage remains missing.
- ❌ Regressions in dashboard-filter handling may ship unnoticed.
```
</details>
```suggestion
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open `tests/integration_tests/charts/data/api_tests.py:1815-1822`;
`TestGetChartDataWithDashboardFilter` has class-level
`@pytest.mark.skip(...)`, which
skips every method in the class.
2. In the same class (`api_tests.py:1858-2032`), all new
`filters_dashboard_id` test cases
are defined (200/400/403 branches), so they are all bypassed together by
that class skip.
3. Check production path in `superset/charts/data/api.py:183-205`;
`get_data()` parses
`filters_dashboard_id`, validates integer conversion, and handles
`ValueError`/`SupersetSecurityException` branches.
4. Because the class is globally skipped, CI never executes these branch
validations from
the new suite, so regressions in this feature path can merge undetected.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/integration_tests/charts/data/api_tests.py
**Line:** 1816:1821
**Comment:**
*Logic Error: The newly added dashboard-filter GET test suite is fully
disabled by a class-level skip marker, so none of the new behavior is actually
validated in CI. This can let regressions in `filters_dashboard_id` handling
ship unnoticed; remove the class-level skip (or narrow skips to only specific
failing tests) so these tests execute.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38638&comment_hash=aa0d2b394216b45e229c6a32215bdbd030de1fe61873383deb48bf3f72d9a3b5&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38638&comment_hash=aa0d2b394216b45e229c6a32215bdbd030de1fe61873383deb48bf3f72d9a3b5&reaction=dislike'>👎</a>
--
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]
