mirror of
https://github.com/apache/superset.git
synced 2026-05-30 04:39:20 +00:00
fix(mcp): fix E501 in update_chart.py and update_chart test mocks for column validation
Split an 89-char comment line and an over-limit condition in update_chart.py to satisfy the ruff E501 rule. Also applied ruff format. Two TestUpdateChartValidationGate tests expected CHART_VALIDATION_FAILED but received CHART_DATASET_NOT_FOUND because _validate_update_against_dataset calls DatasetValidator.validate_against_dataset before validate_and_compile, and the existing mocks provided a Mock() object for chart.datasource whose .id attribute is an auto-generated MagicMock (not a real int). Added a patch for DatasetValidator.validate_against_dataset returning (True, None) so the column-validation tier is bypassed and the test reaches the mocked validate_and_compile response as intended. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -196,7 +196,8 @@ def _validate_update_against_dataset(
|
||||
}
|
||||
)
|
||||
|
||||
# Column existence + fuzzy-match validation (mirrors generate_chart pipeline layer 2)
|
||||
# Column existence + fuzzy-match validation
|
||||
# (mirrors generate_chart pipeline layer 2)
|
||||
from superset.mcp_service.chart.validation.dataset_validator import DatasetValidator
|
||||
|
||||
is_col_valid, col_error = DatasetValidator.validate_against_dataset(
|
||||
@@ -414,11 +415,13 @@ async def update_chart( # noqa: C901
|
||||
|
||||
# Normalize column case to match dataset canonical names
|
||||
# (mirrors generate_chart pipeline layer 4)
|
||||
if parsed_config is not None and getattr(chart, "datasource_id", None) is not None:
|
||||
chart_datasource_id = getattr(chart, "datasource_id", None)
|
||||
if parsed_config is not None and chart_datasource_id is not None:
|
||||
from superset.mcp_service.chart.validation.dataset_validator import (
|
||||
DatasetValidator,
|
||||
NORMALIZATION_EXCEPTIONS,
|
||||
)
|
||||
|
||||
try:
|
||||
parsed_config = DatasetValidator.normalize_column_names(
|
||||
parsed_config, chart.datasource_id
|
||||
|
||||
10
tests/unit_tests/mcp_service/chart/tool/test_update_chart.py
Normal file → Executable file
10
tests/unit_tests/mcp_service/chart/tool/test_update_chart.py
Normal file → Executable file
@@ -1175,6 +1175,11 @@ class TestUpdateChartValidationGate:
|
||||
)
|
||||
@patch("superset.daos.chart.ChartDAO.find_by_id", new_callable=Mock)
|
||||
@patch("superset.db.session")
|
||||
@patch(
|
||||
"superset.mcp_service.chart.validation.dataset_validator"
|
||||
".DatasetValidator.validate_against_dataset",
|
||||
new=Mock(return_value=(True, None)),
|
||||
)
|
||||
@pytest.mark.asyncio
|
||||
async def test_preview_path_validation_failure_skips_cache(
|
||||
self,
|
||||
@@ -1238,6 +1243,11 @@ class TestUpdateChartValidationGate:
|
||||
)
|
||||
@patch("superset.daos.chart.ChartDAO.find_by_id", new_callable=Mock)
|
||||
@patch("superset.db.session")
|
||||
@patch(
|
||||
"superset.mcp_service.chart.validation.dataset_validator"
|
||||
".DatasetValidator.validate_against_dataset",
|
||||
new=Mock(return_value=(True, None)),
|
||||
)
|
||||
@pytest.mark.asyncio
|
||||
async def test_persist_path_validation_failure_skips_db_write(
|
||||
self,
|
||||
|
||||
Reference in New Issue
Block a user